Skip to content

Commit b5ecbb5

Browse files
remarkablemarkremarkablemark
authored
Merge pull request #15 from ai-action/refactor/components
refactor(components): standardize approval selects and decision handling
2 parents 444a362 + 68b4e9e commit b5ecbb5

12 files changed

Lines changed: 249 additions & 238 deletions

‎src/components/App.test.tsx‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { Text } from 'ink';
22
import { render } from 'ink-testing-library';
33

4-
import { tick } from '../utils/test';
4+
import { test } from '../utils';
55

6-
vi.mock('../utils', () => ({
6+
vi.mock('../utils', async () => ({
7+
...(await vi.importActual('../utils')),
78
config: {
89
loadConfig: vi.fn(() => ({
910
host: 'http://localhost:11434',
@@ -97,18 +98,18 @@ describe('App', () => {
9798
const { lastFrame, rerender } = render(<App />);
9899
capturedCallbacks.onCommand?.('/model');
99100
rerender(<App />);
100-
await tick();
101+
await test.tick();
101102
expect(lastFrame()).toContain('ModelPicker');
102103
});
103104

104105
it('returns to chat and updates model when onSelect is called', async () => {
105106
const { lastFrame, rerender } = render(<App />);
106107
capturedCallbacks.onCommand?.('/model');
107108
rerender(<App />);
108-
await tick();
109+
await test.tick();
109110
capturedCallbacks.onSelect?.('llama3');
110111
rerender(<App />);
111-
await tick();
112+
await test.tick();
112113
expect(lastFrame()).toContain('llama3');
113114
expect(lastFrame()).not.toContain('ModelPicker');
114115
});
@@ -117,10 +118,10 @@ describe('App', () => {
117118
const { lastFrame, rerender } = render(<App />);
118119
capturedCallbacks.onCommand?.('/model');
119120
rerender(<App />);
120-
await tick();
121+
await test.tick();
121122
capturedCallbacks.onCancel?.();
122123
rerender(<App />);
123-
await tick();
124+
await test.tick();
124125
expect(lastFrame()).not.toContain('ModelPicker');
125126
expect(lastFrame()).toContain('>');
126127
});
@@ -129,7 +130,7 @@ describe('App', () => {
129130
const { lastFrame, rerender } = render(<App />);
130131
capturedCallbacks.onCommand?.('/unknown');
131132
rerender(<App />);
132-
await tick();
133+
await test.tick();
133134
expect(lastFrame()).not.toContain('ModelPicker');
134135
});
135136

@@ -142,19 +143,19 @@ describe('App', () => {
142143
// Call the callback passed to Footer - cycles to Auto
143144
capturedCallbacks.onToggleMode?.();
144145
rerender(<App />);
145-
await tick();
146+
await test.tick();
146147
expect(lastFrame()).toContain('Mode: Auto');
147148

148149
// Call again - cycles to Plan
149150
capturedCallbacks.onToggleMode?.();
150151
rerender(<App />);
151-
await tick();
152+
await test.tick();
152153
expect(lastFrame()).toContain('Mode: Plan');
153154

154155
// Call again - cycles back to Safe
155156
capturedCallbacks.onToggleMode?.();
156157
rerender(<App />);
157-
await tick();
158+
await test.tick();
158159
expect(lastFrame()).toContain('Mode: Safe');
159160
});
160161

@@ -165,12 +166,12 @@ describe('App', () => {
165166

166167
capturedCallbacks.onModeChange?.('auto');
167168
rerender(<App />);
168-
await tick();
169+
await test.tick();
169170
expect(lastFrame()).toContain('Mode: Auto');
170171

171172
capturedCallbacks.onModeChange?.('safe');
172173
rerender(<App />);
173-
await tick();
174+
await test.tick();
174175
expect(lastFrame()).toContain('Mode: Safe');
175176
});
176177
});

‎src/components/Chat/Chat.test.tsx‎

Lines changed: 66 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
11
import { Text } from 'ink';
22
import { render } from 'ink-testing-library';
33

4-
import { MODE } from '../../constants';
5-
import { ollama, tools } from '../../utils';
6-
import { tick } from '../../utils/test';
4+
import { DECISION, MODE } from '../../constants';
5+
import { ollama, test, tools } from '../../utils';
76

87
const mockState = vi.hoisted(() => ({
9-
handlers: [] as ((value: string) => void)[],
8+
handler: undefined as ((value: string) => void) | undefined,
109
testInput: '',
1110
shouldReset: false,
1211
clear() {
13-
this.handlers.length = 0;
12+
this.handler = undefined;
1413
this.testInput = '';
1514
this.shouldReset = true;
1615
},
@@ -23,19 +22,35 @@ const planApprovalState = vi.hoisted(() => ({
2322
},
2423
}));
2524

25+
const toolApprovalState = vi.hoisted(() => ({
26+
onChange: undefined as ((value: DECISION.Decision) => void) | undefined,
27+
clear() {
28+
this.onChange = undefined;
29+
},
30+
}));
31+
2632
vi.mock('@inkjs/ui', async () => {
27-
const actual = await vi.importActual<typeof import('@inkjs/ui')>('@inkjs/ui');
33+
const actual = await vi.importActual('@inkjs/ui');
2834
const { Text } = await import('ink');
2935
return {
3036
...actual,
3137
Select: ({
3238
options,
3339
onChange,
3440
}: {
35-
options: { label: string; value: MODE.Name }[];
36-
onChange?: (value: MODE.Name) => void;
41+
options: { label: string; value: string }[];
42+
onChange?: (value: string) => void;
3743
}) => {
38-
planApprovalState.onChange = onChange;
44+
const isPlanApproval = options.some(({ value }) =>
45+
Object.values(MODE.NAME).includes(value as MODE.Name),
46+
);
47+
48+
if (isPlanApproval) {
49+
planApprovalState.onChange = onChange;
50+
} else {
51+
toolApprovalState.onChange = onChange;
52+
}
53+
3954
return (
4055
<>
4156
{options.map(({ value, label }) => (
@@ -47,33 +62,29 @@ vi.mock('@inkjs/ui', async () => {
4762
};
4863
});
4964

50-
vi.mock('../../utils', async () => {
51-
const actual =
52-
await vi.importActual<typeof import('../../utils')>('../../utils');
53-
return {
54-
...actual,
55-
ollama: {
56-
streamChat: vi.fn().mockImplementation(function* () {
57-
yield { type: 'content', content: 'Mocked' };
58-
yield { type: 'content', content: ' response' };
59-
}),
60-
},
61-
tools: {
62-
TOOLS: [],
63-
READ_ONLY_TOOLS: new Set(),
64-
DANGEROUS_TOOLS: new Set(),
65-
executeTool: vi.fn(),
66-
},
67-
};
68-
});
65+
vi.mock('../../utils', async () => ({
66+
...(await vi.importActual('../../utils')),
67+
ollama: {
68+
streamChat: vi.fn().mockImplementation(function* () {
69+
yield { type: 'content', content: 'Mocked' };
70+
yield { type: 'content', content: ' response' };
71+
}),
72+
},
73+
tools: {
74+
TOOLS: [],
75+
READ_ONLY_TOOLS: new Set(),
76+
DANGEROUS_TOOLS: new Set(),
77+
executeTool: vi.fn(),
78+
},
79+
}));
6980

7081
vi.mock('./Input', () => ({
7182
Input: (props: {
7283
onSubmit?: (value: string) => void;
7384
isDisabled?: boolean;
7485
}) => {
7586
if (props.onSubmit) {
76-
mockState.handlers.push(props.onSubmit);
87+
mockState.handler = props.onSubmit;
7788
}
7889

7990
if (props.isDisabled) {
@@ -102,30 +113,33 @@ async function typeText(
102113
) {
103114
mockState.testInput = text;
104115
rerender(tree);
105-
await tick();
116+
await test.tick();
106117
}
107118

108119
function submitInput(value: string) {
109-
for (const handler of mockState.handlers) {
110-
handler(value);
111-
}
120+
mockState.handler?.(value);
112121
mockState.clear();
113122
}
114123

115124
function choosePlanMode(mode: MODE.Name) {
116125
planApprovalState.onChange?.(mode);
117126
}
118127

128+
function chooseToolDecision(decision: DECISION.Decision) {
129+
toolApprovalState.onChange?.(decision);
130+
}
131+
119132
async function waitForStream() {
120133
// Allow time for async generator to yield values
121-
await tick(10);
134+
await test.tick(10);
122135
}
123136

124137
function resetChatMocks() {
125138
vi.restoreAllMocks();
126139
vi.clearAllMocks();
127140
mockState.clear();
128141
planApprovalState.clear();
142+
toolApprovalState.clear();
129143
tools.TOOLS.splice(0, tools.TOOLS.length);
130144
vi.mocked(ollama.streamChat).mockImplementation(async function* () {
131145
await Promise.resolve();
@@ -151,7 +165,7 @@ describe('Chat', () => {
151165
onModeChange={onModeChange}
152166
/>,
153167
);
154-
await tick();
168+
await test.tick();
155169
const frame = lastFrame() ?? '';
156170
expect(frame).not.toContain('coding assistant');
157171
expect(frame).toContain('>');
@@ -167,7 +181,7 @@ describe('Chat', () => {
167181
/>
168182
);
169183
const { lastFrame, rerender } = render(chat);
170-
await tick();
184+
await test.tick();
171185
await typeText(rerender, 'hello', chat);
172186
submitInput('hello');
173187
rerender(chat);
@@ -186,7 +200,7 @@ describe('Chat', () => {
186200
/>
187201
);
188202
const { lastFrame, rerender } = render(chat);
189-
await tick();
203+
await test.tick();
190204
await typeText(rerender, 'hello', chat);
191205
submitInput('hello');
192206
rerender(chat);
@@ -206,13 +220,13 @@ describe('Chat', () => {
206220
/>
207221
);
208222
const { lastFrame, rerender } = render(chat);
209-
await tick();
223+
await test.tick();
210224
const beforeFrame = lastFrame() ?? '';
211225
const systemLineCount = beforeFrame.split('\n').length;
212226
await typeText(rerender, ' ', chat);
213227
submitInput(' ');
214228
rerender(chat);
215-
await tick();
229+
await test.tick();
216230
const afterFrame = lastFrame() ?? '';
217231
const afterLineCount = afterFrame.split('\n').length;
218232
// After submitting blank input, line count should not increase
@@ -231,7 +245,7 @@ describe('Chat', () => {
231245
/>
232246
);
233247
const { lastFrame, rerender } = render(chat);
234-
await tick();
248+
await test.tick();
235249
await typeText(rerender, 'first', chat);
236250
submitInput('first');
237251
rerender(chat);
@@ -260,7 +274,7 @@ describe('Chat', () => {
260274
const { rerender } = render(chat);
261275
submitInput('/model');
262276
rerender(chat);
263-
await tick();
277+
await test.tick();
264278
expect(onCommand).toHaveBeenCalledWith('/model');
265279
});
266280

@@ -715,7 +729,7 @@ describe('Chat with tool calls', () => {
715729
expect(lastFrame()).toContain('Plan Generated');
716730

717731
choosePlanMode(MODE.NAME.PLAN);
718-
await tick();
732+
await test.tick();
719733
rerender(chat);
720734

721735
expect(onModeChange).toHaveBeenCalledWith(MODE.NAME.PLAN);
@@ -724,7 +738,7 @@ describe('Chat with tool calls', () => {
724738
);
725739

726740
choosePlanMode(MODE.NAME.AUTO);
727-
await tick();
741+
await test.tick();
728742
});
729743

730744
it('executes an approved plan immediately in auto mode', async () => {
@@ -867,7 +881,7 @@ describe('Chat with tool calls', () => {
867881
onModeChange={vi.fn()}
868882
/>
869883
);
870-
const { lastFrame, rerender, stdin } = render(chat);
884+
const { lastFrame, rerender } = render(chat);
871885

872886
await typeText(rerender, 'write a file', chat);
873887
submitInput('write a file');
@@ -878,11 +892,8 @@ describe('Chat with tool calls', () => {
878892
// Verify approval prompt is shown
879893
expect(lastFrame()).toContain('Tool requires approval');
880894

881-
// Reject the tool (move to No with right arrow, then Enter)
882-
stdin.write('\x1B[C'); // Right arrow
883-
await tick();
884-
stdin.write('\r'); // Enter
885-
await tick();
895+
chooseToolDecision(DECISION.REJECT);
896+
await waitForStream();
886897
rerender(chat);
887898

888899
// Should show rejection message
@@ -928,7 +939,7 @@ describe('Chat with tool calls', () => {
928939
onModeChange={vi.fn()}
929940
/>
930941
);
931-
const { lastFrame, rerender, stdin } = render(chat);
942+
const { lastFrame, rerender } = render(chat);
932943

933944
await typeText(rerender, 'write a file', chat);
934945
submitInput('write a file');
@@ -939,9 +950,8 @@ describe('Chat with tool calls', () => {
939950
// Verify approval prompt is shown
940951
expect(lastFrame()).toContain('Tool requires approval');
941952

942-
// Approve the tool by pressing Enter (yes is default)
943-
stdin.write('\r'); // Enter
944-
await tick();
953+
chooseToolDecision(DECISION.APPROVE);
954+
await waitForStream();
945955
rerender(chat);
946956

947957
// Should have called executeTool
@@ -991,17 +1001,16 @@ describe('Chat with tool calls', () => {
9911001
onModeChange={vi.fn()}
9921002
/>
9931003
);
994-
const { rerender, stdin } = render(chat);
1004+
const { rerender } = render(chat);
9951005

9961006
await typeText(rerender, 'write a file', chat);
9971007
submitInput('write a file');
9981008
rerender(chat);
9991009
await waitForStream();
10001010
rerender(chat);
10011011

1002-
// Approve the tool by pressing Enter
1003-
stdin.write('\r');
1004-
await tick();
1012+
chooseToolDecision(DECISION.APPROVE);
1013+
await waitForStream();
10051014
rerender(chat);
10061015

10071016
// Should have called executeTool

0 commit comments

Comments
 (0)