Skip to content

Commit b6bb205

Browse files
authored
Fix where activation goes missing after user modifying rc scripts (#880)
Resolves: #865 If user modifies already existing shell profile script needed for shell startup, they get stuck into not activating state because they still have auto activation guard, which blocks shell integration from core from activating. This also means because they have wrong env var in profile script, it won't activate there either. We should better track this, and properly clean up profile script when relying on shell integration for activation. /cc @karthiknadig
1 parent 10e9cdf commit b6bb205

File tree

2 files changed

+23
-10
lines changed

2 files changed

+23
-10
lines changed

src/features/terminal/shells/bash/bashStartup.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,12 @@ function getActivationContent(key: string): string {
6161
async function isStartupSetup(profile: string, key: string): Promise<ShellSetupState> {
6262
if (await fs.pathExists(profile)) {
6363
const content = await fs.readFile(profile, 'utf8');
64-
return hasStartupCode(content, regionStart, regionEnd, [key])
65-
? ShellSetupState.Setup
66-
: ShellSetupState.NotSetup;
67-
} else {
68-
return ShellSetupState.NotSetup;
64+
if (hasStartupCode(content, regionStart, regionEnd, [key])) {
65+
return ShellSetupState.Setup;
66+
}
6967
}
68+
return ShellSetupState.NotSetup;
7069
}
71-
7270
async function setupStartup(profile: string, key: string, name: string): Promise<boolean> {
7371
if (shellIntegrationForActiveTerminal(name, profile)) {
7472
removeStartup(profile, key);

src/features/terminal/terminalManager.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,15 +147,23 @@ export class TerminalManagerImpl implements TerminalManager {
147147
const shellsToSetup: ShellStartupScriptProvider[] = [];
148148
await Promise.all(
149149
providers.map(async (p) => {
150+
const state = await p.isSetup();
150151
if (this.shellSetup.has(p.shellType)) {
151-
traceVerbose(`Shell profile for ${p.shellType} already checked.`);
152-
return;
152+
// This ensures modified scripts are detected even after initial setup
153+
const cachedSetup = this.shellSetup.get(p.shellType);
154+
if ((state === ShellSetupState.Setup) !== cachedSetup) {
155+
traceVerbose(`Shell profile for ${p.shellType} state changed, updating cache.`);
156+
// State changed - clear cache and re-evaluate
157+
this.shellSetup.delete(p.shellType);
158+
} else {
159+
traceVerbose(`Shell profile for ${p.shellType} already checked.`);
160+
return;
161+
}
153162
}
154163
traceVerbose(`Checking shell profile for ${p.shellType}.`);
155-
const state = await p.isSetup();
156164
if (state === ShellSetupState.NotSetup) {
157-
// Check if shell integration is available before marking for setup
158165
if (shellIntegrationForActiveTerminal(p.name)) {
166+
await p.teardownScripts();
159167
this.shellSetup.set(p.shellType, true);
160168
traceVerbose(
161169
`Shell integration available for ${p.shellType}, skipping prompt, and profile modification.`,
@@ -169,6 +177,12 @@ export class TerminalManagerImpl implements TerminalManager {
169177
);
170178
}
171179
} else if (state === ShellSetupState.Setup) {
180+
if (shellIntegrationForActiveTerminal(p.name)) {
181+
await p.teardownScripts();
182+
traceVerbose(
183+
`Shell integration available for ${p.shellType}, removed profile script in favor of shell integration.`,
184+
);
185+
}
172186
this.shellSetup.set(p.shellType, true);
173187
traceVerbose(`Shell profile for ${p.shellType} is setup.`);
174188
} else if (state === ShellSetupState.NotInstalled) {
@@ -228,6 +242,7 @@ export class TerminalManagerImpl implements TerminalManager {
228242
let actType = getAutoActivationType();
229243
const shellType = identifyTerminalShell(terminal);
230244
if (actType === ACT_TYPE_SHELL) {
245+
await this.handleSetupCheck(shellType);
231246
actType = await this.getEffectiveActivationType(shellType);
232247
}
233248

0 commit comments

Comments
 (0)