perf(terminal): stream output and history - #8564

💭 I'm scanning the diff for Effect service convention issues, focusing on the terminal manager and adapter files, the ws.ts entry point, and contract definitions, while noting the mobile/web app changes are React-based rather than Effect services. In Manager.ts specifically I'm checking how Semaphore is being used. make(1) and its import, and reviewing the newly added readSnapshot method on the service interface to make sure the imports align with conventions.

1. browse_code — apps/server/src/terminal/Manager.ts

2. browse_code — apps/server/src/terminal/PtyAdapter.ts

💭 These changes look fine to me. I'm now checking the ws.ts modifications involving Queue, Option, and Effect.gen, then reviewing the diff for any convention violations—things like error swallowing with Effect.ignore, questionable semantics around readSnapshot's TerminalClearInput parameter, semaphore placement in session state, and an import change in terminal.ts. I'm noticing that in terminalSession.ts, the import statement changed from a type-only import to a regular import with inline type prefixes on individual items, which suggests a value import might have been added alongside the type imports—I need to look at what's actually being imported now.

3. browse_code — packages/client-runtime/src/state/terminalSession.ts

4. browse_code — packages/client-runtime/src/state/terminal.ts

💭 The terminalSession.ts change just swapped a type-only import style for an equivalent one with no actual value imports—harmless and not a real convention violation. I'm now checking whether any new error classes were introduced in this diff and don't see any. I'm going through ws.ts, ProjectSetupScriptRunner.test.ts, and Manager.test.ts for convention issues—test layers and service provisions look fine, nothing flagged. I'm scanning the mobile terminal surface too, but it relies on native promises rather than Effect services, so it's out of scope for these checks. Checking Manager.ts and its naming conventions for service construction, everything matches existing patterns like makeWithOptions. I don't see any missing catchTags or improper queue usage in the attach stream code, though I want to look closer at how queuePersist's error handling catches errors in Manager.ts. The Effect.catch usage there is pre-existing and appropriate since it handles the full error channel. Import ordering with DateTime before Context is also pre-existing, not a new issue. I'm now scanning the touched files more broadly for any new ManagedRuntime, runPromise, or Layer.succeed calls to be thorough—so far ws.ts shows nothing new, and the test files use acceptable seams like providing a mock FileSystem service or constructing one via .of() for write tracking. The web ghostty terminal files don't use Effect at all. I want to do one more targeted grep across the changed files for .runPromise and ManagedRuntime just to confirm nothing slipped through.

5. git_tools — apps/server/src/ws.ts 9b2d043..f459a54

💭 I'm checking mobile module files for Effect usage and confirming service interface references look correct, like the TerminalManager Service type usage. I'm also scanning Manager.ts's new code for any error-handling anti-patterns, but the nested catch logging appears legitimate and consistent with existing style. One remaining question is whether production service construction properly acquires dependencies via yield, so I'm checking whether TerminalManagerOptions added service-instance injection like a ptyAdapter field, or whether the diff only added numeric options fields — I need to look at the options interface directly in Manager.ts to confirm.

6. browse_code — apps/server/src/terminal/Manager.ts

💭 That pty adapter injection was already present before this PR, so it's not something introduced here and doesn't warrant a finding. Since there's nothing else to flag, I'll report success with a concise "All clear" summary, keeping the output minimal despite the conflicting instruction to output plain text.