Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/vs/platform/agentHost/common/state/sessionState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,12 @@ export interface ISessionGitState {
readonly hasGitHubRemote?: boolean;
/** Current branch name. */
readonly branchName?: string;
/**
* Whether `HEAD` is detached, which is why {@link branchName} is absent.
* Distinguishes a legitimately branch-less checkout from git state left
* behind by a probe that failed before it could resolve the branch.
*/
readonly isDetachedHead?: boolean;
/** Base branch the work targets (e.g. `main`). */
readonly baseBranchName?: string;
/** Upstream tracking branch (e.g. `origin/feature`). */
Expand Down Expand Up @@ -1581,6 +1587,7 @@ export function readSessionGitState(meta: SessionMeta | undefined): ISessionGitS
const result: {
hasGitHubRemote?: boolean;
branchName?: string;
isDetachedHead?: boolean;
baseBranchName?: string;
upstreamBranchName?: string;
incomingChanges?: number;
Expand All @@ -1593,6 +1600,7 @@ export function readSessionGitState(meta: SessionMeta | undefined): ISessionGitS
} = {};
if (typeof raw['hasGitHubRemote'] === 'boolean') { result.hasGitHubRemote = raw['hasGitHubRemote']; }
if (typeof raw['branchName'] === 'string') { result.branchName = raw['branchName']; }
if (typeof raw['isDetachedHead'] === 'boolean') { result.isDetachedHead = raw['isDetachedHead']; }
if (typeof raw['baseBranchName'] === 'string') { result.baseBranchName = raw['baseBranchName']; }
if (typeof raw['upstreamBranchName'] === 'string') { result.upstreamBranchName = raw['upstreamBranchName']; }
if (typeof raw['incomingChanges'] === 'number') { result.incomingChanges = raw['incomingChanges']; }
Expand All @@ -1605,6 +1613,23 @@ export function readSessionGitState(meta: SessionMeta | undefined): ISessionGitS
return result;
}

/**
* Whether a session's git state should be recomputed because it does not
* describe a usable checkout.
*
* A state that was never computed obviously qualifies. So does one that is
* missing its branch without a detached `HEAD` to explain it: `git status` is
* the only probe that reports the branch, so such a state is the residue of a
* probe that failed, and consumers that key off the branch (Agent Merge binds
* its pull request that way) stay stranded until it is recomputed. A detached
* `HEAD` is a legitimate branch-less checkout and must not be mistaken for it,
* or every caller would refresh in a loop against a repository that will never
* report a branch.
*/
export function needsSessionGitStateRefresh(gitState: ISessionGitState | undefined): boolean {
return gitState === undefined || (gitState.branchName === undefined && !gitState.isDetachedHead);
}

/**
* Returns a new {@link SessionMeta} with the git-state payload set to
* `gitState`, or with the git slot removed if `gitState` is `undefined`.
Expand Down
34 changes: 31 additions & 3 deletions src/vs/platform/agentHost/node/agentHostGitService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,18 @@ export class AgentHostGitService implements IAgentHostGitService {
configuredBaseBranch ? undefined : this._runGit(repositoryRoot, ['symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD']),
]);

// `git status` is the only probe that reports the branch, so a state
// computed without it is not merely incomplete — it is misleading.
// Callers persist the result wholesale, so returning a branch-less
// object here would overwrite the last known good branch and strand
// every consumer that keys off it (Agent Merge binds its pull request
// by branch). Report the failure instead and let callers keep what
// they already had.
if (statusOutput === undefined) {
this._logService.warn(`[agentHostGitService] Not reporting session git state because git status failed: ${repositoryRoot.fsPath}`);
return undefined;
}

const status = parseGitStatusV2(statusOutput);
const hasGitHubRemote = parseHasGitHubRemote(remotesOutput);
const baseBranchName = configuredBaseBranch ?? parseDefaultBranchRef(defaultBranchRef);
Expand Down Expand Up @@ -982,6 +994,7 @@ export class AgentHostGitService implements IAgentHostGitService {
const result: ISessionGitState = {
hasGitHubRemote,
branchName: status.branchName,
isDetachedHead: status.isDetachedHead,
baseBranchName,
upstreamBranchName: status.upstreamBranchName,
incomingChanges: status.incomingChanges,
Expand Down Expand Up @@ -1048,6 +1061,16 @@ export class AgentHostGitService implements IAgentHostGitService {
// raw progress/diagnostic text is still available.
if (stderr) {
this._logService.warn(`[agentHostGitService] > git ${args.join(' ')} failed; full stderr:\n${stderr}`);
} else if (didTimeOut || error.killed) {
// A timed-out or signalled git writes nothing to stderr,
// so this is the only trace such a failure ever leaves.
// Callers that degrade quietly on `undefined` are then
// impossible to diagnose from logs alone.
this._logService.warn(`[agentHostGitService] > git ${args.join(' ')} failed: ${formatGitError(args, timeoutMs, didTimeOut, error, stderr)}`);
} else {
// A silent non-zero exit is how the `--quiet` probes
// report "not found", so this stays below `warn`.
this._logService.trace(`[agentHostGitService] > git ${args.join(' ')} failed: ${formatGitError(args, timeoutMs, didTimeOut, error, stderr)}`);
}
if (options?.throwOnError) {
reject(new Error(formatGitError(args, timeoutMs, didTimeOut, error, stderr), { cause: error }));
Expand Down Expand Up @@ -1504,6 +1527,7 @@ export function parseGitDiffRawNumstat(output: string, repositoryRoot: URI, sess
*/
export function parseGitStatusV2(output: string | undefined): {
branchName?: string;
isDetachedHead?: boolean;
upstreamBranchName?: string;
outgoingChanges?: number;
incomingChanges?: number;
Expand All @@ -1513,6 +1537,7 @@ export function parseGitStatusV2(output: string | undefined): {
return {};
}
let branchName: string | undefined;
let isDetachedHead: boolean | undefined;
let upstreamBranchName: string | undefined;
let outgoingChanges: number | undefined;
let incomingChanges: number | undefined;
Expand All @@ -1522,8 +1547,11 @@ export function parseGitStatusV2(output: string | undefined): {
if (!line) { continue; }
if (line.startsWith('# branch.head ')) {
const head = line.substring('# branch.head '.length).trim();
// `(detached)` is what git emits for a detached HEAD. Treat as no branch.
branchName = head === '(detached)' ? undefined : head;
// `(detached)` is what git emits for a detached HEAD. Treat as no
// branch, but report why so consumers can tell an intentionally
// branch-less checkout from a status probe that never ran.
isDetachedHead = head === '(detached)' ? true : undefined;
branchName = isDetachedHead ? undefined : head;
} else if (line.startsWith('# branch.upstream ')) {
upstreamBranchName = line.substring('# branch.upstream '.length).trim();
} else if (line.startsWith('# branch.ab ')) {
Expand All @@ -1536,7 +1564,7 @@ export function parseGitStatusV2(output: string | undefined): {
uncommittedChanges++;
}
}
return { branchName, upstreamBranchName, outgoingChanges, incomingChanges, uncommittedChanges };
return { branchName, isDetachedHead, upstreamBranchName, outgoingChanges, incomingChanges, uncommittedChanges };
}

/** Exported for tests. */
Expand Down
55 changes: 52 additions & 3 deletions src/vs/platform/agentHost/node/agentMergeController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { deriveGitHubEndpoints } from '../common/githubEndpoints.js';
import { SessionConfigKey } from '../common/sessionConfigKeys.js';
import { ActionType } from '../common/state/protocol/common/actions.js';
import { AuthRequiredReason } from '../common/state/sessionActions.js';
import { getSessionRelatedPullRequestUrls, isAhpChatChannel, isSessionStatusArchived, parseRequiredSessionUriFromChatUri, readSessionGitHubState, readSessionGitState, SessionLifecycle, TurnState } from '../common/state/sessionState.js';
import { getSessionRelatedPullRequestUrls, isAhpChatChannel, isSessionStatusArchived, needsSessionGitStateRefresh, parseRequiredSessionUriFromChatUri, readSessionGitHubState, readSessionGitState, SessionLifecycle, TurnState } from '../common/state/sessionState.js';
import { IAgentConfigurationService } from './agentConfigurationService.js';
import { IAgentHostGitHubEndpointService } from './agentHostGitHubEndpointService.js';
import { AgentHostStateManager, IAgentHostStateManager } from './agentHostStateManager.js';
Expand All @@ -47,6 +47,13 @@ class AgentMergeRuntime extends Disposable {
readonly evaluationScheduler: RunOnceScheduler;
readonly backstopScheduler: RunOnceScheduler;
ref: PullRequestRef | undefined;
/**
* Whether this runtime already tried to recompute git state that reported
* no usable branch. Caps that repair at one git call per runtime so a
* checkout that can never report a branch does not spawn one on every
* backstop.
*/
didRefreshForMissingBranch = false;

constructor(
readonly session: string,
Expand Down Expand Up @@ -337,8 +344,10 @@ export class AgentMergeController extends Disposable {
if (!runtime || !state || !agentMerge?.enabled || this._stateManager.hasActiveTurn(session)) {
return;
}
const gitState = readSessionGitState(state._meta);
const branchName = gitState?.branchName;
const branchName = await this._resolveCurrentBranch(session, runtime, state);
if (!this._isCurrentRuntime(session, runtime)) {
return;
}
if (!branchName) {
this._logService.trace(`[AgentMergeController] Waiting for a current branch: session=${session}`);
runtime.backstopScheduler.schedule();
Expand Down Expand Up @@ -479,6 +488,46 @@ export class AgentMergeController extends Disposable {
}
}

/**
* Resolves the branch Agent Merge should act on, repairing session git
* state that does not report one.
*
* A failed git probe can leave persisted git state without a branch. The
* refresh that would repair it normally rides along with a client watching
* the session or an edit landing in the worktree, and neither happens for a
* session this controller is holding resident on its own. Every later step
* — binding the pull request, subscribing to it, acting on its feedback —
* is gated on the branch, so without this the session idles on the backstop
* indefinitely and Agent Merge silently never runs.
*
* A detached `HEAD` is excluded: it reports no branch by design, so
* refreshing would never produce one. The attempt is capped at once per
* runtime regardless, so any other checkout that cannot report a branch
* costs a single git call rather than one per backstop.
*/
private async _resolveCurrentBranch(session: string, runtime: AgentMergeRuntime, state: NonNullable<ReturnType<AgentHostStateManager['getSessionState']>>): Promise<string | undefined> {
const gitState = readSessionGitState(state._meta);
if (gitState?.branchName) {
return gitState.branchName;
}
if (runtime.didRefreshForMissingBranch || !needsSessionGitStateRefresh(gitState)) {
return undefined;
}
runtime.didRefreshForMissingBranch = true;
this._logService.debug(`[AgentMergeController] Refreshing git state because the session reports no branch: session=${session}`);
await this._gitStateService.refreshSessionGitState(session, state.workingDirectories?.[0] ? URI.parse(state.workingDirectories[0]) : undefined);
if (!this._isCurrentRuntime(session, runtime)) {
return undefined;
}
const refreshed = readSessionGitState(this._stateManager.getSessionState(session)?._meta)?.branchName;
if (refreshed) {
this._logService.info(`[AgentMergeController] Recovered the session branch after refreshing git state: session=${session}`);
} else {
this._logService.warn(`[AgentMergeController] Session still reports no branch after refreshing git state: session=${session}`);
}
return refreshed;
}
Comment thread
benibenj marked this conversation as resolved.

private async _resolveRef(parsed: IParsedPullRequestUrl, signal: AbortSignal): Promise<PullRequestRef | undefined> {
const credential = await this._gitHubService.credentials.getCredential(signal);
// The bound pull request URL carries its own host: after a restore or an
Expand Down
9 changes: 6 additions & 3 deletions src/vs/platform/agentHost/node/agentService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import type { InvokeChangesetOperationParams, InvokeChangesetOperationResult } f
import { AhpErrorCodes, AHP_SESSION_NOT_FOUND, ContentEncoding, JSON_RPC_INTERNAL_ERROR, ProtocolError, ResourceChangeType, ResourceType, ResourceWriteMode, type CreateResourceWatchParams, type CreateResourceWatchResult, type DirectoryEntry, type ResourceCopyParams, type ResourceCopyResult, type ResourceDeleteParams, type ResourceDeleteResult, type ResourceListResult, type ResourceMkdirParams, type ResourceMkdirResult, type ResourceMoveParams, type ResourceMoveResult, type ResourceReadResult, type ResourceResolveParams, type ResourceResolveResult, type ResourceWatchState, type ResourceWriteParams, type ResourceWriteResult, type IStateSnapshot } from '../common/state/sessionProtocol.js';
import { ChangesSummary, ChatInteractivity, ChatOriginKind, MessageAttachmentKind, type Annotation, type AnnotationEntry, type AnnotationsState, type ChatOrigin, type Customization, type Message, type MessageAttachment, type MessageResourceAttachment } from '../common/state/protocol/state.js';
import type { ChatPendingMessageSetAction, ChatTurnStartedAction, SessionConfigChangedAction } from '../common/state/protocol/actions.js';
import { ISessionGitHubState, ISessionGitState, MessageKind, ResponsePartKind, SESSION_META_GITHUB_KEY, SESSION_META_GIT_KEY, SESSION_META_MULTI_ROOT_KEY, SESSION_META_SOURCE_CONTROL_KEY, AH_META_ORCHESTRATION_DB_KEY, readSessionSpawnDepth, parseSessionOrchestration, withSessionSpawnDepth, withSessionOrchestration, SessionLifecycle, SessionStatus, ToolCallStatus, ToolResultContentType, AH_META_WORKSPACELESS_DB_KEY, AH_META_IS_ARCHIVED_DB_KEY, AH_META_IS_DONE_DB_KEY, AH_META_IS_READ_DB_KEY, buildChatUri, buildDefaultChatUri, buildResourceWatchChannelUri, buildSubagentChatUri, buildSubagentSessionUriPrefix, hostBuildInfoFromProduct, isAhpChatChannel, isDefaultChatUri, isSubagentChatUri, isSubagentSession, parseChatUri, parseDefaultChatUri, parseRequiredSessionUriFromChatUri, parseResourceWatchChannelUri, parseSessionMultiRootMetadata, parseSubagentSessionUri, readSessionExternal, readSessionGitHubState, readSessionGitState, readSessionMultiRootMetadata, readSessionSourceControlState, readSessionWorkspaceless, withSessionExternal, withSessionGitHubState, withSessionGitState, withSessionMultiRootMetadata, withSessionSourceControlState, withSessionStatusFlag, withSessionWorkspaceless, withSessionFolderPickerDecision, readSessionFolderPickerDecision, parseSessionFolderPickerDecision, SESSION_META_FOLDER_PICKER_KEY, readSessionEhcliAdoptable, type ISessionSourceControlState, type SessionConfigState, type SessionSummary, type ToolResultSubagentContent, type Turn, type UsageInfo, chatStorageUri, hasReportedUsage } from '../common/state/sessionState.js';
import { ISessionGitHubState, ISessionGitState, MessageKind, ResponsePartKind, SESSION_META_GITHUB_KEY, SESSION_META_GIT_KEY, SESSION_META_MULTI_ROOT_KEY, SESSION_META_SOURCE_CONTROL_KEY, AH_META_ORCHESTRATION_DB_KEY, readSessionSpawnDepth, parseSessionOrchestration, withSessionSpawnDepth, withSessionOrchestration, SessionLifecycle, SessionStatus, ToolCallStatus, ToolResultContentType, AH_META_WORKSPACELESS_DB_KEY, AH_META_IS_ARCHIVED_DB_KEY, AH_META_IS_DONE_DB_KEY, AH_META_IS_READ_DB_KEY, buildChatUri, buildDefaultChatUri, buildResourceWatchChannelUri, buildSubagentChatUri, buildSubagentSessionUriPrefix, hostBuildInfoFromProduct, isAhpChatChannel, isDefaultChatUri, isSubagentChatUri, isSubagentSession, needsSessionGitStateRefresh, parseChatUri, parseDefaultChatUri, parseRequiredSessionUriFromChatUri, parseResourceWatchChannelUri, parseSessionMultiRootMetadata, parseSubagentSessionUri, readSessionExternal, readSessionGitHubState, readSessionGitState, readSessionMultiRootMetadata, readSessionSourceControlState, readSessionWorkspaceless, withSessionExternal, withSessionGitHubState, withSessionGitState, withSessionMultiRootMetadata, withSessionSourceControlState, withSessionStatusFlag, withSessionWorkspaceless, withSessionFolderPickerDecision, readSessionFolderPickerDecision, parseSessionFolderPickerDecision, SESSION_META_FOLDER_PICKER_KEY, readSessionEhcliAdoptable, type ISessionSourceControlState, type SessionConfigState, type SessionSummary, type ToolResultSubagentContent, type Turn, type UsageInfo, chatStorageUri, hasReportedUsage } from '../common/state/sessionState.js';
import { readToolCallMeta } from '../common/meta/agentToolCallMeta.js';
import { readEphemeralSessionMeta, withEphemeralSessionMeta } from '../common/meta/agentEphemeralSessionMeta.js';
import { readChatSurfaceMeta, withChatSurfaceMeta } from '../common/meta/agentChatSurfaceMeta.js';
Expand Down Expand Up @@ -3842,9 +3842,12 @@ export class AgentService extends Disposable implements IAgentService {
// restore path that normally calls `_attachGitState` is skipped — so
// trigger it lazily here for the first subscriber. `_attachGitState`
// is async and updates `_meta.git` once ready, which clients see via
// the normal state-update stream.
// the normal state-update stream. State that does not describe a
// usable checkout counts as missing too: a failed probe can persist
// a branch-less remnant, and it would otherwise mask the very
// repair this lazy refresh exists to perform.
const sessionState = this._stateManager.getSessionState(resourceStr);
if (!isAhpChatChannel(resourceStr) && sessionState && readSessionGitState(sessionState._meta) === undefined) {
if (!isAhpChatChannel(resourceStr) && sessionState && needsSessionGitStateRefresh(readSessionGitState(sessionState._meta))) {
const workingDirectory = sessionState.workingDirectories?.[0]
? URI.parse(sessionState.workingDirectories[0])
: undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,20 @@ suite('AgentHostGitService - getSessionGitState (real git)', () => {
assert.strictEqual(result.hasGitHubRemote, false);
});

(hasGit ? test : test.skip)('reports no state at all when the status probe fails', async () => {
const dir = initRepo({ remote: 'https://github.com/owner/repo.git' });
const before = await svc!.getSessionGitState(URI.file(dir));
// The repository root is cached from the call above, so the probes still
// run against a repository that can no longer answer them — the same
// shape a probe takes when it times out under load. A partial state
// would be persisted over the branch this session still depends on.
rmDirWithRetry(join(dir, '.git'));

const after = await svc!.getSessionGitState(URI.file(dir));

assert.deepStrictEqual({ before: before?.branchName, after }, { before: 'main', after: undefined });
});

(hasGit ? test : test.skip)('reports outgoingChanges relative to base branch when local branch has no upstream', async () => {
// Create a bare "remote" repo and set up the working repo so that
// `refs/remotes/origin/HEAD` exists (required for baseBranchName parsing).
Expand Down
Loading