fix(vscode): file an editor comment under the session it was delivered to
A comment written with no session tab open opens one, and that panel knows its directory long before it has loaded the session list and selected its session. The webview filed the draft on the first snapshot that had a directory, with the session key falling back to "draft" because no session was current yet. The panel's composer reads the session's key, so the chip never appeared, while the editor thread saw the draft in the store snapshot and reported it attached. A session panel now stamps its session on every comment it delivers, and the webview waits until it actually shows that session before filing. The sidebar files on its current session or an open new-session draft, and no longer falls back to "draft" merely because nothing is selected yet. The resolver is a pure module with tests. Claude-Session: https://claude.ai/code/session_01VqV56Hez25hTxXH4ipJfzH
This commit is contained in:
@@ -87,7 +87,7 @@ The webview build emits each worker as one self-contained file. VS Code webviews
|
||||
- A thread never owns a draft. It mints the draft id, hands the payload to a chat webview (the active or newly opened session panel, else the sidebar), and follows the webview's whole-draft-list `inlineComments:sync` snapshots: present means show, absent after having been seen means dispose. A snapshot is tagged with the surface that produced it (a panel id or `sidebar`) and only decides that surface's own threads, because every webview runs its own draft store.
|
||||
- A comment the composer never confirms holding within 30 s is retracted from every surface's pending hold, its thread disposed, and the user told, so a thread cannot promise a send that will never happen.
|
||||
- `inlineCommentSelection.ts` holds the pure pieces (line ranges, the diff-side and real-path resolution for `git:` documents, the pending hold, removal broadcast, thread fate) without the `vscode` import so they are unit-tested directly.
|
||||
- Webview side: `webview/inlineCommentRemovals.ts` remembers removals that arrive before a delayed delivery lands, so a comment dropped while its panel was still booting does not appear as a chip later. The extension is not activated on startup for this; the right-click command activates it, and the gutter `+` appears from then on.
|
||||
- Webview side: `webview/inlineCommentTarget.ts` decides where a delivered comment is filed. A session panel stamps its session on every comment it delivers and the webview waits until it shows that session; the sidebar files on its current session or open draft. Filing on the first snapshot with a directory put the draft under `draft` while a fresh panel was still loading its session list, a key that composer never reads. `webview/inlineCommentRemovals.ts` remembers removals that arrive before a delayed delivery lands, so a comment dropped while its panel was still booting does not appear as a chip later. The extension is not activated on startup for this; the right-click command activates it, and the gutter `+` appears from then on.
|
||||
|
||||
## Shared webview message ordering
|
||||
|
||||
|
||||
@@ -30,6 +30,12 @@ type LineCommentPayload = {
|
||||
type SessionPanelState = {
|
||||
/** This panel's id, which is also its surface identity for comment threads. */
|
||||
id: string;
|
||||
/**
|
||||
* The session this panel was opened for; null for a new-session panel. A
|
||||
* comment delivered here names it, so the webview files the draft under that
|
||||
* session's key rather than whatever it shows while still booting.
|
||||
*/
|
||||
sessionId: string | null;
|
||||
panel: vscode.WebviewPanel;
|
||||
sseStreams: Map<string, AbortController>;
|
||||
/**
|
||||
@@ -148,6 +154,7 @@ export class SessionEditorPanelProvider {
|
||||
|
||||
const state: SessionPanelState = {
|
||||
id: panelId,
|
||||
sessionId: initialSessionId,
|
||||
panel,
|
||||
sseStreams: new Map(),
|
||||
pendingLineComments: [],
|
||||
@@ -183,7 +190,7 @@ export class SessionEditorPanelProvider {
|
||||
void panel.webview.postMessage({
|
||||
type: 'command',
|
||||
command: 'addLineComment',
|
||||
payload: pending,
|
||||
payload: { ...pending, targetSessionId: state.sessionId ?? undefined },
|
||||
});
|
||||
}
|
||||
|
||||
@@ -363,7 +370,7 @@ export class SessionEditorPanelProvider {
|
||||
void entry.panel.webview.postMessage({
|
||||
type: 'command',
|
||||
command: 'addLineComment',
|
||||
payload,
|
||||
payload: { ...payload, targetSessionId: entry.sessionId ?? undefined },
|
||||
});
|
||||
return entry.id;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import { describe, test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { resolveCommentTarget } from './inlineCommentTarget';
|
||||
|
||||
const snapshot = (overrides = {}) => ({
|
||||
currentSessionId: null,
|
||||
sessionDirectory: null,
|
||||
draftOpen: false,
|
||||
draftDirectory: null,
|
||||
currentDirectory: '/repo',
|
||||
...overrides,
|
||||
});
|
||||
|
||||
describe('resolveCommentTarget', () => {
|
||||
test('a booting panel with a directory but no session keeps waiting', () => {
|
||||
// This was the bug: the draft went under `draft` here, a key the
|
||||
// session's composer never reads, and the chip never appeared.
|
||||
assert.equal(resolveCommentTarget(snapshot(), 'ses_1'), null);
|
||||
assert.equal(resolveCommentTarget(snapshot()), null);
|
||||
});
|
||||
|
||||
test('a session panel files only once it shows the session the comment is for', () => {
|
||||
assert.equal(resolveCommentTarget(snapshot({ currentSessionId: 'ses_other', sessionDirectory: '/repo' }), 'ses_1'), null);
|
||||
assert.deepEqual(
|
||||
resolveCommentTarget(snapshot({ currentSessionId: 'ses_1', sessionDirectory: '/repo/wt' }), 'ses_1'),
|
||||
{ directory: '/repo/wt', sessionKey: 'ses_1' },
|
||||
);
|
||||
});
|
||||
|
||||
test('the session directory wins over the webview directory', () => {
|
||||
assert.deepEqual(
|
||||
resolveCommentTarget(snapshot({ currentSessionId: 'ses_1', sessionDirectory: '/repo/wt' })),
|
||||
{ directory: '/repo/wt', sessionKey: 'ses_1' },
|
||||
);
|
||||
assert.deepEqual(
|
||||
resolveCommentTarget(snapshot({ currentSessionId: 'ses_1' })),
|
||||
{ directory: '/repo', sessionKey: 'ses_1' },
|
||||
);
|
||||
});
|
||||
|
||||
test('the sidebar files on an open new-session draft', () => {
|
||||
assert.deepEqual(
|
||||
resolveCommentTarget(snapshot({ draftOpen: true, draftDirectory: '/repo/other' })),
|
||||
{ directory: '/repo/other', sessionKey: 'draft' },
|
||||
);
|
||||
assert.deepEqual(
|
||||
resolveCommentTarget(snapshot({ draftOpen: true })),
|
||||
{ directory: '/repo', sessionKey: 'draft' },
|
||||
);
|
||||
});
|
||||
|
||||
test('nothing is filed without any directory', () => {
|
||||
assert.equal(resolveCommentTarget(snapshot({ currentSessionId: 'ses_1', currentDirectory: null })), null);
|
||||
assert.equal(resolveCommentTarget(snapshot({ draftOpen: true, currentDirectory: null })), null);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,53 @@
|
||||
/**
|
||||
* Where a comment arriving from the editor should be filed.
|
||||
*
|
||||
* Inline drafts are keyed by directory and session, and the composer reads
|
||||
* exactly one key: the current session's, or `draft` while a new-session draft
|
||||
* is open. A panel that has just been opened knows its directory before it has
|
||||
* selected its session, so filing on the first snapshot that has a directory
|
||||
* put the draft under `draft`, a key that panel's composer never reads. The
|
||||
* chip never appeared while the editor thread reported it attached.
|
||||
*/
|
||||
|
||||
export interface CommentTargetSnapshot {
|
||||
currentSessionId: string | null;
|
||||
/** The current session's directory, when the session has one. */
|
||||
sessionDirectory: string | null;
|
||||
/** Whether a new-session draft is open, and the directory it points at. */
|
||||
draftOpen: boolean;
|
||||
draftDirectory: string | null;
|
||||
/** The webview's current directory, the last resort. */
|
||||
currentDirectory: string | null;
|
||||
}
|
||||
|
||||
export interface CommentTarget {
|
||||
directory: string;
|
||||
sessionKey: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* The draft key for this snapshot, or null while the surface is not yet
|
||||
* showing the session the comment is for.
|
||||
*
|
||||
* @param targetSessionId the session the extension delivered the comment to,
|
||||
* when it knows one (a session panel); undefined for the sidebar and a
|
||||
* new-session panel, which file wherever their composer currently is
|
||||
*/
|
||||
export function resolveCommentTarget(snapshot: CommentTargetSnapshot, targetSessionId?: string): CommentTarget | null {
|
||||
const { currentSessionId } = snapshot;
|
||||
if (targetSessionId) {
|
||||
if (currentSessionId !== targetSessionId) return null;
|
||||
const directory = snapshot.sessionDirectory ?? snapshot.currentDirectory;
|
||||
return directory ? { directory, sessionKey: targetSessionId } : null;
|
||||
}
|
||||
if (currentSessionId) {
|
||||
const directory = snapshot.sessionDirectory ?? snapshot.currentDirectory;
|
||||
return directory ? { directory, sessionKey: currentSessionId } : null;
|
||||
}
|
||||
if (snapshot.draftOpen) {
|
||||
const directory = snapshot.draftDirectory ?? snapshot.currentDirectory;
|
||||
return directory ? { directory, sessionKey: 'draft' } : null;
|
||||
}
|
||||
// No session and no draft yet: the surface is still booting.
|
||||
return null;
|
||||
}
|
||||
@@ -1,5 +1,6 @@
|
||||
import { createVSCodeAPIs } from './api';
|
||||
import { createRemovalTombstones } from './inlineCommentRemovals';
|
||||
import { resolveCommentTarget } from './inlineCommentTarget';
|
||||
import { onCommand, onThemeChange, postBridgeNotification, proxyApiRequest, proxySessionMessageRequest, sendBridgeMessage, startSseProxy, stopSseProxy } from './api/bridge';
|
||||
import { vscodeStreamPerfCount, vscodeStreamPerfMeasure, vscodeStreamPerfObserve } from './api/streamPerf';
|
||||
import { extractBodyBase64, extractBodyText, extractJsonBody, hasInitBody } from './requestBodyTransport';
|
||||
@@ -1348,11 +1349,15 @@ onCommand('addLineComment', (payload) => {
|
||||
code?: unknown;
|
||||
language?: unknown;
|
||||
comment?: unknown;
|
||||
targetSessionId?: unknown;
|
||||
};
|
||||
|
||||
// The editor thread mints the id so it can track its own draft without a
|
||||
// round trip. Absent when the comment came from anywhere else.
|
||||
const draftId = typeof record.draftId === 'string' && record.draftId ? record.draftId : undefined;
|
||||
// A session panel is told which session the comment is for, so it can wait
|
||||
// until it actually shows that session. The sidebar files wherever it is.
|
||||
const targetSessionId = typeof record.targetSessionId === 'string' && record.targetSessionId ? record.targetSessionId : undefined;
|
||||
const relativePath = typeof record.relativePath === 'string' ? record.relativePath : '';
|
||||
const source = record.source === 'diff' ? 'diff' : 'file';
|
||||
const side = record.side === 'original' || record.side === 'modified' ? record.side : undefined;
|
||||
@@ -1379,27 +1384,30 @@ onCommand('addLineComment', (payload) => {
|
||||
// reads. Directory precedence matches the composer's own.
|
||||
const resolveTarget = () => {
|
||||
const sessionState = useSessionUIStore.getState();
|
||||
const currentSessionId = sessionState.currentSessionId;
|
||||
const sessionDirectory = currentSessionId ? sessionState.getDirectoryForSession(currentSessionId) : null;
|
||||
const draftDirectory = sessionState.newSessionDraft?.open
|
||||
? sessionState.newSessionDraft.bootstrapPendingDirectory ?? sessionState.newSessionDraft.directoryOverride ?? null
|
||||
: null;
|
||||
const directory = sessionDirectory ?? draftDirectory ?? useDirectoryStore.getState().currentDirectory;
|
||||
return directory ? { directory, sessionKey: currentSessionId ?? 'draft' } : null;
|
||||
const currentSessionId = sessionState.currentSessionId ?? null;
|
||||
const draft = sessionState.newSessionDraft;
|
||||
return resolveCommentTarget({
|
||||
currentSessionId,
|
||||
sessionDirectory: currentSessionId ? sessionState.getDirectoryForSession(currentSessionId) ?? null : null,
|
||||
draftOpen: Boolean(draft?.open),
|
||||
draftDirectory: draft?.open ? draft.bootstrapPendingDirectory ?? draft.directoryOverride ?? null : null,
|
||||
currentDirectory: useDirectoryStore.getState().currentDirectory ?? null,
|
||||
}, targetSessionId);
|
||||
};
|
||||
|
||||
// A comment can arrive before the chat surface has finished booting: the
|
||||
// extension opens the sidebar and posts after a fixed delay, which a cold
|
||||
// webview can outlast. Dropping the draft here loses a comment the user
|
||||
// already wrote and already saw accepted in the editor, so wait for the
|
||||
// target to land instead.
|
||||
// A comment can arrive before the chat surface shows its session: a panel
|
||||
// opened for the comment knows its directory long before the session list
|
||||
// has loaded and the session is selected. Filing before that put the draft
|
||||
// under a key this composer never reads. Wait for the surface to land on
|
||||
// the session (or an open draft) instead, within the extension's own
|
||||
// confirmation deadline.
|
||||
let target = resolveTarget();
|
||||
for (let attempt = 0; !target && attempt < 40; attempt += 1) {
|
||||
for (let attempt = 0; !target && attempt < 80; attempt += 1) {
|
||||
await new Promise((resolve) => setTimeout(resolve, 250));
|
||||
target = resolveTarget();
|
||||
}
|
||||
if (!target) {
|
||||
console.warn('[openchamber] no directory resolved; dropping inline comment', { relativePath, startLine });
|
||||
console.warn('[openchamber] chat surface never showed the session; dropping inline comment', { relativePath, startLine, targetSessionId });
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user