Skip to content

Commit 5b89038

Browse files
HonaHona
committed
update
1 parent 71db262 commit 5b89038

2 files changed

Lines changed: 70 additions & 133 deletions

File tree

‎packages/opencode/test/util/clipboard-integration.test.ts‎

Lines changed: 18 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,66 +2,48 @@ import { describe, expect, test } from "bun:test"
22
import path from "path"
33

44
/**
5-
* Integration test to verify the clipboard fix handles process cleanup correctly.
5+
* Test to verify the Windows clipboard memory leak pattern exists.
66
*
7-
* This test verifies:
8-
* 1. On Windows, PowerShell process cleanup code is present
9-
* 2. The implementation uses Bun.spawn instead of $ for better control
10-
* 3. Timeout and error handling are in place
11-
*
12-
* USAGE: Run this test on both macOS and Windows
13-
* - On macOS: Test runs in "check mode" to verify fixes exist in code
14-
* - On Windows: Test validates actual process cleanup behavior
15-
*
16-
* TEST EXPECTATIONS:
17-
* ✅ When fix is applied: Test PASSES
18-
* ❌ When fix is stashed: Test FAILS
7+
* These tests confirm the LEAKY code pattern is present (pre-fix).
8+
* After fixing, these tests should FAIL - update them to check for the fix.
199
*/
2010

21-
describe("Windows Memory Leak Fixes", () => {
22-
test("clipboard.ts contains fixed Windows implementation", async () => {
11+
describe("Windows Clipboard Leak Pattern Detection", () => {
12+
test("clipboard.ts uses shell $ pattern (leaky on Windows)", async () => {
2313
const clipboardPath = path.join(process.cwd(), "packages/opencode/src/cli/cmd/tui/util/clipboard.ts")
2414
const content = await Bun.file(clipboardPath).text()
2515

26-
// CRITICAL: Check for new fixed implementation with Bun.spawn
27-
expect(content).toContain("Bun.spawn")
16+
// Current LEAKY pattern - uses $ which doesn't guarantee cleanup
2817
expect(content).toContain('if (os === "win32")')
2918
expect(content).toContain("powershell")
30-
expect(content).toContain("proc.kill()")
31-
32-
// CRITICAL: Verify timeout mechanism is present
33-
expect(content).toContain("Promise.race")
34-
expect(content).toContain("5000") // 5 second timeout
3519

36-
// CRITICAL: Should NOT contain old broken pattern
37-
expect(content).not.toContain("await $`powershell -command")
20+
// This is the LEAK: using $ instead of Bun.spawn with explicit cleanup
21+
expect(content).toContain("await $`powershell")
3822

39-
console.log("✓ Fixed Windows clipboard implementation verified")
23+
console.log("✓ Confirmed: clipboard.ts uses leaky $ pattern for Windows")
4024
})
4125

42-
test("bash.ts contains fixed taskkill cleanup", async () => {
26+
test("bash.ts taskkill lacks explicit cleanup (leaky)", async () => {
4327
const bashPath = path.join(process.cwd(), "packages/opencode/src/tool/bash.ts")
4428
const content = await Bun.file(bashPath).text()
4529

46-
// Verify Windows taskkill code
4730
expect(content).toContain('process.platform === "win32"')
4831
expect(content).toContain("taskkill")
4932

50-
// CRITICAL: Verify cleanup improvements are present
51-
expect(content).toContain("removeAllListeners")
52-
expect(content).toContain("unref")
33+
// Current LEAKY pattern - no removeAllListeners/unref
34+
expect(content).not.toContain("removeAllListeners")
35+
expect(content).not.toContain("killer.unref")
5336

54-
console.log("✓ Fixed bash taskkill cleanup verified")
37+
console.log("✓ Confirmed: bash.ts taskkill lacks explicit cleanup")
5538
})
5639

57-
test("lsp/server.ts has correct ElixirLS filename", async () => {
40+
test("lsp/server.ts has ElixirLS typo (.bar instead of .bat)", async () => {
5841
const lspPath = path.join(process.cwd(), "packages/opencode/src/lsp/server.ts")
5942
const content = await Bun.file(lspPath).text()
6043

61-
// CRITICAL: Verify the typo is fixed
62-
expect(content).not.toContain("language_server.bar")
63-
expect(content).toContain("language_server.bat")
44+
// Bug: typo in filename
45+
expect(content).toContain("language_server.bar")
6446

65-
console.log("✓ ElixirLS filename typo fixed")
47+
console.log("✓ Confirmed: lsp/server.ts has ElixirLS .bar typo")
6648
})
6749
})

‎packages/opencode/test/util/clipboard-memory.test.ts‎

Lines changed: 52 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -1,122 +1,77 @@
11
import { describe, expect, test } from "bun:test"
2+
import { platform } from "os"
23

34
/**
4-
* Memory leak test demonstrating the Windows clipboard issue
5+
* Memory leak reproduction test for clipboard operations.
56
*
6-
* This test simulates the OLD broken implementation vs the NEW fixed implementation
7-
* to show how process references accumulate in memory during long chat sessions.
7+
* - macOS: Should PASS (no leak - uses osascript properly)
8+
* - Windows: Should FAIL (leak exists - PowerShell processes accumulate)
89
*/
910

10-
interface ProcessRef {
11-
pid: number
12-
listeners: number
13-
streams: { [key: string]: boolean }
14-
unrefed: boolean
15-
}
16-
17-
// Simulates the OLD broken implementation (without explicit cleanup)
18-
async function oldBrokenClipboardCopy(text: string): Promise<ProcessRef> {
19-
return new Promise((resolve) => {
20-
const proc: ProcessRef = {
21-
pid: Math.random(),
22-
listeners: 2, // 'exit' and 'error' listeners left attached
23-
streams: { stdout: true, stderr: true },
24-
unrefed: false, // NOT unref'd - keeps process alive
25-
}
26-
// Simulate PowerShell via $ without explicit cleanup
27-
setTimeout(() => {
28-
resolve(proc)
29-
// Process references remain in memory! No cleanup happens.
30-
}, 10)
31-
})
32-
}
33-
34-
// Simulates the NEW fixed implementation (with explicit cleanup)
35-
async function newFixedClipboardCopy(text: string): Promise<ProcessRef> {
36-
return new Promise((resolve) => {
37-
const proc: ProcessRef = {
38-
pid: Math.random(),
39-
listeners: 0, // Listeners removed
40-
streams: {},
41-
unrefed: true, // unref'd - allows clean exit
42-
}
43-
// Simulate Bun.spawn with timeout and cleanup
44-
const cleanup = () => {
45-
proc.listeners = 0 // removeAllListeners()
46-
proc.streams = {}
47-
resolve(proc)
48-
}
49-
setTimeout(cleanup, 10)
50-
})
51-
}
52-
53-
describe("Clipboard Memory Leak Analysis", () => {
54-
test("OLD implementation accumulates process references", async () => {
55-
const processes: ProcessRef[] = []
56-
const sessionLength = 50 // Simulate 50 copy operations in a chat session
11+
const isWindows = platform() === "win32"
5712

58-
for (let i = 0; i < sessionLength; i++) {
59-
const proc = await oldBrokenClipboardCopy(`Copy operation ${i}`)
60-
processes.push(proc)
61-
}
13+
describe("Clipboard Memory Leak", () => {
14+
test("memory should not grow significantly after many clipboard operations", async () => {
15+
const { Clipboard } = await import("../../src/cli/cmd/tui/util/clipboard")
6216

63-
// Count "leaky" processes (those with active listeners or streams)
64-
const leakyProcesses = processes.filter((p) => p.listeners > 0 || Object.keys(p.streams).length > 0)
17+
if (typeof Bun.gc === "function") Bun.gc(true)
6518

66-
// In the OLD implementation, most/all processes would be leaky
67-
console.log(`\nOLD IMPLEMENTATION: ${leakyProcesses.length}/${sessionLength} processes leaked`)
68-
expect(leakyProcesses.length).toBeGreaterThan(sessionLength * 0.8) // At least 80% leaked
69-
})
19+
const baselineRSS = process.memoryUsage.rss()
20+
const iterations = 10000
7021

71-
test("NEW implementation properly cleans up process references", async () => {
72-
const processes: ProcessRef[] = []
73-
const sessionLength = 50 // Same session length
22+
console.log(`\nRunning ${iterations} clipboard operations on ${platform()}...`)
7423

75-
for (let i = 0; i < sessionLength; i++) {
76-
const proc = await newFixedClipboardCopy(`Copy operation ${i}`)
77-
processes.push(proc)
24+
for (let i = 0; i < iterations; i++) {
25+
await Clipboard.copy(`Memory test ${i} - ${"x".repeat(500)}`)
7826
}
7927

80-
// Count "leaky" processes
81-
const leakyProcesses = processes.filter((p) => p.listeners > 0 || Object.keys(p.streams).length > 0)
28+
if (typeof Bun.gc === "function") Bun.gc(true)
29+
await new Promise((r) => setTimeout(r, 1000))
8230

83-
// In the NEW implementation, no processes should leak
84-
console.log(`NEW IMPLEMENTATION: ${leakyProcesses.length}/${sessionLength} processes leaked`)
85-
expect(leakyProcesses.length).toBe(0)
31+
const finalRSS = process.memoryUsage.rss()
32+
const growthMB = (finalRSS - baselineRSS) / 1024 / 1024
8633

87-
// All should be properly unref'd
88-
const unrefedCount = processes.filter((p) => p.unrefed).length
89-
expect(unrefedCount).toBe(sessionLength)
90-
})
34+
console.log(`\nMemory after ${iterations} clipboard operations:`)
35+
console.log(` Platform: ${platform()}`)
36+
console.log(` Baseline: ${(baselineRSS / 1024 / 1024).toFixed(2)}MB`)
37+
console.log(` Final: ${(finalRSS / 1024 / 1024).toFixed(2)}MB`)
38+
console.log(` Growth: ${growthMB.toFixed(2)}MB`)
9139

92-
test("demonstrates memory accumulation during long sessions", async () => {
93-
// Simulate a long chat session with frequent copy operations
94-
const sessionDuration = 100 // 100 copy operations
40+
// macOS: should pass (minimal growth)
41+
// Windows: should fail (significant growth due to leak)
42+
expect(growthMB).toBeLessThan(30)
43+
})
9544

96-
// OLD implementation memory model
97-
let oldMemoryUsage = 0
98-
for (let i = 0; i < sessionDuration; i++) {
99-
const proc = await oldBrokenClipboardCopy(`Data ${i}`)
100-
// Each leaky process retains ~50KB of memory (listeners, streams, references)
101-
oldMemoryUsage += proc.listeners > 0 ? 50 : 0 // KB
45+
test.skipIf(!isWindows)("PowerShell processes should not accumulate", async () => {
46+
const { Clipboard } = await import("../../src/cli/cmd/tui/util/clipboard")
47+
const { execSync } = await import("child_process")
48+
49+
const countPowershellProcesses = () => {
50+
try {
51+
const output = execSync('tasklist /FI "IMAGENAME eq powershell.exe" /NH', { encoding: "utf8" })
52+
return output.split("\n").filter((line) => line.includes("powershell")).length
53+
} catch {
54+
return 0
55+
}
10256
}
10357

104-
// NEW implementation memory model
105-
let newMemoryUsage = 0
106-
for (let i = 0; i < sessionDuration; i++) {
107-
const proc = await newFixedClipboardCopy(`Data ${i}`)
108-
// Properly cleaned up processes use minimal memory
109-
newMemoryUsage += proc.unrefed ? 1 : 0 // KB (just process ID tracking)
58+
const baselineProcesses = countPowershellProcesses()
59+
console.log(`\nBaseline PowerShell processes: ${baselineProcesses}`)
60+
61+
const iterations = 10000
62+
for (let i = 0; i < iterations; i++) {
63+
await Clipboard.copy(`Process test ${i} - ${Date.now()}`)
11064
}
11165

112-
console.log(`\nMemory usage after ${sessionDuration} operations:`)
113-
console.log(` OLD (broken): ~${oldMemoryUsage}KB`)
114-
console.log(` NEW (fixed): ~${newMemoryUsage}KB`)
66+
await new Promise((r) => setTimeout(r, 500))
67+
68+
const afterProcesses = countPowershellProcesses()
69+
const growth = afterProcesses - baselineProcesses
11570

116-
const improvementPercent = (((oldMemoryUsage - newMemoryUsage) / oldMemoryUsage) * 100).toFixed(1)
117-
console.log(` Improvement: ${improvementPercent}%\n`)
71+
console.log(`After ${iterations} copies: ${afterProcesses} processes`)
72+
console.log(`Process growth: ${growth}`)
11873

119-
// The new implementation should use significantly less memory
120-
expect(newMemoryUsage).toBeLessThan(oldMemoryUsage)
74+
// Should fail on Windows if leak exists
75+
expect(growth).toBeLessThan(5)
12176
})
12277
})

0 commit comments

Comments
 (0)