chore: apply coderabbit requested changes pre-0.39.0

This commit is contained in:
Ralph Khreish
2025-12-16 23:55:40 +01:00
parent a1170c5173
commit 2cbdf11d32
2 changed files with 154 additions and 8 deletions

View File

@@ -202,7 +202,7 @@ describe('shell-utils', () => {
expect(result.alreadyExists).toBeUndefined();
expect(fs.appendFileSync).toHaveBeenCalledWith(
shellConfigPath,
'\n# My comment\nexport MY_VAR=my_value\n'
"\n# My comment\nexport MY_VAR='my_value'\n"
);
// Cleanup
@@ -249,7 +249,7 @@ describe('shell-utils', () => {
expect(result.alreadyExists).toBeUndefined();
expect(fs.appendFileSync).toHaveBeenCalledWith(
shellConfigPath,
'\nexport MY_VAR=my_value\n'
"\nexport MY_VAR='my_value'\n"
);
// Cleanup
@@ -275,7 +275,7 @@ describe('shell-utils', () => {
expect(result.alreadyExists).toBeUndefined();
expect(fs.appendFileSync).toHaveBeenCalledWith(
shellConfigPath,
'\nexport MY_VAR=my_value\n'
"\nexport MY_VAR='my_value'\n"
);
// Cleanup
@@ -455,6 +455,125 @@ describe('shell-utils', () => {
process.env.PSModulePath = originalPSModulePath;
Object.defineProperty(process, 'platform', { value: originalPlatform });
});
it('should use fish shell syntax for config.fish files', () => {
// Arrange
const originalShell = process.env.SHELL;
process.env.SHELL = '/usr/bin/fish';
const shellConfigPath = path.join(
mockHomeDir,
'.config',
'fish',
'config.fish'
);
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.readFileSync).mockReturnValue('# existing content\n');
vi.mocked(fs.appendFileSync).mockImplementation(() => {});
// Act
const result = addShellExport('MY_VAR', 'my_value', 'My comment');
// Assert
expect(result.success).toBe(true);
expect(fs.appendFileSync).toHaveBeenCalledWith(
shellConfigPath,
"\n# My comment\nset -gx MY_VAR 'my_value'\n"
);
// Cleanup
process.env.SHELL = originalShell;
});
it('should detect existing fish shell export', () => {
// Arrange
const originalShell = process.env.SHELL;
process.env.SHELL = '/usr/bin/fish';
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.readFileSync).mockReturnValue('set -gx MY_VAR old_value\n');
// Act
const result = addShellExport('MY_VAR', 'my_value');
// Assert
expect(result.success).toBe(true);
expect(result.alreadyExists).toBe(true);
expect(fs.appendFileSync).not.toHaveBeenCalled();
// Cleanup
process.env.SHELL = originalShell;
});
it('should detect existing fish shell export with different flag order', () => {
// Arrange
const originalShell = process.env.SHELL;
process.env.SHELL = '/usr/bin/fish';
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.readFileSync).mockReturnValue('set -xg MY_VAR old_value\n');
// Act
const result = addShellExport('MY_VAR', 'my_value');
// Assert
expect(result.success).toBe(true);
expect(result.alreadyExists).toBe(true);
expect(fs.appendFileSync).not.toHaveBeenCalled();
// Cleanup
process.env.SHELL = originalShell;
});
it('should NOT skip fish when variable name only appears in a comment', () => {
// Arrange
const originalShell = process.env.SHELL;
process.env.SHELL = '/usr/bin/fish';
const shellConfigPath = path.join(
mockHomeDir,
'.config',
'fish',
'config.fish'
);
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.readFileSync).mockReturnValue(
'# MY_VAR is mentioned here but not set\n# set -gx MY_VAR in a comment\n'
);
vi.mocked(fs.appendFileSync).mockImplementation(() => {});
// Act
const result = addShellExport('MY_VAR', 'my_value');
// Assert
expect(result.success).toBe(true);
expect(result.alreadyExists).toBeUndefined();
expect(fs.appendFileSync).toHaveBeenCalledWith(
shellConfigPath,
"\nset -gx MY_VAR 'my_value'\n"
);
// Cleanup
process.env.SHELL = originalShell;
});
it('should create fish config directory if it does not exist', () => {
// Arrange
const originalShell = process.env.SHELL;
process.env.SHELL = '/usr/bin/fish';
// Fish config directory doesn't exist initially
vi.mocked(fs.existsSync).mockReturnValue(false);
vi.mocked(fs.mkdirSync).mockImplementation(() => undefined);
vi.mocked(fs.readFileSync).mockReturnValue('');
vi.mocked(fs.appendFileSync).mockImplementation(() => {});
// Act
const result = addShellExport('MY_VAR', 'my_value');
// Assert - fish config file not found since we don't auto-create it (unlike PowerShell)
expect(result.success).toBe(false);
expect(result.message).toContain('not found');
// Cleanup
process.env.SHELL = originalShell;
});
});
describe('enableDeferredMcpLoading', () => {
@@ -475,7 +594,7 @@ describe('shell-utils', () => {
expect(result.shellConfigFile).toBe(shellConfigPath);
expect(fs.appendFileSync).toHaveBeenCalledWith(
shellConfigPath,
'\n# Claude Code deferred MCP loading (added by Taskmaster)\nexport ENABLE_EXPERIMENTAL_MCP_CLI=true\n'
"\n# Claude Code deferred MCP loading (added by Taskmaster)\nexport ENABLE_EXPERIMENTAL_MCP_CLI='true'\n"
);
// Cleanup

View File

@@ -91,6 +91,13 @@ function isPowerShellProfile(filePath: string): boolean {
return filePath.endsWith('.ps1');
}
/**
* Checks if a shell config file is a fish config
*/
function isFishConfig(filePath: string): boolean {
return filePath.includes('config.fish');
}
/**
* Adds an export statement to the user's shell configuration file
* Handles both Unix-style shells (bash, zsh, fish) and PowerShell
@@ -161,9 +168,23 @@ export function addShellExport(
// Check if the export already exists using precise regex patterns
// This avoids false positives from comments or partial matches
const alreadyExists = isPowerShellProfile(shellConfigFile)
? new RegExp(`^\\s*\\$env:${envVar}\\s*=`, 'm').test(content)
: new RegExp(`^\\s*export\\s+${envVar}\\s*=`, 'm').test(content);
let alreadyExists: boolean;
if (isPowerShellProfile(shellConfigFile)) {
alreadyExists = new RegExp(`^\\s*\\$env:${envVar}\\s*=`, 'm').test(
content
);
} else if (isFishConfig(shellConfigFile)) {
// Fish uses 'set -gx VAR value' or 'set -xg VAR value' syntax
// Only match exported variables (must have 'x' flag)
alreadyExists = new RegExp(
`^\\s*set\\s+-[gux]*x[gux]*\\s+${envVar}\\s+`,
'm'
).test(content);
} else {
alreadyExists = new RegExp(`^\\s*export\\s+${envVar}\\s*=`, 'm').test(
content
);
}
if (alreadyExists) {
return {
@@ -183,8 +204,14 @@ export function addShellExport(
const escapedValue = value.replace(/["`$]/g, '`$&');
exportLine = `$env:${envVar} = "${escapedValue}"`;
commentPrefix = '#';
} else if (isFishConfig(shellConfigFile)) {
// Fish shell syntax - use 'set -gx VAR value' (global export)
// Escape single quotes in fish by closing, escaping, reopening: 'foo'\''bar'
const escapedValue = value.replace(/'/g, "'\\''");
exportLine = `set -gx ${envVar} '${escapedValue}'`;
commentPrefix = '#';
} else {
// Unix shell syntax (bash, zsh, fish) - use single quotes and escape embedded single quotes
// Unix shell syntax (bash, zsh) - use single quotes and escape embedded single quotes
const escapedValue = value.replace(/'/g, "'\\''");
exportLine = `export ${envVar}='${escapedValue}'`;
commentPrefix = '#';