Skip to content

Commit e40b0a2

Browse files
committed
simplify internal structures.
1 parent 475961d commit e40b0a2

File tree

4 files changed

+175
-164
lines changed

4 files changed

+175
-164
lines changed

src/tools/system/shellMessage.test.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, it, expect, beforeEach, afterEach } from "vitest";
2-
import { shellStartTool } from "./shellStart.js";
2+
import { processStates, shellStartTool } from "./shellStart.js";
33
import { MockLogger } from "../../utils/mockLogger.js";
44
import { shellMessageTool } from "./shellMessage.js";
55

@@ -10,14 +10,14 @@ describe("shellMessageTool", () => {
1010
let testInstanceId = "";
1111

1212
beforeEach(() => {
13-
global.startedProcesses.clear();
13+
processStates.clear();
1414
});
1515

1616
afterEach(() => {
17-
for (const process of global.startedProcesses.values()) {
18-
process.kill();
17+
for (const processState of processStates.values()) {
18+
processState.process.kill();
1919
}
20-
global.startedProcesses.clear();
20+
processStates.clear();
2121
});
2222

2323
it("should interact with a running process", async () => {
@@ -82,8 +82,8 @@ describe("shellMessageTool", () => {
8282
);
8383

8484
expect(result.completed).toBe(true);
85-
// Process should still be in startedProcesses even after completion
86-
expect(global.startedProcesses.has(startResult.instanceId)).toBe(true);
85+
// Process should still be in processStates even after completion
86+
expect(processStates.has(startResult.instanceId)).toBe(true);
8787
});
8888

8989
it("should handle SIGTERM signal correctly", async () => {
@@ -179,10 +179,8 @@ describe("shellMessageTool", () => {
179179
{ logger: mockLogger }
180180
);
181181

182-
console.log({ checkResult });
183-
184182
expect(checkResult.signaled).toBe(true);
185183
expect(checkResult.completed).toBe(true);
186-
expect(global.startedProcesses.has(startResult.instanceId)).toBe(true);
184+
expect(processStates.has(startResult.instanceId)).toBe(true);
187185
});
188186
});

src/tools/system/shellMessage.ts

Lines changed: 92 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,56 +1,87 @@
11
import { Tool } from "../../core/types.js";
22
import { z } from "zod";
33
import { zodToJsonSchema } from "zod-to-json-schema";
4+
import { processStates } from "./shellStart.js";
45

56
// Define valid NodeJS signals as a union type
67
type NodeSignals =
7-
| 'SIGABRT'
8-
| 'SIGALRM'
9-
| 'SIGBUS'
10-
| 'SIGCHLD'
11-
| 'SIGCONT'
12-
| 'SIGFPE'
13-
| 'SIGHUP'
14-
| 'SIGILL'
15-
| 'SIGINT'
16-
| 'SIGIO'
17-
| 'SIGIOT'
18-
| 'SIGKILL'
19-
| 'SIGPIPE'
20-
| 'SIGPOLL'
21-
| 'SIGPROF'
22-
| 'SIGPWR'
23-
| 'SIGQUIT'
24-
| 'SIGSEGV'
25-
| 'SIGSTKFLT'
26-
| 'SIGSTOP'
27-
| 'SIGSYS'
28-
| 'SIGTERM'
29-
| 'SIGTRAP'
30-
| 'SIGTSTP'
31-
| 'SIGTTIN'
32-
| 'SIGTTOU'
33-
| 'SIGUNUSED'
34-
| 'SIGURG'
35-
| 'SIGUSR1'
36-
| 'SIGUSR2'
37-
| 'SIGVTALRM'
38-
| 'SIGWINCH'
39-
| 'SIGXCPU'
40-
| 'SIGXFSZ';
8+
| "SIGABRT"
9+
| "SIGALRM"
10+
| "SIGBUS"
11+
| "SIGCHLD"
12+
| "SIGCONT"
13+
| "SIGFPE"
14+
| "SIGHUP"
15+
| "SIGILL"
16+
| "SIGINT"
17+
| "SIGIO"
18+
| "SIGIOT"
19+
| "SIGKILL"
20+
| "SIGPIPE"
21+
| "SIGPOLL"
22+
| "SIGPROF"
23+
| "SIGPWR"
24+
| "SIGQUIT"
25+
| "SIGSEGV"
26+
| "SIGSTKFLT"
27+
| "SIGSTOP"
28+
| "SIGSYS"
29+
| "SIGTERM"
30+
| "SIGTRAP"
31+
| "SIGTSTP"
32+
| "SIGTTIN"
33+
| "SIGTTOU"
34+
| "SIGUNUSED"
35+
| "SIGURG"
36+
| "SIGUSR1"
37+
| "SIGUSR2"
38+
| "SIGVTALRM"
39+
| "SIGWINCH"
40+
| "SIGXCPU"
41+
| "SIGXFSZ";
4142

4243
const parameterSchema = z.object({
4344
instanceId: z.string().describe("The ID returned by shellStart"),
4445
stdin: z.string().optional().describe("Input to send to process"),
45-
signal: z.enum([
46-
'SIGABRT', 'SIGALRM', 'SIGBUS', 'SIGCHLD', 'SIGCONT',
47-
'SIGFPE', 'SIGHUP', 'SIGILL', 'SIGINT', 'SIGIO',
48-
'SIGIOT', 'SIGKILL', 'SIGPIPE', 'SIGPOLL', 'SIGPROF',
49-
'SIGPWR', 'SIGQUIT', 'SIGSEGV', 'SIGSTKFLT', 'SIGSTOP',
50-
'SIGSYS', 'SIGTERM', 'SIGTRAP', 'SIGTSTP', 'SIGTTIN',
51-
'SIGTTOU', 'SIGUNUSED', 'SIGURG', 'SIGUSR1', 'SIGUSR2',
52-
'SIGVTALRM', 'SIGWINCH', 'SIGXCPU', 'SIGXFSZ'
53-
] as const).optional().describe("Signal to send to the process (e.g., SIGTERM, SIGINT)"),
46+
signal: z
47+
.enum([
48+
"SIGABRT",
49+
"SIGALRM",
50+
"SIGBUS",
51+
"SIGCHLD",
52+
"SIGCONT",
53+
"SIGFPE",
54+
"SIGHUP",
55+
"SIGILL",
56+
"SIGINT",
57+
"SIGIO",
58+
"SIGIOT",
59+
"SIGKILL",
60+
"SIGPIPE",
61+
"SIGPOLL",
62+
"SIGPROF",
63+
"SIGPWR",
64+
"SIGQUIT",
65+
"SIGSEGV",
66+
"SIGSTKFLT",
67+
"SIGSTOP",
68+
"SIGSYS",
69+
"SIGTERM",
70+
"SIGTRAP",
71+
"SIGTSTP",
72+
"SIGTTIN",
73+
"SIGTTOU",
74+
"SIGUNUSED",
75+
"SIGURG",
76+
"SIGUSR1",
77+
"SIGUSR2",
78+
"SIGVTALRM",
79+
"SIGWINCH",
80+
"SIGXCPU",
81+
"SIGXFSZ",
82+
] as const)
83+
.optional()
84+
.describe("Signal to send to the process (e.g., SIGTERM, SIGINT)"),
5485
description: z
5586
.string()
5687
.max(80)
@@ -79,60 +110,53 @@ export const shellMessageTool: Tool<Parameters, ReturnType> = {
79110
parameters: zodToJsonSchema(parameterSchema),
80111
returns: zodToJsonSchema(returnSchema),
81112

82-
execute: async ({ instanceId, stdin, signal }, { logger }): Promise<ReturnType> => {
113+
execute: async (
114+
{ instanceId, stdin, signal },
115+
{ logger }
116+
): Promise<ReturnType> => {
83117
logger.verbose(
84118
`Interacting with shell process ${instanceId}${stdin ? " with input" : ""}${signal ? ` with signal ${signal}` : ""}`
85119
);
86120

87121
try {
88-
const process = global.startedProcesses.get(instanceId);
89-
if (!process) {
90-
throw new Error(`No process found with ID ${instanceId}`);
91-
}
92-
93-
const processOutput = global.processOutputs.get(instanceId);
94-
if (!processOutput) {
95-
throw new Error(`No output buffers found for process ${instanceId}`);
96-
}
97-
98-
const processState = global.processState.get(instanceId);
122+
const processState = processStates.get(instanceId);
99123
if (!processState) {
100-
throw new Error(`No state information found for process ${instanceId}`);
124+
throw new Error(`No process found with ID ${instanceId}`);
101125
}
102126

103127
// Send signal if provided
104128
if (signal) {
105-
const wasKilled = process.kill(signal);
129+
const wasKilled = processState.process.kill(signal);
106130
if (!wasKilled) {
107131
return {
108132
stdout: "",
109133
stderr: "",
110-
completed: processState.completed,
134+
completed: processState.state.completed,
111135
signaled: false,
112-
error: `Failed to send signal ${signal} to process (process may have already terminated)`
136+
error: `Failed to send signal ${signal} to process (process may have already terminated)`,
113137
};
114138
}
115-
processState.signaled = true;
139+
processState.state.signaled = true;
116140
}
117141

118142
// Send input if provided
119143
if (stdin) {
120-
if (!process.stdin?.writable) {
144+
if (!processState.process.stdin?.writable) {
121145
throw new Error("Process stdin is not available");
122146
}
123-
process.stdin.write(`${stdin}\n`);
147+
processState.process.stdin.write(`${stdin}\n`);
124148
}
125149

126150
// Wait a brief moment for output to be processed
127151
await new Promise((resolve) => setTimeout(resolve, 100));
128152

129153
// Get accumulated output
130-
const stdout = processOutput.stdout.join("");
131-
const stderr = processOutput.stderr.join("");
154+
const stdout = processState.stdout.join("");
155+
const stderr = processState.stderr.join("");
132156

133157
// Clear the buffers
134-
processOutput.stdout = [];
135-
processOutput.stderr = [];
158+
processState.stdout = [];
159+
processState.stderr = [];
136160

137161
logger.verbose("Interaction completed successfully");
138162
if (stdout) {
@@ -145,8 +169,8 @@ export const shellMessageTool: Tool<Parameters, ReturnType> = {
145169
return {
146170
stdout: stdout.trim(),
147171
stderr: stderr.trim(),
148-
completed: processState.completed,
149-
signaled: processState.signaled,
172+
completed: processState.state.completed,
173+
signaled: processState.state.signaled,
150174
};
151175
} catch (error) {
152176
if (error instanceof Error) {
Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,26 @@
1-
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
2-
import { shellStartTool } from './shellStart.js';
3-
import { MockLogger } from '../../utils/mockLogger.js';
1+
import { describe, it, expect, beforeEach, afterEach } from "vitest";
2+
import { processStates, shellStartTool } from "./shellStart.js";
3+
import { MockLogger } from "../../utils/mockLogger.js";
44

5-
describe('shellStartTool', () => {
5+
describe("shellStartTool", () => {
66
const mockLogger = new MockLogger();
77

88
beforeEach(() => {
9-
global.startedProcesses.clear();
9+
processStates.clear();
1010
});
1111

1212
afterEach(() => {
13-
for (const process of global.startedProcesses.values()) {
14-
process.kill();
13+
for (const processState of processStates.values()) {
14+
processState.process.kill();
1515
}
16-
global.startedProcesses.clear();
16+
processStates.clear();
1717
});
1818

19-
it('should start a process and return instance ID', async () => {
19+
it("should start a process and return instance ID", async () => {
2020
const result = await shellStartTool.execute(
2121
{
2222
command: 'echo "test"',
23-
description: 'Test process',
23+
description: "Test process",
2424
},
2525
{ logger: mockLogger }
2626
);
@@ -29,65 +29,65 @@ describe('shellStartTool', () => {
2929
expect(result.error).toBeUndefined();
3030
});
3131

32-
it('should handle invalid commands', async () => {
32+
it("should handle invalid commands", async () => {
3333
const result = await shellStartTool.execute(
3434
{
35-
command: 'nonexistentcommand',
36-
description: 'Invalid command test',
35+
command: "nonexistentcommand",
36+
description: "Invalid command test",
3737
},
3838
{ logger: mockLogger }
3939
);
4040

4141
expect(result.error).toBeDefined();
4242
});
4343

44-
it('should keep process in startedProcesses after completion', async () => {
44+
it("should keep process in processStates after completion", async () => {
4545
const result = await shellStartTool.execute(
4646
{
4747
command: 'echo "test"',
48-
description: 'Completion test',
48+
description: "Completion test",
4949
},
5050
{ logger: mockLogger }
5151
);
5252

5353
// Wait for process to complete
54-
await new Promise(resolve => setTimeout(resolve, 100));
54+
await new Promise((resolve) => setTimeout(resolve, 100));
5555

56-
// Process should still be in startedProcesses
57-
expect(global.startedProcesses.has(result.instanceId)).toBe(true);
56+
// Process should still be in processStates
57+
expect(processStates.has(result.instanceId)).toBe(true);
5858
});
5959

60-
it('should handle piped commands correctly', async () => {
60+
it("should handle piped commands correctly", async () => {
6161
// Start a process that uses pipes
6262
const result = await shellStartTool.execute(
6363
{
64-
command: 'grep "test"', // Just grep waiting for stdin
65-
description: 'Pipe test',
64+
command: 'grep "test"', // Just grep waiting for stdin
65+
description: "Pipe test",
6666
},
6767
{ logger: mockLogger }
6868
);
6969

7070
expect(result.instanceId).toBeDefined();
7171
expect(result.error).toBeUndefined();
7272

73-
// Process should be in startedProcesses
74-
expect(global.startedProcesses.has(result.instanceId)).toBe(true);
73+
// Process should be in processStates
74+
expect(processStates.has(result.instanceId)).toBe(true);
7575

7676
// Get the process
77-
const process = global.startedProcesses.get(result.instanceId);
78-
expect(process).toBeDefined();
79-
77+
const processState = processStates.get(result.instanceId);
78+
expect(processState).toBeDefined();
79+
8080
// Write to stdin and check output
81-
if (process?.stdin) {
82-
process.stdin.write('this is a test line\n');
83-
process.stdin.write('not matching line\n');
84-
process.stdin.write('another test here\n');
85-
81+
if (processState?.process.stdin) {
82+
processState.process.stdin.write("this is a test line\n");
83+
processState.process.stdin.write("not matching line\n");
84+
processState.process.stdin.write("another test here\n");
85+
8686
// Wait for output
87-
await new Promise(resolve => setTimeout(resolve, 200));
88-
87+
await new Promise((resolve) => setTimeout(resolve, 200));
88+
8989
// Process should have filtered only lines with "test"
9090
// This part might need adjustment based on how output is captured
9191
}
9292
});
93-
});
93+
});

0 commit comments

Comments
 (0)