Skip to content

Commit 94f876a

Browse files
kartikloopsCopilot
andauthored
Feat/ts pty handler migration (#126)
* add vscode workspace settings for typescript sdk path * add plan doc * add core session state and error models * add interactive session error models * add circular buffer for pty output * add pty handler for managing node-pty processes * add interactive session manager * add tests for circular buffer * add tests for command validation * add tests for pty handler * add tests for interactive session * update eslint config with coverage ignore and stricter rules * fix pending waiters, env cast and resolver types in pty handler * guard read chunk size against zero in session onData handler * add test for cleanup resolving pending waitForExit waiters * add real openroad repl integration check script * code cleanup * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kartik Mittal <100078751+kartikloops@users.noreply.github.com> * added case for READ_CHUNK_SIZE is larger than limit Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kartik Mittal <100078751+kartikloops@users.noreply.github.com> * fix isAlive signaling shutdown when process death is detected * add test for isAlive shutdown signal on process death * fix force terminate returning before alive flag clears * update tests for force terminate waiting for exit * fix terminate not disposing pty listeners and pending waiters * update terminate tests to assert pty cleanup is called * fix clear not waking pending waitForData callers * add regression test for clear waking pending waitForData callers * signal shutdown in readOutput drain path so writer task does not loop indefinitely * add regression test for readOutput signaling shutdown on dead session * fix waitForData not re-checking dataAvailable under mutex after runExclusive * add regression test for waitForData race between fast-path check and mutex * handle append rejection in onData handler to avoid silent data loss * add regression test for append rejection transitioning session to terminated * move MacOSPTYHandler to production and add create_pty_handler factory * use create_pty_handler so macOS gets the keepalive and drain subclass * remove duplicate MacOSPTYHandler from test file and import from production * remove MacOSPTYHandler and create_pty_handler — not needed for PoC * await SIGKILL exit in terminateProcess so cleanup cannot race the exit listener * block path traversal sequences in validateCommand argument check * reject absolute paths in python validateCommand to match typescript behavior * truncate single oversized chunk to maxSize to enforce capacity invariant * throw on unrecognised boolean env vars and lazily init settings to prevent silent security disable * fix ansi decoder escape coverage, annotate mode cr handling, mode validation and pre-compile sequence patterns * wait for exit after kill() throw to observe natural death rather than returning blind * bound waitForExit after SIGKILL to 5s to prevent indefinite hang on D-state processes * revert python absolute path change and align typescript to accept absolute paths via basename check * revert python pty handler and integration test to main --------- Signed-off-by: Kartik Mittal <100078751+kartikloops@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 7151a77 commit 94f876a

15 files changed

Lines changed: 2516 additions & 16 deletions

File tree

.vscode/settings.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"typescript.tsdk": "typescript/node_modules/typescript/lib"
3+
}
Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
import { describe, it, expect } from "vitest";
2+
import { CircularBuffer } from "../../src/interactive/buffer.js";
3+
4+
describe("CircularBuffer", () => {
5+
describe("basic operations", () => {
6+
it("starts empty", () => {
7+
const buf = new CircularBuffer(100);
8+
expect(buf.size).toBe(0);
9+
expect(buf.chunkCount).toBe(0);
10+
});
11+
12+
it("appends and drains a single chunk", async () => {
13+
const buf = new CircularBuffer(100);
14+
await buf.append("hello");
15+
expect(buf.size).toBe(5);
16+
expect(buf.chunkCount).toBe(1);
17+
18+
const chunks = await buf.drainAll();
19+
expect(chunks).toEqual(["hello"]);
20+
expect(buf.size).toBe(0);
21+
});
22+
23+
it("handles multiple chunks with peek and drain", async () => {
24+
const buf = new CircularBuffer(100);
25+
await buf.append("chunk1");
26+
await buf.append("chunk2");
27+
await buf.append("chunk3");
28+
29+
expect(buf.chunkCount).toBe(3);
30+
expect(buf.size).toBe(18);
31+
32+
const peeked = await buf.peekAll();
33+
expect(peeked).toEqual(["chunk1", "chunk2", "chunk3"]);
34+
expect(buf.size).toBe(18);
35+
36+
const drained = await buf.drainAll();
37+
expect(drained).toEqual(["chunk1", "chunk2", "chunk3"]);
38+
expect(buf.size).toBe(0);
39+
});
40+
});
41+
42+
describe("eviction", () => {
43+
it("evicts oldest chunks when limit is exceeded", async () => {
44+
const buf = new CircularBuffer(10);
45+
await buf.append("12345");
46+
await buf.append("67890");
47+
await buf.append("ABCDE");
48+
49+
const chunks = await buf.drainAll();
50+
expect(chunks).toEqual(["67890", "ABCDE"]);
51+
});
52+
53+
it("truncates a single oversized chunk to the last maxSize characters", async () => {
54+
const buf = new CircularBuffer(10);
55+
await buf.append("12345");
56+
await buf.append("67890");
57+
await buf.append("LARGE_CHUNK_EXCEEDS");
58+
59+
const chunks = await buf.drainAll();
60+
expect(chunks).toHaveLength(1);
61+
expect(chunks[0]).toBe("NK_EXCEEDS");
62+
expect(buf.size).toBe(0);
63+
});
64+
});
65+
66+
describe("edge cases", () => {
67+
it("ignores empty string appends", async () => {
68+
const buf = new CircularBuffer(100);
69+
await buf.append("");
70+
expect(buf.size).toBe(0);
71+
expect(buf.chunkCount).toBe(0);
72+
73+
await buf.append("hello");
74+
await buf.append("");
75+
expect(buf.size).toBe(5);
76+
expect(buf.chunkCount).toBe(1);
77+
});
78+
79+
it("zero-size buffer discards all data", async () => {
80+
const buf = new CircularBuffer(0);
81+
await buf.append("test");
82+
expect(buf.size).toBe(0);
83+
});
84+
85+
it("very small buffer truncates oversized chunk to maxSize", async () => {
86+
const buf = new CircularBuffer(1);
87+
await buf.append("ab");
88+
const chunks = await buf.drainAll();
89+
expect(chunks).toHaveLength(1);
90+
expect(chunks[0]).toBe("b");
91+
expect(buf.size).toBe(0);
92+
});
93+
});
94+
95+
describe("toText", () => {
96+
it("joins chunks into a single string", async () => {
97+
const buf = new CircularBuffer(100);
98+
await buf.append("hello");
99+
await buf.append(" ");
100+
await buf.append("world");
101+
102+
const chunks = await buf.drainAll();
103+
expect(buf.toText(chunks)).toBe("hello world");
104+
});
105+
106+
it("returns empty string for empty array", () => {
107+
const buf = new CircularBuffer(100);
108+
expect(buf.toText([])).toBe("");
109+
});
110+
});
111+
112+
describe("waitForData", () => {
113+
it("returns false when no data arrives within timeout", async () => {
114+
const buf = new CircularBuffer(100);
115+
const result = await buf.waitForData(10);
116+
expect(result).toBe(false);
117+
});
118+
119+
it("returns true immediately when data is already present", async () => {
120+
const buf = new CircularBuffer(100);
121+
await buf.append("test");
122+
const result = await buf.waitForData(10);
123+
expect(result).toBe(true);
124+
});
125+
126+
it("wakes up when data arrives asynchronously", async () => {
127+
const buf = new CircularBuffer(100);
128+
await buf.clear();
129+
130+
const addTask = (async () => {
131+
await new Promise<void>((r) => setTimeout(r, 10));
132+
await buf.append("delayed");
133+
})();
134+
135+
const result = await buf.waitForData(100);
136+
expect(result).toBe(true);
137+
await addTask;
138+
});
139+
140+
it("returns true when append fires between fast-path check and runExclusive callback (re-check prevents missed wakeup)", async () => {
141+
const buf = new CircularBuffer(100);
142+
143+
// Start waitForData (fast-path sees _dataAvailable = false and enters the Promise)
144+
const waiter = buf.waitForData(5000);
145+
146+
// append() fires here — before runExclusive's callback has a chance to push
147+
// wakeUp into _resolvers. Without the re-check inside runExclusive, wakeUp
148+
// would be pushed after append() already drained an empty _resolvers, and
149+
// the caller would wait the full 5-second timeout.
150+
await buf.append("raced data");
151+
152+
const result = await waiter;
153+
expect(result).toBe(true);
154+
});
155+
});
156+
157+
describe("clear", () => {
158+
it("removes all data and resets size", async () => {
159+
const buf = new CircularBuffer(100);
160+
await buf.append("test1");
161+
await buf.append("test2");
162+
163+
await buf.clear();
164+
expect(buf.size).toBe(0);
165+
expect(buf.chunkCount).toBe(0);
166+
});
167+
168+
it("wakes pending waitForData() immediately with false so callers do not hang", async () => {
169+
const buf = new CircularBuffer(100);
170+
171+
// waitForData with a large timeout — clear() must unblock it before the timeout fires
172+
const waiter = buf.waitForData(5000);
173+
await buf.clear();
174+
175+
const result = await waiter;
176+
expect(result).toBe(false);
177+
});
178+
});
179+
180+
describe("getStats", () => {
181+
it("returns zero stats on empty buffer", async () => {
182+
const buf = new CircularBuffer(100);
183+
const stats = await buf.getStats();
184+
expect(stats.totalChars).toBe(0);
185+
expect(stats.chunkCount).toBe(0);
186+
expect(stats.maxSize).toBe(100);
187+
expect(stats.utilizationPercent).toBe(0);
188+
});
189+
190+
it("reflects data added to the buffer", async () => {
191+
const buf = new CircularBuffer(100);
192+
await buf.append("test_data");
193+
const stats = await buf.getStats();
194+
expect(stats.totalChars).toBe(9);
195+
expect(stats.chunkCount).toBe(1);
196+
expect(stats.utilizationPercent).toBe(9);
197+
});
198+
});
199+
200+
describe("concurrent access", () => {
201+
it("handles concurrent writers and a reader", async () => {
202+
const buf = new CircularBuffer(1000);
203+
204+
const writer = async (prefix: string, count: number) => {
205+
for (let i = 0; i < count; i++) {
206+
await buf.append(`${prefix}_${i}`);
207+
await new Promise<void>((r) => setTimeout(r, 1));
208+
}
209+
};
210+
211+
const reader = async () => {
212+
const all: string[] = [];
213+
for (let i = 0; i < 10; i++) {
214+
const chunks = await buf.drainAll();
215+
all.push(...chunks);
216+
await new Promise<void>((r) => setTimeout(r, 2));
217+
}
218+
return all;
219+
};
220+
221+
const [, , collected] = await Promise.all([writer("A", 5), writer("B", 5), reader()]);
222+
223+
expect(collected.length).toBeGreaterThan(0);
224+
const text = buf.toText(collected);
225+
expect(text).toContain("A_");
226+
expect(text).toContain("B_");
227+
});
228+
});
229+
});

0 commit comments

Comments
 (0)