Skip to content

Commit 239cb4e

Browse files
committed
fix(cli): harden install error handling and state updates
1 parent 6327d5d commit 239cb4e

File tree

6 files changed

+237
-8
lines changed

6 files changed

+237
-8
lines changed

docs/ai/testing/feature-install-command.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,19 @@ feature: install-command
4040
- [x] `ConfigManager.addSkill` skips duplicates.
4141
- [x] `SkillManager.addSkill` persists skill metadata (`registry`, `name`) to config.
4242

43+
### Install Orchestration (`services/install/install.service.ts`)
44+
45+
- [x] Installs environments/phases/skills in happy path.
46+
- [x] Prompts once and skips conflicting artifacts when overwrite is declined.
47+
- [x] Overwrites conflicting artifacts when prompted confirmation is accepted.
48+
- [x] Auto-overwrites without prompt when `--overwrite` is set.
49+
- [x] Skill failures are collected as warnings and do not change exit code.
50+
- [x] Exit code is non-zero when environment/phase failures occur.
51+
4352
## Integration Tests
4453

4554
- [x] Command-level flow covered via `install` command tests with mocked orchestrator.
55+
- [x] Service-level integration flow covered with mocked dependencies for overwrite and partial-failure branches.
4656
- [ ] Real filesystem integration test for `ai-devkit install` happy path.
4757
- [ ] Real filesystem integration test for `--overwrite` confirmation flow.
4858
- [ ] Real skill install partial-failure integration with network errors.
@@ -53,6 +63,7 @@ Executed on February 23, 2026:
5363

5464
```bash
5565
npm run test -- --runInBand \
66+
src/__tests__/services/install/install.service.test.ts \
5667
src/__tests__/util/config.test.ts \
5768
src/__tests__/commands/install.test.ts \
5869
src/__tests__/services/config/config.service.test.ts \
@@ -62,6 +73,17 @@ npm run test -- --runInBand \
6273
```
6374

6475
Result: targeted suites pass locally (command/config/config-service/config-manager/init/skill-manager coverage).
76+
Additional focused run:
77+
78+
```bash
79+
npm run test -- --runInBand \
80+
src/__tests__/services/install/install.service.test.ts \
81+
src/__tests__/commands/install.test.ts \
82+
src/__tests__/util/config.test.ts \
83+
src/__tests__/services/config/config.service.test.ts
84+
```
85+
86+
Result: `4 passed, 4 total` suites and `16 passed, 16 total` tests.
6587

6688
## Manual Testing
6789

packages/cli/src/__tests__/commands/init.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ describe('init command template mode', () => {
111111
mockLoadInitTemplate.mockResolvedValue({});
112112
});
113113

114+
afterEach(() => {
115+
process.exitCode = undefined;
116+
});
117+
114118
it('uses template values and installs multiple skills from same registry without prompts', async () => {
115119
mockLoadInitTemplate.mockResolvedValue({
116120
environments: ['codex'],

packages/cli/src/__tests__/commands/install.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ describe('install command', () => {
4747
});
4848
});
4949

50+
afterEach(() => {
51+
process.exitCode = undefined;
52+
});
53+
5054
it('fails with non-zero exit code when config loading fails', async () => {
5155
mockLoadConfigFile.mockRejectedValue(new Error('Config file not found: /tmp/.ai-devkit.json'));
5256

@@ -99,4 +103,22 @@ describe('install command', () => {
99103
expect(mockUi.summary).toHaveBeenCalled();
100104
expect(process.exitCode).toBe(0);
101105
});
106+
107+
it('fails with non-zero exit code when reconcile step throws', async () => {
108+
mockLoadConfigFile.mockResolvedValue({
109+
configPath: '/tmp/.ai-devkit.json',
110+
data: {}
111+
});
112+
mockLoadAndValidateInstallConfig.mockReturnValue({
113+
environments: ['codex'],
114+
phases: ['requirements'],
115+
skills: []
116+
});
117+
mockReconcileAndInstall.mockRejectedValue(new Error('install failed'));
118+
119+
await installCommand({});
120+
121+
expect(mockUi.error).toHaveBeenCalledWith('install failed');
122+
expect(process.exitCode).toBe(1);
123+
});
102124
});
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import { jest } from '@jest/globals';
2+
3+
const mockPrompt: any = jest.fn();
4+
5+
const mockConfigManager: any = {
6+
read: jest.fn(),
7+
create: jest.fn(),
8+
update: jest.fn(),
9+
addPhase: jest.fn()
10+
};
11+
12+
const mockTemplateManager: any = {
13+
checkEnvironmentExists: jest.fn(),
14+
fileExists: jest.fn(),
15+
setupMultipleEnvironments: jest.fn(),
16+
copyPhaseTemplate: jest.fn()
17+
};
18+
19+
const mockSkillManager: any = {
20+
addSkill: jest.fn()
21+
};
22+
23+
jest.mock('inquirer', () => ({
24+
__esModule: true,
25+
default: {
26+
prompt: (...args: unknown[]) => mockPrompt(...args)
27+
}
28+
}));
29+
30+
jest.mock('../../../lib/Config', () => ({
31+
ConfigManager: jest.fn(() => mockConfigManager)
32+
}));
33+
34+
jest.mock('../../../lib/TemplateManager', () => ({
35+
TemplateManager: jest.fn(() => mockTemplateManager)
36+
}));
37+
38+
jest.mock('../../../lib/EnvironmentSelector', () => ({
39+
EnvironmentSelector: jest.fn()
40+
}));
41+
42+
jest.mock('../../../lib/SkillManager', () => ({
43+
SkillManager: jest.fn(() => mockSkillManager)
44+
}));
45+
46+
import { getInstallExitCode, reconcileAndInstall } from '../../../services/install/install.service';
47+
48+
describe('install service', () => {
49+
const installConfig = {
50+
environments: ['codex' as const],
51+
phases: ['requirements' as const],
52+
skills: [{ registry: 'codeaholicguy/ai-devkit', name: 'debug' }]
53+
};
54+
55+
beforeEach(() => {
56+
jest.clearAllMocks();
57+
58+
mockConfigManager.read.mockResolvedValue({
59+
environments: [],
60+
phases: []
61+
});
62+
mockConfigManager.create.mockResolvedValue({
63+
environments: [],
64+
phases: []
65+
});
66+
mockConfigManager.update.mockResolvedValue({});
67+
mockConfigManager.addPhase.mockResolvedValue({});
68+
69+
mockTemplateManager.checkEnvironmentExists.mockResolvedValue(false);
70+
mockTemplateManager.fileExists.mockResolvedValue(false);
71+
mockTemplateManager.setupMultipleEnvironments.mockResolvedValue([]);
72+
mockTemplateManager.copyPhaseTemplate.mockResolvedValue('docs/ai/requirements/README.md');
73+
74+
mockSkillManager.addSkill.mockResolvedValue(undefined);
75+
mockPrompt.mockResolvedValue({ overwrite: false });
76+
});
77+
78+
it('installs all sections on happy path', async () => {
79+
const report = await reconcileAndInstall(installConfig, {});
80+
81+
expect(mockConfigManager.update).toHaveBeenCalledWith({
82+
environments: ['codex'],
83+
phases: ['requirements'],
84+
skills: [{ registry: 'codeaholicguy/ai-devkit', name: 'debug' }]
85+
});
86+
expect(report.environments.installed).toBe(1);
87+
expect(report.phases.installed).toBe(1);
88+
expect(report.skills.installed).toBe(1);
89+
expect(report.warnings).toEqual([]);
90+
});
91+
92+
it('prompts and skips conflicting artifacts when overwrite is not confirmed', async () => {
93+
mockTemplateManager.checkEnvironmentExists
94+
.mockResolvedValueOnce(true)
95+
.mockResolvedValueOnce(true);
96+
mockTemplateManager.fileExists
97+
.mockResolvedValueOnce(true)
98+
.mockResolvedValueOnce(true);
99+
mockPrompt.mockResolvedValue({ overwrite: false });
100+
101+
const report = await reconcileAndInstall(installConfig, {});
102+
103+
expect(mockPrompt).toHaveBeenCalledTimes(1);
104+
expect(report.environments.skipped).toBe(1);
105+
expect(report.phases.skipped).toBe(1);
106+
expect(report.skills.installed).toBe(1);
107+
expect(mockConfigManager.update).toHaveBeenCalledWith({
108+
environments: [],
109+
phases: [],
110+
skills: [{ registry: 'codeaholicguy/ai-devkit', name: 'debug' }]
111+
});
112+
});
113+
114+
it('overwrites conflicting artifacts when overwrite is confirmed via prompt', async () => {
115+
mockTemplateManager.checkEnvironmentExists
116+
.mockResolvedValueOnce(true)
117+
.mockResolvedValueOnce(true);
118+
mockTemplateManager.fileExists
119+
.mockResolvedValueOnce(true)
120+
.mockResolvedValueOnce(true);
121+
mockPrompt.mockResolvedValue({ overwrite: true });
122+
123+
const report = await reconcileAndInstall(installConfig, {});
124+
125+
expect(mockPrompt).toHaveBeenCalledTimes(1);
126+
expect(report.environments.installed).toBe(1);
127+
expect(report.phases.installed).toBe(1);
128+
});
129+
130+
it('auto-overwrites and does not prompt when --overwrite is set', async () => {
131+
mockTemplateManager.checkEnvironmentExists
132+
.mockResolvedValueOnce(true)
133+
.mockResolvedValueOnce(true);
134+
mockTemplateManager.fileExists
135+
.mockResolvedValueOnce(true)
136+
.mockResolvedValueOnce(true);
137+
138+
const report = await reconcileAndInstall(installConfig, { overwrite: true });
139+
140+
expect(mockPrompt).not.toHaveBeenCalled();
141+
expect(report.environments.installed).toBe(1);
142+
expect(report.phases.installed).toBe(1);
143+
});
144+
145+
it('reports skill failures as warnings and continues', async () => {
146+
mockSkillManager.addSkill.mockRejectedValue(new Error('network down'));
147+
148+
const report = await reconcileAndInstall(installConfig, {});
149+
150+
expect(report.skills.failed).toBe(1);
151+
expect(report.warnings).toEqual([
152+
'Skill codeaholicguy/ai-devkit/debug failed: network down'
153+
]);
154+
expect(getInstallExitCode(report)).toBe(0);
155+
});
156+
157+
it('returns non-zero exit code when environment or phase failures occur', () => {
158+
const report = {
159+
environments: { installed: 0, skipped: 0, failed: 1 },
160+
phases: { installed: 0, skipped: 0, failed: 0 },
161+
skills: { installed: 0, skipped: 0, failed: 0 },
162+
warnings: []
163+
};
164+
165+
expect(getInstallExitCode(report)).toBe(1);
166+
});
167+
});

packages/cli/src/commands/install.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,16 @@ export async function installCommand(options: InstallCommandOptions): Promise<vo
4343
return;
4444
}
4545

46-
const report = await reconcileAndInstall(validatedConfig, {
47-
overwrite: options.overwrite
48-
});
46+
let report;
47+
try {
48+
report = await reconcileAndInstall(validatedConfig, {
49+
overwrite: options.overwrite
50+
});
51+
} catch (error) {
52+
ui.error(error instanceof Error ? error.message : String(error));
53+
process.exitCode = 1;
54+
return;
55+
}
4956

5057
ui.summary({
5158
title: 'Install Summary',

packages/cli/src/services/install/install.service.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,9 @@ export async function reconcileAndInstall(
5050
throw new Error('Failed to initialize project config for install command.');
5151
}
5252

53-
await configManager.update({
54-
environments: config.environments,
55-
phases: config.phases,
56-
skills: config.skills
57-
});
53+
const successfulEnvironments: typeof config.environments = [];
54+
const successfulPhases: typeof config.phases = [];
55+
const successfulSkills: typeof config.skills = [];
5856

5957
for (const envCode of config.environments) {
6058
try {
@@ -66,6 +64,7 @@ export async function reconcileAndInstall(
6664

6765
await templateManager.setupMultipleEnvironments([envCode]);
6866
report.environments.installed += 1;
67+
successfulEnvironments.push(envCode);
6968
} catch (error) {
7069
report.environments.failed += 1;
7170
report.warnings.push(
@@ -85,6 +84,7 @@ export async function reconcileAndInstall(
8584
await templateManager.copyPhaseTemplate(phase);
8685
await configManager.addPhase(phase);
8786
report.phases.installed += 1;
87+
successfulPhases.push(phase);
8888
} catch (error) {
8989
report.phases.failed += 1;
9090
report.warnings.push(
@@ -97,6 +97,7 @@ export async function reconcileAndInstall(
9797
try {
9898
await skillManager.addSkill(skill.registry, skill.name);
9999
report.skills.installed += 1;
100+
successfulSkills.push(skill);
100101
} catch (error) {
101102
report.skills.failed += 1;
102103
report.warnings.push(
@@ -105,6 +106,12 @@ export async function reconcileAndInstall(
105106
}
106107
}
107108

109+
await configManager.update({
110+
environments: successfulEnvironments,
111+
phases: successfulPhases,
112+
skills: successfulSkills
113+
});
114+
108115
return report;
109116
}
110117

0 commit comments

Comments
 (0)