Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
22 changes: 22 additions & 0 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 @@ -1048,6 +1060,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
35 changes: 33 additions & 2 deletions src/vs/platform/agentHost/node/agentMergeController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,8 +337,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 +481,35 @@ export class AgentMergeController extends Disposable {
}
}

/**
* Resolves the branch Agent Merge should act on, repairing session git
* state that is missing it.
*
* 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.
*/
private async _resolveCurrentBranch(session: string, runtime: AgentMergeRuntime, state: NonNullable<ReturnType<AgentHostStateManager['getSessionState']>>): Promise<string | undefined> {
const branchName = readSessionGitState(state._meta)?.branchName;
if (branchName) {
return branchName;
}
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}`);
}
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
6 changes: 4 additions & 2 deletions src/vs/platform/agentHost/node/agentService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3842,9 +3842,11 @@ 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. A state without a branch is treated
// as missing too: a failed git probe can persist one, 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 && readSessionGitState(sessionState._meta)?.branchName === undefined) {
Comment thread
benibenj marked this conversation as resolved.
Outdated
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
65 changes: 64 additions & 1 deletion src/vs/platform/agentHost/test/node/agentMergeController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { AgentHostAutoApprovePolicyRestrictedConfigKey, platformRootSchema, plat
import { IAgentHostGitStateService } from '../../common/agentHostGitStateService.js';
import { SessionConfigKey } from '../../common/sessionConfigKeys.js';
import { ActionType } from '../../common/state/protocol/common/actions.js';
import { SessionStatus, buildDefaultChatUri, MessageKind, type SessionSummary } from '../../common/state/sessionState.js';
import { SessionStatus, buildDefaultChatUri, MessageKind, withSessionGitState, type SessionSummary } from '../../common/state/sessionState.js';
import { IGitHubService } from '../../../github/common/githubService.js';
import { AgentConfigurationService } from '../../node/agentConfigurationService.js';
import { AgentHostGitHubEndpointService } from '../../node/agentHostGitHubEndpointService.js';
Expand Down Expand Up @@ -218,6 +218,69 @@ suite('AgentMergeController', () => {
});
});

test('recovers a session whose persisted git state lost its branch', async () => {
const logService = new NullLogService();
const stateManager = disposables.add(new AgentHostStateManager(logService));
const configurationService = disposables.add(new AgentConfigurationService(stateManager, logService));
configurationService.updateRootConfig({ [AgentMergeConfigKey.Enabled]: true });
const session = `copilot:/agent-merge-controller-${++sessionCounter}`;
let refreshCount = 0;
const gitStateService = new class extends mock<IAgentHostGitStateService>() {
override readonly onDidRefreshSessionGitState = Event.None;
override readonly onDidChangeSessionGitHubState = Event.None;
override async refreshSessionGitState(sessionKey: string): Promise<void> {
refreshCount++;
stateManager.setSessionMeta(sessionKey, withSessionGitState(stateManager.getSessionState(sessionKey)?._meta, { branchName: 'feature', baseBranchName: 'main' }));
}
// The follow-up evaluation triggered by capturing the target reaches
// this; it finds no pull request and idles on the backstop.
override async attachSessionGitHubPullRequest(): Promise<void> { }
}();
const endpointService = disposables.add(new AgentHostGitHubEndpointService(configurationService, logService));
disposables.add(new AgentMergeController(
{
startTurn: () => false,
cancelTurn: () => { },
getAutonomousSessionConfig: () => ({}),
},
stateManager,
configurationService,
gitStateService,
new class extends mock<IGitHubService>() { }(),
endpointService,
logService,
));
stateManager.createSession(summary(session));
stateManager.setSessionConfig(session, {
schema: platformSessionSchema.toProtocol(),
values: {},
});
// A failed git probe leaves the branch behind but keeps the base branch,
// which is exactly the state that used to stall Agent Merge forever.
stateManager.setSessionMeta(session, withSessionGitState(undefined, { baseBranchName: 'main' }));
configurationService.updateSessionConfig(session, {
[SessionConfigKey.AgentMerge]: { enabled: true },
});

const captured = new Promise<void>(resolve => {
disposables.add(stateManager.onDidChangeSessionConfig(event => {
if (event.session.toString() === session && readAgentMergeSessionState(event.current?.values)?.target) {
resolve();
}
}));
});
stateManager.dispatchServerAction(session, { type: ActionType.SessionReady });
await captured;

assert.deepStrictEqual({
refreshCount,
branchName: readAgentMergeSessionState(configurationService.getSessionConfigValues(session))?.target?.branchName,
}, {
refreshCount: 1,
branchName: 'feature',
});
});

function createControllerHarness(disposables: ReturnType<typeof ensureNoDisposablesAreLeakedInTestSuite>): {
readonly stateManager: AgentHostStateManager;
readonly configurationService: AgentConfigurationService;
Expand Down
Loading