Skip to content

Commit 019ef57

Browse files
author
Eric Oliver
committed
fix knip which found a defect
1 parent 49fef2f commit 019ef57

File tree

4 files changed

+266
-20
lines changed

4 files changed

+266
-20
lines changed

‎knip.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
"cli/index.ts",
1212
"cli/cli-entry.ts",
1313
"cli/simple-index.ts",
14+
"api/api-entry.ts",
1415
"workers/countTokens.ts"
1516
],
1617
"project": [

‎src/core/adapters/api/ApiTerminal.ts‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,11 +332,35 @@ export class ApiTerminal implements ITerminal {
332332
// Limit to 50 processes
333333
const line = lines[i]
334334
if (line.trim() && (!filter || line.toLowerCase().includes(filter.toLowerCase()))) {
335-
processes.push({
336-
pid: i, // Simplified - would need proper parsing
337-
name: line.split(/\s+/)[0] || "unknown",
338-
cmd: line.trim(),
339-
})
335+
let pid: number
336+
let name: string
337+
338+
if (process.platform === "win32") {
339+
// Windows tasklist CSV format: "Image Name","PID","Session Name","Session#","Mem Usage"
340+
const csvMatch = line.match(/^"([^"]+)","(\d+)",/)
341+
if (csvMatch) {
342+
name = csvMatch[1]
343+
pid = parseInt(csvMatch[2], 10)
344+
} else {
345+
// Fallback: try splitting by comma and removing quotes
346+
const columns = line.split(",").map((col) => col.replace(/"/g, "").trim())
347+
name = columns[0] || "unknown"
348+
pid = parseInt(columns[1], 10)
349+
}
350+
} else {
351+
// Unix/Linux/Mac ps aux format: USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
352+
const columns = line.split(/\s+/)
353+
pid = parseInt(columns[1], 10) // PID is second column (columns[1])
354+
name = columns[0] || "unknown" // USER is first column
355+
}
356+
357+
if (!isNaN(pid)) {
358+
processes.push({
359+
pid: pid, // Properly parsed process ID
360+
name: name,
361+
cmd: line.trim(),
362+
})
363+
}
340364
}
341365
}
342366
}

‎src/core/adapters/api/__tests__/ApiTerminal.test.ts‎

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
import { ApiTerminal } from "../ApiTerminal"
2+
import { CommandResult, ExecuteCommandOptions } from "../../../interfaces/ITerminal"
3+
4+
// Mock the executeCommand method
5+
class TestApiTerminal extends ApiTerminal {
6+
private mockResult: CommandResult = {
7+
exitCode: 0,
8+
stdout: "",
9+
stderr: "",
10+
success: true,
11+
command: "",
12+
executionTime: 0,
13+
}
14+
15+
setMockResult(result: Partial<CommandResult>) {
16+
this.mockResult = { ...this.mockResult, ...result }
17+
}
18+
19+
override async executeCommand(command: string, options?: ExecuteCommandOptions): Promise<CommandResult> {
20+
return { ...this.mockResult, command }
21+
}
22+
}
23+
24+
describe("ApiTerminal", () => {
25+
let terminal: TestApiTerminal
26+
27+
beforeEach(() => {
28+
terminal = new TestApiTerminal()
29+
})
30+
31+
describe("getProcesses", () => {
32+
it("should parse Unix/Linux ps aux output correctly", async () => {
33+
const mockOutput = `USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
34+
root 1 0.0 0.1 77616 8604 ? Ss Dec07 0:01 /sbin/init
35+
root 2 0.0 0.0 0 0 ? S Dec07 0:00 [kthreadd]
36+
user 1234 1.2 2.3 123456 7890 pts/0 R+ 10:30 0:05 node server.js
37+
user 5678 0.5 1.1 98765 4321 pts/1 S 10:25 0:02 vim file.txt`
38+
39+
terminal.setMockResult({
40+
success: true,
41+
stdout: mockOutput,
42+
exitCode: 0,
43+
})
44+
45+
// Mock process.platform for Unix-like systems
46+
const originalPlatform = process.platform
47+
Object.defineProperty(process, "platform", { value: "linux" })
48+
49+
const processes = await terminal.getProcesses()
50+
51+
// Restore original platform
52+
Object.defineProperty(process, "platform", { value: originalPlatform })
53+
54+
expect(processes).toHaveLength(4)
55+
expect(processes[0]).toEqual({
56+
pid: 1,
57+
name: "root",
58+
cmd: "root 1 0.0 0.1 77616 8604 ? Ss Dec07 0:01 /sbin/init",
59+
})
60+
expect(processes[1]).toEqual({
61+
pid: 2,
62+
name: "root",
63+
cmd: "root 2 0.0 0.0 0 0 ? S Dec07 0:00 [kthreadd]",
64+
})
65+
expect(processes[2]).toEqual({
66+
pid: 1234,
67+
name: "user",
68+
cmd: "user 1234 1.2 2.3 123456 7890 pts/0 R+ 10:30 0:05 node server.js",
69+
})
70+
})
71+
72+
it("should parse Windows tasklist CSV output correctly", async () => {
73+
const mockOutput = `"Image Name","PID","Session Name","Session#","Mem Usage"
74+
"System Idle Process","0","Services","0","24 K"
75+
"System","4","Services","0","7,428 K"
76+
"notepad.exe","1234","Console","1","15,432 K"
77+
"chrome.exe","5678","Console","1","123,456 K"`
78+
79+
terminal.setMockResult({
80+
success: true,
81+
stdout: mockOutput,
82+
exitCode: 0,
83+
})
84+
85+
// Mock process.platform for Windows
86+
const originalPlatform = process.platform
87+
Object.defineProperty(process, "platform", { value: "win32" })
88+
89+
const processes = await terminal.getProcesses()
90+
91+
// Restore original platform
92+
Object.defineProperty(process, "platform", { value: originalPlatform })
93+
94+
expect(processes).toHaveLength(4)
95+
expect(processes[0]).toEqual({
96+
pid: 0,
97+
name: "System Idle Process",
98+
cmd: `"System Idle Process","0","Services","0","24 K"`,
99+
})
100+
expect(processes[1]).toEqual({
101+
pid: 4,
102+
name: "System",
103+
cmd: `"System","4","Services","0","7,428 K"`,
104+
})
105+
expect(processes[2]).toEqual({
106+
pid: 1234,
107+
name: "notepad.exe",
108+
cmd: `"notepad.exe","1234","Console","1","15,432 K"`,
109+
})
110+
})
111+
112+
it("should handle Windows CSV fallback parsing", async () => {
113+
const mockOutput = `Image Name,PID,Session Name,Session#,Mem Usage
114+
System Idle Process,0,Services,0,24 K
115+
notepad.exe,1234,Console,1,15432 K`
116+
117+
terminal.setMockResult({
118+
success: true,
119+
stdout: mockOutput,
120+
exitCode: 0,
121+
})
122+
123+
const originalPlatform = process.platform
124+
Object.defineProperty(process, "platform", { value: "win32" })
125+
126+
const processes = await terminal.getProcesses()
127+
128+
Object.defineProperty(process, "platform", { value: originalPlatform })
129+
130+
expect(processes).toHaveLength(2)
131+
expect(processes[0]).toEqual({
132+
pid: 0,
133+
name: "System Idle Process",
134+
cmd: "System Idle Process,0,Services,0,24 K",
135+
})
136+
expect(processes[1]).toEqual({
137+
pid: 1234,
138+
name: "notepad.exe",
139+
cmd: "notepad.exe,1234,Console,1,15432 K",
140+
})
141+
})
142+
143+
it("should filter processes by name when filter is provided", async () => {
144+
const mockOutput = `USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
145+
root 1 0.0 0.1 77616 8604 ? Ss Dec07 0:01 /sbin/init
146+
user 1234 1.2 2.3 123456 7890 pts/0 R+ 10:30 0:05 node server.js
147+
user 5678 0.5 1.1 98765 4321 pts/1 S 10:25 0:02 vim file.txt`
148+
149+
terminal.setMockResult({
150+
success: true,
151+
stdout: mockOutput,
152+
exitCode: 0,
153+
})
154+
155+
const processes = await terminal.getProcesses("node")
156+
157+
expect(processes).toHaveLength(1)
158+
expect(processes[0].pid).toBe(1234)
159+
expect(processes[0].name).toBe("user")
160+
})
161+
162+
it("should skip lines with invalid PIDs", async () => {
163+
const mockOutput = `USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
164+
root abc 0.0 0.1 77616 8604 ? Ss Dec07 0:01 /sbin/init
165+
user 1234 1.2 2.3 123456 7890 pts/0 R+ 10:30 0:05 node server.js`
166+
167+
terminal.setMockResult({
168+
success: true,
169+
stdout: mockOutput,
170+
exitCode: 0,
171+
})
172+
173+
const processes = await terminal.getProcesses()
174+
175+
expect(processes).toHaveLength(1)
176+
expect(processes[0].pid).toBe(1234)
177+
})
178+
179+
it("should return empty array when command fails", async () => {
180+
terminal.setMockResult({
181+
success: false,
182+
stderr: "Command not found",
183+
exitCode: 1,
184+
})
185+
186+
const processes = await terminal.getProcesses()
187+
188+
expect(processes).toHaveLength(0)
189+
})
190+
191+
it("should limit to 50 processes", async () => {
192+
// Generate 60 lines of mock output
193+
let mockOutput = "USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND\n"
194+
for (let i = 1; i <= 60; i++) {
195+
mockOutput += `user ${i.toString().padStart(4, " ")} 0.0 0.1 12345 6789 ? S Dec07 0:00 process${i}\n`
196+
}
197+
198+
terminal.setMockResult({
199+
success: true,
200+
stdout: mockOutput,
201+
exitCode: 0,
202+
})
203+
204+
const processes = await terminal.getProcesses()
205+
206+
expect(processes).toHaveLength(49) // 50 processes - 1 (header line is skipped)
207+
})
208+
})
209+
})

‎src/core/adapters/api/index.ts‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { CoreInterfaces } from "../../interfaces"
2-
import { createCliAdapters, CliAdapterOptions } from "../cli"
2+
import { createCliAdapters } from "../cli"
33
import { ApiUserInterface } from "./ApiUserInterface"
4+
import { ApiBrowser } from "./ApiBrowser"
5+
import { ApiFileSystem } from "./ApiFileSystem"
6+
import { ApiStorageService } from "./ApiStorage"
7+
import { ApiTelemetryService } from "./ApiTelemetryService"
48

59
/**
610
* Options for creating API adapters
@@ -18,39 +22,43 @@ export interface ApiAdapterOptions {
1822

1923
/**
2024
* Create API adapter implementations for all abstraction interfaces
21-
* For now, this reuses CLI adapters but replaces the user interface with API-specific one
25+
* Uses API-specific implementations where available, falling back to CLI adapters for terminal
2226
*/
2327
export async function createApiAdapters(options: ApiAdapterOptions = {}): Promise<CoreInterfaces> {
2428
const { workspaceRoot = process.cwd(), verbose = false, debug = false } = options
2529

26-
// Create CLI adapters as base
30+
// Create API-specific adapters
31+
const apiUserInterface = new ApiUserInterface({ verbose, debug })
32+
const apiBrowser = new ApiBrowser({ verbose })
33+
const apiFileSystem = new ApiFileSystem(workspaceRoot, { verbose })
34+
const apiStorage = new ApiStorageService({ verbose })
35+
const apiTelemetry = new ApiTelemetryService({ enabled: false, verbose })
36+
37+
// For terminal, we still use CLI adapter as it provides the actual terminal functionality
2738
const cliAdapters = createCliAdapters({
2839
workspaceRoot,
2940
isInteractive: false, // API mode is non-interactive
3041
verbose,
3142
})
3243

33-
// Replace user interface with API-specific implementation
34-
const apiUserInterface = new ApiUserInterface({ verbose, debug })
35-
3644
// Log adapter creation if verbose mode is enabled
3745
if (verbose) {
3846
console.log(`Created API adapters:`)
3947
console.log(` - UserInterface (API-specific, debug: ${debug})`)
40-
console.log(` - FileSystem (CLI-based, workspace: ${workspaceRoot})`)
48+
console.log(` - FileSystem (API-specific, workspace: ${workspaceRoot})`)
4149
console.log(` - Terminal (CLI-based)`)
42-
console.log(` - Browser (CLI-based)`)
43-
console.log(` - Telemetry (CLI-based)`)
44-
console.log(` - Storage (CLI-based)`)
50+
console.log(` - Browser (API-specific)`)
51+
console.log(` - Telemetry (API-specific)`)
52+
console.log(` - Storage (API-specific)`)
4553
}
4654

4755
return {
4856
userInterface: apiUserInterface,
49-
fileSystem: cliAdapters.fileSystem,
57+
fileSystem: apiFileSystem,
5058
terminal: cliAdapters.terminal,
51-
browser: cliAdapters.browser,
52-
telemetry: cliAdapters.telemetry,
53-
storage: cliAdapters.storage,
59+
browser: apiBrowser,
60+
telemetry: apiTelemetry,
61+
storage: apiStorage,
5462
}
5563
}
5664

@@ -100,5 +108,9 @@ export function validateApiAdapterOptions(options: ApiAdapterOptions): void {
100108
}
101109
}
102110

103-
// Re-export the API user interface for direct use
111+
// Re-export all API adapters for direct use
104112
export { ApiUserInterface } from "./ApiUserInterface"
113+
export { ApiBrowser } from "./ApiBrowser"
114+
export { ApiFileSystem } from "./ApiFileSystem"
115+
export { ApiStorageService } from "./ApiStorage"
116+
export { ApiTelemetryService } from "./ApiTelemetryService"

0 commit comments

Comments
 (0)