Skip to content

Commit d5cbc42

Browse files
ThomasK33EhabY
authored andcommitted
fix: address review feedback
- Trim verbose JSDoc on mementoManager chat ID methods. - Wrap commands.open() in try/finally so the pending chat ID is cleared even if it throws. - Track auth retry timer ID and clear it on dispose. - Use textContent instead of innerHTML in webview error handler.
1 parent 52e8e49 commit d5cbc42

File tree

3 files changed

+25
-32
lines changed

3 files changed

+25
-32
lines changed

src/core/mementoManager.ts

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -58,22 +58,12 @@ export class MementoManager {
5858
return isFirst === true;
5959
}
6060

61-
/**
62-
* Store a chat ID to open after a window reload.
63-
* Used by the /open deep link handler: it must call
64-
* commands.open() which triggers a remote-authority
65-
* reload, wiping in-memory state. The chat ID is
66-
* persisted here so the extension can pick it up on
67-
* the other side of the reload.
68-
*/
61+
/** Store a chat ID to open after a remote-authority reload. */
6962
public async setPendingChatId(chatId: string): Promise<void> {
7063
await this.memento.update("pendingChatId", chatId);
7164
}
7265

73-
/**
74-
* Read and clear the pending chat ID. Returns
75-
* undefined if none was stored.
76-
*/
66+
/** Read and clear the pending chat ID (undefined if none). */
7767
public async getAndClearPendingChatId(): Promise<string | undefined> {
7868
const chatId = this.memento.get<string>("pendingChatId");
7969
if (chatId !== undefined) {
@@ -82,11 +72,7 @@ export class MementoManager {
8272
return chatId;
8373
}
8474

85-
/**
86-
* Clear the pending chat ID without reading it. Used when
87-
* the open flow is abandoned (e.g. user cancels a prompt)
88-
* so the stale ID does not leak into a future reload.
89-
*/
75+
/** Clear the pending chat ID without reading it. */
9076
public async clearPendingChatId(): Promise<void> {
9177
await this.memento.update("pendingChatId", undefined);
9278
}

src/uri/uriHandler.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,19 +100,21 @@ async function handleOpen(ctx: UriRouteContext): Promise<void> {
100100

101101
await setupDeployment(params, serviceContainer, deploymentManager);
102102

103-
const opened = await commands.open(
104-
owner,
105-
workspace,
106-
agent ?? undefined,
107-
folder ?? undefined,
108-
openRecent,
109-
);
110-
111-
// If commands.open() returned without opening a window (e.g. the
112-
// user cancelled a prompt), clear the pending chat ID so it does
113-
// not leak into a future, unrelated reload.
114-
if (!opened && chatId) {
115-
await mementoManager.clearPendingChatId();
103+
let opened = false;
104+
try {
105+
opened = await commands.open(
106+
owner,
107+
workspace,
108+
agent ?? undefined,
109+
folder ?? undefined,
110+
openRecent,
111+
);
112+
} finally {
113+
// Clear the pending chat ID if commands.open() did not
114+
// actually open a window (user cancelled, or it threw).
115+
if (!opened && chatId) {
116+
await mementoManager.clearPendingChatId();
117+
}
116118
}
117119
}
118120

src/webviews/chat/chatPanelProvider.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export class ChatPanelProvider
2727
private view?: vscode.WebviewView;
2828
private disposables: vscode.Disposable[] = [];
2929
private chatId: string | undefined;
30+
private authRetryTimer: ReturnType<typeof setTimeout> | undefined;
3031

3132
constructor(
3233
private readonly client: CoderApi,
@@ -116,7 +117,10 @@ export class ChatPanelProvider
116117
`Chat: no session token yet, retrying in ${delay}ms ` +
117118
`(attempt ${attempt + 1}/${ChatPanelProvider.MAX_AUTH_RETRIES})`,
118119
);
119-
setTimeout(() => this.sendAuthToken(attempt + 1), delay);
120+
this.authRetryTimer = setTimeout(
121+
() => this.sendAuthToken(attempt + 1),
122+
delay,
123+
);
120124
return;
121125
}
122126
this.logger.warn(
@@ -212,7 +216,7 @@ export class ChatPanelProvider
212216
}
213217
214218
if (data.type === 'coder:auth-error') {
215-
status.innerHTML = '';
219+
status.textContent = '';
216220
status.appendChild(document.createTextNode(data.error || 'Authentication failed.'));
217221
const btn = document.createElement('button');
218222
btn.id = 'retry-btn';
@@ -244,6 +248,7 @@ text-align:center;}</style></head>
244248
}
245249

246250
dispose(): void {
251+
clearTimeout(this.authRetryTimer);
247252
for (const d of this.disposables) {
248253
d.dispose();
249254
}

0 commit comments

Comments
 (0)