Skip to content

[codex] fix Oh My Zsh completion compinit setup #1033

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TabishB merged 1 commit into from
May 1, 2026
Merged

[codex] fix Oh My Zsh completion compinit setup #1033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 3 additions & 31 deletions src/core/completions/installers/zsh-installer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,27 +166,6 @@ export class ZshInstaller {
}
}

/**
* Check if fpath configuration is needed for a given directory
* Used to verify if Oh My Zsh (or other) completions directory is already in fpath
*
* @param completionsDir - Directory to check for in fpath
* @returns true if configuration is needed, false if directory is already referenced
*/
private async needsFpathConfig(completionsDir: string): Promise<boolean> {
try {
const zshrcPath = this.getZshrcPath();
const content = await fs.readFile(zshrcPath, 'utf-8');

// Check if fpath already includes this directory
return !content.includes(completionsDir);
} catch (error) {
// If we can't read .zshrc, assume config is needed
console.debug(`Unable to read .zshrc to check fpath config: ${error instanceof Error ? error.message : String(error)}`);
return true;
}
}

/**
* Remove .zshrc configuration
* Used during uninstallation
Expand Down Expand Up @@ -287,17 +266,10 @@ export class ZshInstaller {
// Write the completion script
await fs.writeFile(targetPath, completionScript, 'utf-8');

// Auto-configure .zshrc
// Auto-configure .zshrc for standard Zsh only.
// Oh My Zsh loads custom/completions and runs compinit itself.
let zshrcConfigured = false;
if (isOhMyZsh) {
// For Oh My Zsh, verify that custom/completions is in fpath
// If not, add it to .zshrc
const needsConfig = await this.needsFpathConfig(targetDir);
if (needsConfig) {
zshrcConfigured = await this.configureZshrc(targetDir);
}
} else {
// Standard Zsh always needs .zshrc configuration
if (!isOhMyZsh) {
zshrcConfigured = await this.configureZshrc(targetDir);
}

Expand Down
30 changes: 16 additions & 14 deletions test/core/completions/installers/zsh-installer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ describe('ZshInstaller', () => {

const result = await installer.install(testScript);

expect(result.zshrcConfigured).toBe(false);
expect(result.instructions).toBeDefined();
expect(result.instructions!.length).toBeGreaterThan(0);
// Should include guidance about verifying fpath for Oh My Zsh
Expand Down Expand Up @@ -630,28 +631,29 @@ describe('ZshInstaller', () => {
expect(content).toContain('compinit');
});

it('should configure .zshrc for Oh My Zsh when fpath is missing', async () => {
it('should not configure .zshrc for Oh My Zsh', async () => {
const ohMyZshPath = path.join(testHomeDir, '.oh-my-zsh');
await fs.mkdir(ohMyZshPath, { recursive: true });
const zshrcPath = path.join(testHomeDir, '.zshrc');
const originalZshrc = [
'export ZSH="$HOME/.oh-my-zsh"',
'source "$ZSH/oh-my-zsh.sh"',
'',
].join('\n');
await fs.writeFile(zshrcPath, originalZshrc);

const result = await installer.install(testScript);

expect(result.success).toBe(true);
expect(result.isOhMyZsh).toBe(true);
// Should configure .zshrc if fpath doesn't already include the directory
expect(result.zshrcConfigured).toBe(true);

// Verify .zshrc was created with fpath configuration
const zshrcPath = path.join(testHomeDir, '.zshrc');
const exists = await fs.access(zshrcPath).then(() => true).catch(() => false);
expect(exists).toBe(true);
expect(result.zshrcConfigured).toBe(false);

if (exists) {
const content = await fs.readFile(zshrcPath, 'utf-8');
expect(content).toContain('fpath=');
// Check for custom/completions or custom\completions (Windows path separator)
expect(content).toMatch(/custom[/\\]completions/);
}
const content = await fs.readFile(zshrcPath, 'utf-8');
expect(content).toBe(originalZshrc);
expect(content).not.toContain('# OPENSPEC:START');
expect(content).not.toContain('autoload -Uz compinit');
expect(content).not.toContain('compinit');
expect(result.instructions!.join('\n')).toContain('Oh My Zsh');
});

it('should not include manual instructions when .zshrc was auto-configured', async () => {
Expand Down
Loading