Skip to content

Commit 62ecf31

Browse files
committed
Preserve registered shell init revisions
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bef9b9c0-98f2-4b26-8b8b-18119efe7b13
1 parent 133c07d commit 62ecf31

2 files changed

Lines changed: 58 additions & 9 deletions

File tree

src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -999,6 +999,8 @@ export class CopilotAgentSession extends Disposable {
999999
private _shellInitScriptRegistered = false;
10001000
private _shellInitScriptMaterializationAttempted = false;
10011001
private _shellInitSandboxGrantApplied = false;
1002+
private _shellInitScriptRevision = 0;
1003+
private _registeredShellInitScriptResource: URI | undefined;
10021004
private readonly _shellInitScriptSequencer = new Sequencer();
10031005
private _shellInitScriptDisposing = false;
10041006
/**
@@ -3916,49 +3918,68 @@ export class CopilotAgentSession extends Disposable {
39163918
// The SDK requires init scripts to be readable when registered.
39173919
await this._applyEffectiveSandboxConfig(true);
39183920
this._shellInitSandboxGrantApplied = true;
3919-
const ref = await this._materializeShellInitScript(snippets[0]);
3920-
if (!ref) {
3921+
const materialized = await this._materializeShellInitScript(snippets[0]);
3922+
if (!materialized) {
39213923
// The write failed. Leave the cache unchanged so the next turn
39223924
// retries rather than permanently treating the script as applied.
39233925
return;
39243926
}
3925-
const result = await this._wrapper.session.rpc.options.update({ shell: { initScripts: [ref] } });
3927+
const result = await this._wrapper.session.rpc.options.update({ shell: { initScripts: [materialized.ref] } });
39263928
if (!result.success) {
3929+
await this._deleteShellInitScriptFile(materialized.resource);
39273930
throw new Error('Copilot SDK rejected shell init script update');
39283931
}
3932+
const previousResource = this._registeredShellInitScriptResource;
39293933
this._shellInitScriptRegistered = true;
3934+
this._registeredShellInitScriptResource = materialized.resource;
39303935
this._lastAppliedShellInitScripts = serialized;
3936+
if (previousResource) {
3937+
await this._deleteShellInitScriptFile(previousResource);
3938+
}
39313939
this._logService.trace(`[Copilot:${this.sessionId}] Applied shell init script`);
39323940
} catch (err) {
39333941
this._logService.warn(`[Copilot:${this.sessionId}] Failed to update shell init scripts`, err);
39343942
}
39353943
}
39363944

3937-
private async _materializeShellInitScript(script: IShellInitScript): Promise<{ shell: IShellInitScript['shell']; path: string } | undefined> {
3938-
const resource = URI.joinPath(this._shellInitScriptInstanceDirectory(), script.shell === 'powershell' ? 'init.ps1' : 'init.sh');
3945+
private async _materializeShellInitScript(script: IShellInitScript): Promise<{ ref: { shell: IShellInitScript['shell']; path: string }; resource: URI } | undefined> {
3946+
const extension = script.shell === 'powershell' ? 'ps1' : 'sh';
3947+
const resource = URI.joinPath(this._shellInitScriptInstanceDirectory(), `init-${++this._shellInitScriptRevision}.${extension}`);
39393948
const atomic = this._fileService.hasCapability(resource, FileSystemProviderCapabilities.FileAtomicWrite)
39403949
? { postfix: '.vsctmp' }
39413950
: false;
39423951
this._shellInitScriptMaterializationAttempted = true;
39433952
try {
39443953
await this._fileService.writeFile(resource, VSBuffer.fromString(script.script), { atomic });
3945-
return { shell: script.shell, path: resource.fsPath };
3954+
return { ref: { shell: script.shell, path: resource.fsPath }, resource };
39463955
} catch (error) {
39473956
this._logService.warn(`[Copilot:${this.sessionId}] Failed to write shell init script: ${getErrorMessage(error)}`);
39483957
return undefined;
39493958
}
39503959
}
39513960

3961+
private async _deleteShellInitScriptFile(resource: URI): Promise<void> {
3962+
try {
3963+
await this._fileService.del(resource);
3964+
} catch (error) {
3965+
if (!(error instanceof Error) || toFileOperationResult(error) !== FileOperationResult.FILE_NOT_FOUND) {
3966+
this._logService.warn(`[Copilot:${this.sessionId}] Failed to remove replaced shell init script: ${getErrorMessage(error)}`);
3967+
}
3968+
}
3969+
}
3970+
39523971
private async _clearShellInitScript(): Promise<void> {
39533972
if (!this._shellInitScriptMaterializationAttempted) {
39543973
return;
39553974
}
39563975
try {
39573976
await this._fileService.del(this._shellInitScriptInstanceDirectory(), { recursive: true });
39583977
this._shellInitScriptMaterializationAttempted = false;
3978+
this._registeredShellInitScriptResource = undefined;
39593979
} catch (error) {
39603980
if (error instanceof Error && toFileOperationResult(error) === FileOperationResult.FILE_NOT_FOUND) {
39613981
this._shellInitScriptMaterializationAttempted = false;
3982+
this._registeredShellInitScriptResource = undefined;
39623983
} else {
39633984
this._logService.warn(`[Copilot:${this.sessionId}] Failed to remove shell init script: ${getErrorMessage(error)}`);
39643985
}

src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12421,7 +12421,7 @@ Use the attached image as context.
1242112421

1242212422
await session.send('go', undefined, 'turn-1', 'interactive');
1242312423
const scriptPath = (mockSession.shellInitScriptUpdates.at(-1) as Array<{ path: string }>)?.[0]?.path;
12424-
assert.ok(scriptPath?.startsWith(TEST_SHELL_INIT_DIR) && scriptPath.endsWith('init.sh'), String(scriptPath));
12424+
assert.ok(scriptPath?.startsWith(TEST_SHELL_INIT_DIR) && /init-\d+\.sh$/.test(scriptPath), String(scriptPath));
1242512425

1242612426
const afterFirst = mockSession.shellInitScriptUpdates.length;
1242712427
await session.send('go', undefined, 'turn-2', 'interactive');
@@ -12430,12 +12430,14 @@ Use the attached image as context.
1243012430
setConfigValue(SessionConfigKey.ShellInitSnippets, [{ ...initScript, script: 'changed' }]);
1243112431
fireSessionConfigChange({ [SessionConfigKey.ShellInitSnippets]: [{ ...initScript, script: 'changed' }] });
1243212432
await timeout(0);
12433+
const changedScriptPath = (mockSession.shellInitScriptUpdates.at(-1) as Array<{ path: string }>)?.[0]?.path;
12434+
assert.ok(changedScriptPath && changedScriptPath !== scriptPath, String(changedScriptPath));
1243312435

1243412436
setConfigValue(SessionConfigKey.ShellInitSnippets, []);
1243512437
await session.send('go', undefined, 'turn-3', 'interactive');
1243612438
assert.deepStrictEqual(mockSession.shellInitScriptUpdates, [
1243712439
[{ shell: 'bash', path: scriptPath }],
12438-
[{ shell: 'bash', path: scriptPath }],
12440+
[{ shell: 'bash', path: changedScriptPath }],
1243912441
[],
1244012442
]);
1244112443
});
@@ -12455,6 +12457,32 @@ Use the attached image as context.
1245512457
assert.deepStrictEqual(mockSession.shellInitScriptUpdates, [registered]);
1245612458
});
1245712459

12460+
test('keeps the registered script file unchanged when a replacement is rejected', async () => {
12461+
const { session, mockSession, storedFileContents, setConfigValue } = await createAgentSession(disposables);
12462+
setConfigValue(SessionConfigKey.ShellInitSnippets, [initScript]);
12463+
await session.send('go', undefined, 'turn-1', 'interactive');
12464+
const registeredPath = (mockSession.shellInitScriptUpdates.at(-1) as Array<{ path: string }>)[0].path;
12465+
12466+
mockSession.shellInitScriptUpdateSuccess = false;
12467+
setConfigValue(SessionConfigKey.ShellInitSnippets, [{ ...initScript, script: 'rejected' }]);
12468+
await session.send('go', undefined, 'turn-2', 'interactive');
12469+
const updatesAfterRejection = mockSession.shellInitScriptUpdates.length;
12470+
12471+
mockSession.shellInitScriptUpdateSuccess = true;
12472+
setConfigValue(SessionConfigKey.ShellInitSnippets, [initScript]);
12473+
await session.send('go', undefined, 'turn-3', 'interactive');
12474+
12475+
assert.deepStrictEqual({
12476+
registeredContent: storedFileContents.get(URI.file(registeredPath).toString()),
12477+
registeredFileCount: [...storedFileContents.keys()].filter(key => key.includes('/agentHost/shellInit/')).length,
12478+
updateCount: mockSession.shellInitScriptUpdates.length,
12479+
}, {
12480+
registeredContent: initScript.script,
12481+
registeredFileCount: 1,
12482+
updateCount: updatesAfterRejection,
12483+
});
12484+
});
12485+
1245812486
test('each instance owns a distinct directory so a stale cleanup cannot delete a successor script', async () => {
1245912487
// dispose() queues the deletion without awaiting it; a resumed
1246012488
// replacement for the same SDK session may register its script first.
@@ -12489,7 +12517,7 @@ Use the attached image as context.
1248912517
});
1249012518
setConfigValue(SessionConfigKey.ShellInitSnippets, [initScript]);
1249112519
await session.send('go', undefined, 'turn-1', 'interactive');
12492-
assert.strictEqual([...storedFileContents.keys()].filter(key => key.includes('/test-session-1/') && key.endsWith('init.sh')).length, 2);
12520+
assert.strictEqual([...storedFileContents.keys()].filter(key => key.includes('/test-session-1/') && key.endsWith('.sh')).length, 2);
1249312521

1249412522
// The session-directory prune must not take the successor's script
1249512523
// with it; only this instance's directory may be removed.

0 commit comments

Comments
 (0)