Skip to content

Commit d79c005

Browse files
author
俊良
committed
fix(cli): tighten extension reload boundaries
1 parent 6075048 commit d79c005

19 files changed

Lines changed: 270 additions & 20 deletions

‎packages/cli/src/config/extension-file-watcher.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,22 @@ describe('ExtensionFileWatcher', () => {
163163
expect(refreshState.markExtensionContentChanged).not.toHaveBeenCalled();
164164
});
165165

166+
it('does not mark preferences or marketplace metadata as stale', () => {
167+
const refreshState = createRefreshState();
168+
const watcher = new ExtensionFileWatcher(
169+
configWithExtensions([]),
170+
extensionsDir,
171+
refreshState,
172+
);
173+
watcher.startWatching();
174+
175+
fireAllEvent(0, 'change', `${extensionsDir}/extension-preferences.json`);
176+
fireAllEvent(0, 'change', `${extensionsDir}/marketplaces.json`);
177+
178+
expect(refreshState.markExtensionsChanged).not.toHaveBeenCalled();
179+
expect(refreshState.markExtensionContentChanged).not.toHaveBeenCalled();
180+
});
181+
166182
it('auto-refreshes command, skill, and agent content changes', () => {
167183
const refreshState = createRefreshState();
168184
const watcher = new ExtensionFileWatcher(
@@ -222,6 +238,32 @@ describe('ExtensionFileWatcher', () => {
222238
expect(refreshState.markExtensionContentChanged).toHaveBeenCalledOnce();
223239
});
224240

241+
it('marks refresh needed for file-backed hook and LSP config changes', () => {
242+
const refreshState = createRefreshState();
243+
const watcher = new ExtensionFileWatcher(
244+
configWithExtensions([
245+
{
246+
path: `${extensionsDir}/alpha`,
247+
config: {
248+
hooks: 'config/hooks.json',
249+
lspServers: '/tmp/alpha-lsp.json',
250+
},
251+
installMetadata: undefined,
252+
contextFiles: [],
253+
},
254+
]),
255+
extensionsDir,
256+
refreshState,
257+
);
258+
watcher.startWatching();
259+
260+
fireAllEvent(0, 'change', `${extensionsDir}/alpha/config/hooks.json`);
261+
fireAllEvent(0, 'change', '/tmp/alpha-lsp.json');
262+
263+
expect(refreshState.markExtensionsChanged).toHaveBeenCalledTimes(2);
264+
expect(refreshState.markExtensionContentChanged).not.toHaveBeenCalled();
265+
});
266+
225267
it('does not watch inactive linked extension sources or context files', () => {
226268
const activeSource = '/tmp/active-linked-extension';
227269
const inactiveSource = '/tmp/inactive-linked-extension';

‎packages/cli/src/config/extension-file-watcher.ts‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,7 @@ import { ExtensionRefreshState } from './extension-refresh-state.js';
1818

1919
const debugLogger = createDebugLogger('EXTENSION_FILE_WATCHER');
2020

21-
const TOP_LEVEL_FILES = new Set([
22-
'extension-enablement.json',
23-
'extension-preferences.json',
24-
'marketplaces.json',
25-
]);
21+
const TOP_LEVEL_FILES = new Set(['extension-enablement.json']);
2622

2723
const EXTENSION_FILES = new Set([
2824
'qwen-extension.json',
@@ -43,7 +39,7 @@ export class ExtensionFileWatcher {
4339
private bootstrapWatcher?: FSWatcher;
4440
private mutationListenerDisposer?: () => void;
4541
private mutationSuppressionEnds = new Map<number, () => void>();
46-
private contextFiles = new Set<string>();
42+
private staleFiles = new Set<string>();
4743
private watching = false;
4844
private watchGeneration = 0;
4945

@@ -58,7 +54,7 @@ export class ExtensionFileWatcher {
5854
this.watching = true;
5955
const generation = ++this.watchGeneration;
6056
this.subscribeExtensionManagerMutations();
61-
this.contextFiles = this.getContextFiles();
57+
this.staleFiles = this.getStaleFiles();
6258
const roots = this.getWatchRoots();
6359

6460
if (roots.length > 0) {
@@ -144,7 +140,7 @@ export class ExtensionFileWatcher {
144140
return [...roots];
145141
}
146142

147-
private getContextFiles(): Set<string> {
143+
private getStaleFiles(): Set<string> {
148144
const files = new Set<string>();
149145
for (const extension of this.config.getActiveExtensions()) {
150146
for (const filePath of extension.contextFiles) {
@@ -160,10 +156,33 @@ export class ExtensionFileWatcher {
160156
for (const name of names) {
161157
files.add(path.resolve(extension.path, name));
162158
}
159+
this.addManifestFileReference(
160+
files,
161+
extension.path,
162+
extension.config.hooks,
163+
);
164+
this.addManifestFileReference(
165+
files,
166+
extension.path,
167+
extension.config.lspServers,
168+
);
163169
}
164170
return files;
165171
}
166172

173+
private addManifestFileReference(
174+
files: Set<string>,
175+
extensionPath: string,
176+
value: unknown,
177+
): void {
178+
if (typeof value !== 'string') return;
179+
files.add(
180+
path.isAbsolute(value)
181+
? path.resolve(value)
182+
: path.resolve(extensionPath, value),
183+
);
184+
}
185+
167186
private watchExtensionsParent(): void {
168187
this.closeBootstrapWatcher();
169188
const parentDir = path.dirname(this.extensionsDir);
@@ -196,7 +215,7 @@ export class ExtensionFileWatcher {
196215
event: WatchEvent,
197216
changedPath: string,
198217
): RefreshAction | false {
199-
if (this.contextFiles.has(changedPath)) {
218+
if (this.staleFiles.has(changedPath)) {
200219
return 'stale';
201220
}
202221
if (changedPath === path.resolve(this.extensionsDir)) {

‎packages/cli/src/config/extension-refresh-state.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,39 @@ describe('extension refresh state', () => {
200200
}
201201
});
202202

203+
it('preserves changes that arrive before a reload failure', () => {
204+
const staleListener = vi.fn();
205+
const contentListener = vi.fn();
206+
refreshState.on(AppEvent.ExtensionRefreshNeeded, staleListener);
207+
refreshState.on(AppEvent.ExtensionContentChanged, contentListener);
208+
209+
try {
210+
refreshState.notifyExtensionsReloadStarted();
211+
expect(refreshState.isReloadInProgress()).toBe(true);
212+
expect(refreshState.markExtensionContentChanged('content changed')).toBe(
213+
false,
214+
);
215+
expect(refreshState.markExtensionsChanged('manifest changed')).toBe(
216+
false,
217+
);
218+
219+
refreshState.markExtensionsReloadFailed('reload failed');
220+
expect(refreshState.isReloadInProgress()).toBe(false);
221+
expect(refreshState.needsExtensionRefresh()).toBe(true);
222+
expect(staleListener).toHaveBeenCalledWith('reload failed');
223+
224+
refreshState.clearExtensionsChanged();
225+
expect(refreshState.needsExtensionRefresh()).toBe(true);
226+
expect(staleListener).toHaveBeenCalledWith(
227+
'extension files changed during reload',
228+
);
229+
expect(contentListener).not.toHaveBeenCalled();
230+
} finally {
231+
refreshState.off(AppEvent.ExtensionRefreshNeeded, staleListener);
232+
refreshState.off(AppEvent.ExtensionContentChanged, contentListener);
233+
}
234+
});
235+
203236
it('settles only after all overlapping suppressions end', () => {
204237
const onSettle = vi.fn();
205238
const endFirst = refreshState.beginSuppression(onSettle);

‎packages/cli/src/config/extension-refresh-state.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,12 @@ export class ExtensionRefreshState {
8989
}
9090

9191
markExtensionsReloadFailed(reason = EXTENSION_RELOAD_FAILED_REASON): void {
92+
const changedDuringReload = this.changedDuringReload;
93+
const contentChangedDuringReload = this.contentChangedDuringReload;
9294
this.extensionRefreshNeeded = true;
9395
this.reloadInProgress = false;
94-
this.changedDuringReload = false;
95-
this.contentChangedDuringReload = false;
96+
this.changedDuringReload = changedDuringReload;
97+
this.contentChangedDuringReload = contentChangedDuringReload;
9698
this.suppressUntil = 0;
9799
this.events.emit(AppEvent.ExtensionsReloaded);
98100
this.events.emit(AppEvent.ExtensionRefreshNeeded, reason);
@@ -102,6 +104,10 @@ export class ExtensionRefreshState {
102104
return this.extensionRefreshNeeded;
103105
}
104106

107+
isReloadInProgress(): boolean {
108+
return this.reloadInProgress;
109+
}
110+
105111
beginSuppression(onSettle?: () => void): () => void {
106112
this.suppressionDepth++;
107113
let settled = false;

‎packages/cli/src/config/extension-runtime-reload.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ describe('reloadPluginsRuntime', () => {
1717
const refreshTools = vi.fn();
1818
const reloadCommands = vi.fn();
1919
const config = {
20+
isSafeMode: () => false,
2021
getExtensionManager: () => ({
2122
refreshCache,
2223
refreshTools,
@@ -63,6 +64,27 @@ describe('reloadPluginsRuntime', () => {
6364
lspServerCount: 1,
6465
});
6566
});
67+
68+
it('rejects without refreshing extensions in safe mode', async () => {
69+
const refreshCache = vi.fn();
70+
const refreshTools = vi.fn();
71+
const reloadCommands = vi.fn();
72+
const config = {
73+
isSafeMode: () => true,
74+
getExtensionManager: () => ({
75+
refreshCache,
76+
refreshTools,
77+
}),
78+
} as unknown as Config;
79+
80+
await expect(
81+
reloadPluginsRuntime({ config, reloadCommands }),
82+
).rejects.toThrow('Extension reload is disabled in safe mode.');
83+
84+
expect(refreshCache).not.toHaveBeenCalled();
85+
expect(refreshTools).not.toHaveBeenCalled();
86+
expect(reloadCommands).not.toHaveBeenCalled();
87+
});
6688
});
6789

6890
describe('refreshExtensionContentRuntime', () => {
@@ -72,6 +94,7 @@ describe('refreshExtensionContentRuntime', () => {
7294
const refreshSubagentCache = vi.fn();
7395
const reloadCommands = vi.fn();
7496
const config = {
97+
isSafeMode: () => false,
7598
getExtensionManager: () => ({ refreshCache }),
7699
getSkillManager: () => ({ refreshCache: refreshSkillCache }),
77100
getSubagentManager: () => ({ refreshCache: refreshSubagentCache }),
@@ -93,6 +116,7 @@ describe('refreshExtensionContentRuntime', () => {
93116
const refreshSubagentCache = vi.fn();
94117
const reloadCommands = vi.fn();
95118
const config = {
119+
isSafeMode: () => false,
96120
getExtensionManager: () => ({ refreshCache }),
97121
getSkillManager: () => ({ refreshCache: refreshSkillCache }),
98122
getSubagentManager: () => ({ refreshCache: refreshSubagentCache }),
@@ -113,6 +137,7 @@ describe('refreshExtensionContentRuntime', () => {
113137
const refreshSkillCache = vi.fn();
114138
const reloadCommands = vi.fn();
115139
const config = {
140+
isSafeMode: () => false,
116141
getExtensionManager: () => ({ refreshCache }),
117142
getSkillManager: () => ({ refreshCache: refreshSkillCache }),
118143
getSubagentManager: () => undefined,
@@ -126,4 +151,20 @@ describe('refreshExtensionContentRuntime', () => {
126151
expect(refreshSkillCache).toHaveBeenCalledOnce();
127152
expect(reloadCommands).toHaveBeenCalledOnce();
128153
});
154+
155+
it('skips content refresh in safe mode', async () => {
156+
const refreshCache = vi.fn();
157+
const reloadCommands = vi.fn();
158+
const config = {
159+
isSafeMode: () => true,
160+
getExtensionManager: () => ({ refreshCache }),
161+
} as unknown as Config;
162+
163+
await expect(
164+
refreshExtensionContentRuntime({ config, reloadCommands }),
165+
).resolves.toBeUndefined();
166+
167+
expect(refreshCache).not.toHaveBeenCalled();
168+
expect(reloadCommands).not.toHaveBeenCalled();
169+
});
129170
});

‎packages/cli/src/config/extension-runtime-reload.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ export async function reloadPluginsRuntime(options: {
2626
config: Config;
2727
reloadCommands?: () => void | Promise<void>;
2828
}): Promise<ReloadPluginsSummary> {
29+
if (options.config.isSafeMode()) {
30+
throw new Error('Extension reload is disabled in safe mode.');
31+
}
2932
const manager = options.config.getExtensionManager();
3033
await manager.refreshCache();
3134
await manager.refreshTools();
@@ -37,6 +40,8 @@ export async function refreshExtensionContentRuntime(options: {
3740
config: Config;
3841
reloadCommands?: () => void | Promise<void>;
3942
}): Promise<void> {
43+
if (options.config.isSafeMode()) return;
44+
4045
const manager = options.config.getExtensionManager();
4146
const errors: unknown[] = [];
4247
try {

‎packages/cli/src/gemini.tsx‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,25 @@ import { initializeLlmOutputLanguage } from './utils/languageUtils.js';
8080

8181
const debugLogger = createDebugLogger('STARTUP');
8282

83+
interface RuntimeLspReinitializeResult {
84+
reconcile: {
85+
added: string[];
86+
removed: string[];
87+
restarted: string[];
88+
unchanged: string[];
89+
failed: string[];
90+
};
91+
skipped: Array<{ name: string }>;
92+
}
93+
94+
interface RuntimeLspClient {
95+
reinitialize?: () => Promise<RuntimeLspReinitializeResult>;
96+
}
97+
98+
interface RuntimeLspConfig {
99+
reinitializeLsp?: () => Promise<RuntimeLspReinitializeResult | undefined>;
100+
}
101+
83102
function clearCorruptionEnvVars(): void {
84103
delete process.env[ENV_CORRUPTED_PATH];
85104
delete process.env[ENV_WAS_RECOVERED];
@@ -618,9 +637,10 @@ export async function main() {
618637
registerLspHotReload(config, registerCleanup);
619638

620639
const extensionRefreshState = new ExtensionRefreshState();
621-
const extensionFileWatcher = isBareMode(argv.bare)
622-
? undefined
623-
: new ExtensionFileWatcher(config, undefined, extensionRefreshState);
640+
const extensionFileWatcher =
641+
isBareMode(argv.bare) || config.isSafeMode()
642+
? undefined
643+
: new ExtensionFileWatcher(config, undefined, extensionRefreshState);
624644
extensionFileWatcher?.startWatching();
625645
if (extensionFileWatcher) {
626646
const restartExtensionWatcher = () =>
@@ -1092,9 +1112,15 @@ export function registerLspHotReload(
10921112
config: Config,
10931113
registerCleanup: (fn: () => void | Promise<void>) => void,
10941114
): void {
1115+
const lspClient = config.getLspClient?.() as
1116+
| (ReturnType<Config['getLspClient']> & RuntimeLspClient)
1117+
| undefined;
1118+
const runtimeConfig = config as Config & RuntimeLspConfig;
1119+
const reinitializeLsp = runtimeConfig.reinitializeLsp;
10951120
if (
10961121
config.isLspEnabled?.() !== true ||
1097-
!config.getLspClient?.()?.reinitialize
1122+
!lspClient?.reinitialize ||
1123+
!reinitializeLsp
10981124
) {
10991125
return;
11001126
}
@@ -1113,7 +1139,7 @@ export function registerLspHotReload(
11131139
);
11141140
let errorReported = false;
11151141
try {
1116-
const result = await config.reinitializeLsp();
1142+
const result = await reinitializeLsp();
11171143
if (result) {
11181144
const failedServers = getRuntimeReloadFailedNames(result.reconcile);
11191145
debugLogger.info(

‎packages/cli/src/i18n/locales/ca.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2088,4 +2088,6 @@ export default {
20882088
'Failed to refresh extension content: {{message}}. Run /reload-plugins to apply updates.',
20892089
'Failed to refresh extension content. Run /reload-plugins to apply updates.':
20902090
'Failed to refresh extension content. Run /reload-plugins to apply updates.',
2091+
'Extension reload did not complete. Run /reload-plugins to try again.':
2092+
'Extension reload did not complete. Run /reload-plugins to try again.',
20912093
};

‎packages/cli/src/i18n/locales/de.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2133,4 +2133,6 @@ export default {
21332133
'Failed to refresh extension content: {{message}}. Run /reload-plugins to apply updates.',
21342134
'Failed to refresh extension content. Run /reload-plugins to apply updates.':
21352135
'Failed to refresh extension content. Run /reload-plugins to apply updates.',
2136+
'Extension reload did not complete. Run /reload-plugins to try again.':
2137+
'Extension reload did not complete. Run /reload-plugins to try again.',
21362138
};

‎packages/cli/src/i18n/locales/en.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2574,4 +2574,6 @@ export default {
25742574
'Failed to refresh extension content: {{message}}. Run /reload-plugins to apply updates.',
25752575
'Failed to refresh extension content. Run /reload-plugins to apply updates.':
25762576
'Failed to refresh extension content. Run /reload-plugins to apply updates.',
2577+
'Extension reload did not complete. Run /reload-plugins to try again.':
2578+
'Extension reload did not complete. Run /reload-plugins to try again.',
25772579
};

0 commit comments

Comments
 (0)