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
7 changes: 6 additions & 1 deletion src/vs/base/browser/ui/dropdown/dropdownActionViewItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ export class DropdownMenuActionViewItem extends BaseActionViewItem {
export interface IActionWithDropdownActionViewItemOptions extends IActionViewItemOptions {
readonly menuActionsOrProvider: readonly IAction[] | IActionProvider;
readonly menuActionClassNames?: string[];
readonly keybindingProvider?: IKeybindingProvider;
}

export class ActionWithDropdownActionViewItem extends ActionViewItem {
Expand Down Expand Up @@ -220,7 +221,11 @@ export class ActionWithDropdownActionViewItem extends ActionViewItem {
separator.classList.toggle('prominent', menuActionClassNames.includes('prominent'));
append(this.element, separator);

this.dropdownMenuActionViewItem = this._register(new DropdownMenuActionViewItem(this._register(new Action('dropdownAction', nls.localize('moreActions', "More Actions..."))), menuActionsProvider, this.contextMenuProvider, { classNames: ['dropdown', ...ThemeIcon.asClassNameArray(Codicon.dropDownButton), ...menuActionClassNames], hoverDelegate: this.options.hoverDelegate }));
this.dropdownMenuActionViewItem = this._register(new DropdownMenuActionViewItem(this._register(new Action('dropdownAction', nls.localize('moreActions', "More Actions..."))), menuActionsProvider, this.contextMenuProvider, {
classNames: ['dropdown', ...ThemeIcon.asClassNameArray(Codicon.dropDownButton), ...menuActionClassNames],
hoverDelegate: this.options.hoverDelegate,
keybindingProvider: (<IActionWithDropdownActionViewItemOptions>this.options).keybindingProvider,
}));
this.dropdownMenuActionViewItem.render(this.element);

this._register(addDisposableListener(this.element, EventType.KEY_DOWN, e => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC
if (envelope.origin?.clientId === this._clientId
&& envelope.origin.clientSeq !== undefined
&& !envelope.rejectionReason) {
this._subscriptionManager.dropPendingSessionAction(envelope.channel, envelope.origin.clientSeq);
this._subscriptionManager.dropPendingAction(envelope.channel, envelope.origin.clientSeq);
}
if (envelope.serverSeq > maxSeq) {
maxSeq = envelope.serverSeq;
Expand Down Expand Up @@ -877,7 +877,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC
*
* 1. Resend pending optimistic session actions that the server did NOT
* echo back in the replay buffer (i.e. anything still on
* {@link AgentSubscriptionManager.getPendingSessionActions}).
* {@link AgentSubscriptionManager.getPendingActions}).
* 2. Flush every message that {@link _sendNotification} queued onto the
* outbox while the gate was engaged.
*
Expand All @@ -898,7 +898,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC
}

const replays: ProtocolMessage[] = [];
for (const entry of this._subscriptionManager.getPendingSessionActions()) {
for (const entry of this._subscriptionManager.getPendingActions()) {
if (queuedSeqs.has(entry.clientSeq)) {
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import type { Mutable } from '../../../../base/common/types.js';
import type { Annotation } from '../state/protocol/state.js';
import type { Annotation, AnnotationEntry } from '../state/protocol/state.js';

/**
* Shared convention for carrying agent-feedback semantics inside an
Expand Down Expand Up @@ -106,6 +106,68 @@ function isAgentFeedbackStateValue(value: unknown): value is AgentFeedbackStateV
return value === 'created' || value === 'accepted' || value === 'submitted' || value === 'resolved';
}

/**
* Who wrote a specific {@link AnnotationEntry} within a feedback comment.
*
* A comment's origin ({@link AgentFeedbackKindValue}) describes where the
* thread came from; an author describes each individual message in it, so a
* thread the user started can carry agent replies and vice versa. `unknown` is
* used when provenance cannot be established rather than assuming the user.
*/
export type AgentFeedbackAuthorValue = 'user' | 'agent' | 'prReviewer' | 'unknown';

/** Author semantics carried in an {@link AnnotationEntry._meta}. */
export interface IFeedbackAnnotationEntryMeta {
readonly author: AgentFeedbackAuthorValue;
}

function isAgentFeedbackAuthorValue(value: unknown): value is AgentFeedbackAuthorValue {
return value === 'user' || value === 'agent' || value === 'prReviewer' || value === 'unknown';
}

/**
* The author of a comment's opening entry, derived from the thread's origin:
* code review comments are written by an agent and PR review comments by a
* reviewer.
*/
export function authorForFeedbackKind(kind: AgentFeedbackKindValue | undefined): AgentFeedbackAuthorValue {
switch (kind) {
case 'user': return 'user';
case 'codeReview': return 'agent';
case 'prReview': return 'prReviewer';
default: return 'unknown';
}
}

/** Builds the `_meta` bag stamping {@link author} onto an annotation entry. */
export function feedbackAnnotationEntryMeta(author: AgentFeedbackAuthorValue): Record<string, unknown> {
return { [FEEDBACK_ANNOTATION_META_KEY]: { author } satisfies IFeedbackAnnotationEntryMeta };
}

/**
* Reads the author stamped onto an annotation entry, or `undefined` for
* entries written before authors were recorded.
*/
export function readFeedbackAnnotationEntryAuthor(entry: AnnotationEntry): AgentFeedbackAuthorValue | undefined {
const meta = entry._meta;
const slot = meta?.[FEEDBACK_ANNOTATION_META_KEY];
if (!slot || typeof slot !== 'object' || Array.isArray(slot)) {
return undefined;
}
const author = (slot as Record<string, unknown>)['author'];
return isAgentFeedbackAuthorValue(author) ? author : undefined;
}

/**
* Resolves the author of the entry at {@link index} within a comment of
* {@link kind}. Entries written before authors were recorded fall back to the
* thread's origin for the opening entry and to the user for replies — at that
* time replies could only be typed by the user.
*/
export function resolveFeedbackEntryAuthor(entry: AnnotationEntry, index: number, kind: AgentFeedbackKindValue | undefined): AgentFeedbackAuthorValue {
return readFeedbackAnnotationEntryAuthor(entry) ?? (index === 0 ? authorForFeedbackKind(kind) : 'user');
}

/**
* Reads the well-known {@link IFeedbackAnnotationMeta} from an annotation's
* `_meta` bag (under {@link FEEDBACK_ANNOTATION_META_KEY}). The annotations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,19 @@ export function isAgentFeedbackAnnotationsAttachment(attachment: MessageAttachme
/**
* Renders an agent-feedback annotations attachment into the textual hint shown
* to the agent. The hint references the attached comment ids and points the
* agent at the `listComments` tool to read their content.
* agent at the tools for reading and selectively replying to comments.
*/
export function renderAgentFeedbackAnnotationsAttachment(attachment: MessageAnnotationsAttachment): string | undefined {
const ids = attachment.annotationIds?.filter(isString) ?? [];
if (ids.length === 0) {
return undefined;
}
const idList = ids.map(id => `- ${id}`).join('\n');
return `The user attached specific feedback comments to act on (comment ids):\n${idList}\n\n` +
'Use the `listComments` tool to read their content and focus on these comments.';
return `The user selected these feedback comments for you to act on (comment ids):\n${idList}\n\n` +
'Use the `listComments` tool to read their content and focus on these comments. ' +
'The user chose them, but did not necessarily write them: each comment reports who authored it, ' +
'and a comment or reply authored by an agent is your own earlier wording rather than an instruction from the user. ' +
'Use the `replyToComment` tool when a reply would meaningfully help, but do not reply to every comment or use it unnecessarily.';
}

export function getAgentFeedbackAttachmentMetadata(attachment: MessageAttachment): IAgentFeedbackAttachmentMetadata | undefined {
Expand Down
44 changes: 31 additions & 13 deletions src/vs/platform/agentHost/common/state/agentSubscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ interface IPendingAction {
export interface IPendingDispatchAction {
readonly clientSeq: number;
/** The optimistic action awaiting confirmation. */
readonly action: SessionAction | ChatAction;
readonly action: SessionAction | ChatAction | AnnotationsAction;
/** URI of the channel this action targets, as stored on the subscription. */
readonly channel: string;
}
Expand Down Expand Up @@ -775,6 +775,24 @@ export class AnnotationsStateSubscription extends BaseAgentSubscription<Annotati
this._optimisticState = state;
this._onDidChange.fire(state);
}

clearPending(): void {
this._pendingActions.length = 0;
this._optimisticState = undefined;
}

getPendingActions(): IPendingDispatchAction[] {
return this._pendingActions.map(p => ({ clientSeq: p.clientSeq, action: p.action, channel: this._annotationsUri }));
}

dropPendingByClientSeq(clientSeq: number): boolean {
const index = this._pendingActions.findIndex(p => p.clientSeq === clientSeq);
if (index === -1) {
return false;
}
this._pendingActions.splice(index, 1);
return true;
}
}

type ManagedSubscriptionEntry = { sub: ManagedSubscription; kind: StateComponents; refCount: number; holders: Map<number, string> };
Expand Down Expand Up @@ -955,7 +973,7 @@ export class AgentSubscriptionManager extends Disposable {

private _disposeSubscriptionEntry(resource: URI, entry: ManagedSubscriptionEntry): void {
this._tryUnsubscribe(resource);
if (entry.sub instanceof SessionStateSubscription || entry.sub instanceof ChatStateSubscription) {
if (entry.sub instanceof SessionStateSubscription || entry.sub instanceof ChatStateSubscription || entry.sub instanceof AnnotationsStateSubscription) {
entry.sub.clearPending();
}
entry.sub.dispose();
Expand Down Expand Up @@ -1053,30 +1071,30 @@ export class AgentSubscriptionManager extends Disposable {
}

/**
* Snapshot of every pending optimistic action across all session
* subscriptions. Callers use this to replay actions after a transport
* Snapshot of every pending optimistic action that must survive reconnect.
* Callers use this to replay actions after a transport
* reconnect; entries are kept on their subscriptions until they're
* either echoed back by the server or explicitly dropped via
* {@link dropPendingSessionAction}.
* {@link dropPendingAction}.
*/
getPendingSessionActions(): IPendingDispatchAction[] {
getPendingActions(): IPendingDispatchAction[] {
const out: IPendingDispatchAction[] = [];
for (const { sub } of this._subscriptions.values()) {
if (sub instanceof SessionStateSubscription || sub instanceof ChatStateSubscription) {
if (sub instanceof SessionStateSubscription || sub instanceof ChatStateSubscription || sub instanceof AnnotationsStateSubscription) {
out.push(...sub.getPendingActions());
}
}
return out;
}

/**
* Remove a single pending optimistic action for a session by its
* Remove a single pending optimistic action by its
* `clientSeq`. Used during reconnect to evict actions the server
* already processed (and replayed back to us) so they're not resent.
*/
dropPendingSessionAction(sessionUri: string, clientSeq: number): void {
const entry = this._subscriptions.get(URI.parse(sessionUri));
if (entry?.sub instanceof SessionStateSubscription || entry?.sub instanceof ChatStateSubscription) {
dropPendingAction(resource: string, clientSeq: number): void {
const entry = this._subscriptions.get(URI.parse(resource));
if (entry?.sub instanceof SessionStateSubscription || entry?.sub instanceof ChatStateSubscription || entry?.sub instanceof AnnotationsStateSubscription) {
entry.sub.dropPendingByClientSeq(clientSeq);
}
}
Expand All @@ -1100,7 +1118,7 @@ export class AgentSubscriptionManager extends Disposable {
// Clear any pending optimistic actions before reseating confirmed
// state \u2014 they were predicated on the pre-disconnect confirmed
// state and won't reconcile correctly against a fresh snapshot.
if (!preservePending && (entry.sub instanceof SessionStateSubscription || entry.sub instanceof ChatStateSubscription)) {
if (!preservePending && (entry.sub instanceof SessionStateSubscription || entry.sub instanceof ChatStateSubscription || entry.sub instanceof AnnotationsStateSubscription)) {
entry.sub.clearPending();
}
entry.sub.handleSnapshot(state as never, fromSeq);
Expand All @@ -1116,7 +1134,7 @@ export class AgentSubscriptionManager extends Disposable {
for (const resource of missing) {
const entry = this._subscriptions.get(resource);
if (entry) {
if (entry.sub instanceof SessionStateSubscription || entry.sub instanceof ChatStateSubscription) {
if (entry.sub instanceof SessionStateSubscription || entry.sub instanceof ChatStateSubscription || entry.sub instanceof AnnotationsStateSubscription) {
entry.sub.clearPending();
}
entry.sub.setError(new Error(`Subscription no longer available after reconnect: ${resource.toString()}`));
Expand Down
22 changes: 19 additions & 3 deletions src/vs/platform/agentHost/node/agentHostStateManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import { TelemetryLevel } from '../../telemetry/common/telemetry.js';
import { ActionType, ActionEnvelope, ActionOrigin, INotification, IRootConfigChangedAction, SessionAction, ChatAction, RootAction, StateAction, TerminalAction, ChangesetAction, ClientChangesetAction, AnnotationsAction, ClientAnnotationsAction, isRootAction, isSessionAction, isChatAction, isChangesetAction, isAnnotationsAction, type AuthRequiredParams, type ProgressParams } from '../common/state/sessionActions.js';
import type { IStateSnapshot } from '../common/state/sessionProtocol.js';
import { rootReducer, sessionReducer, chatReducer, changesetReducer, annotationsReducer } from '../common/state/sessionReducers.js';
import { createRootState, createSessionState, createChatState, createDefaultChatSummary, chatSummaryFromState, buildDefaultChatUri, parseDefaultChatUri, parseRequiredSessionUriFromChatUri, isAhpChatChannel, isDefaultChatUri, mergeSessionWithDefaultChat, isAhpRootChannel, SessionLifecycle, withHostBuildInfo, type Changeset, type ChangesetState, type AnnotationsState, type ChatState, type ChatSummary, type Customization, type ISessionWithDefaultChat, type Message, type RootState, type SessionConfigState, type SessionMeta, type SessionState, type SessionSummary, type Turn, type URI, ROOT_STATE_URI, ChangesetStatus, IHostBuildInfo, SessionStatus } from '../common/state/sessionState.js';
import { createRootState, createSessionState, createChatState, createDefaultChatSummary, chatSummaryFromState, buildDefaultChatUri, parseDefaultChatUri, parseRequiredSessionUriFromChatUri, parseSubagentSessionUri, isAhpChatChannel, isDefaultChatUri, mergeSessionWithDefaultChat, isAhpRootChannel, SessionLifecycle, withHostBuildInfo, type Changeset, type ChangesetState, type AnnotationsState, type ChatState, type ChatSummary, type Customization, type ISessionWithDefaultChat, type Message, type RootState, type SessionConfigState, type SessionMeta, type SessionState, type SessionSummary, type Turn, type URI, ROOT_STATE_URI, ChangesetStatus, IHostBuildInfo, SessionStatus } from '../common/state/sessionState.js';
import { AgentHostTelemetryLevelConfigKey, IPermissionsValue, platformRootSchema, telemetryLevelToAgentHostConfigValue } from '../common/agentHostSchema.js';
import { SessionConfigKey } from '../common/sessionConfigKeys.js';
import { parseChangesetUri } from '../common/changesetUri.js';
import { buildAnnotationsUri, isAnnotationsUri } from '../common/annotationsUri.js';
import { buildAnnotationsUri, isAnnotationsUri, parseAnnotationsUri } from '../common/annotationsUri.js';
import { AgentHostChangesetStateCache, type IAgentHostChangesetStateRetentionOptions } from './agentHostChangesetStateCache.js';
import { ChangesSummary, ChatInteractivity, type ChatOrigin } from '../common/state/protocol/state.js';
import { arrayEquals, structuralEquals } from '../../../base/common/equals.js';
Expand Down Expand Up @@ -1359,7 +1359,23 @@ export class AgentHostStateManager extends Disposable {
* forthcoming `sessionRemoved` notification.
*/
disposeSessionAnnotations(session: URI): void {
this._annotations.delete(buildAnnotationsUri(session));
for (const resource of this._annotations.keys()) {
const annotations = parseAnnotationsUri(resource);
const subagent = annotations ? parseSubagentSessionUri(annotations.sessionUri) : undefined;
if (annotations?.sessionUri === session || subagent?.parentSession.toString() === session) {
this._annotations.delete(resource);
}
}
}

/** Restores a session's annotations before serving its first snapshot. */
restoreAnnotations(session: URI, state: AnnotationsState): void {
this._annotations.set(buildAnnotationsUri(session), state);
}

/** Returns the current annotations state for a channel, when materialized. */
getAnnotationsState(resource: URI): AnnotationsState | undefined {
return this._annotations.get(resource);
}

// ---- Turn tracking ------------------------------------------------------
Expand Down
Loading
Loading