Fix: endSub has no timeout on readDone, unlike closeSub (#17)

[Bhumika-1432006]

Fixes #17

Bug

endSub in recorder.ts awaited entry.readDone with no timeout before finalizing a command's record. Its sibling handler, closeSub, already bounds the same wait to 1 second, with a comment acknowledging that read() "may never finish." That protection was never applied to endSub — the normal, non-killed-terminal completion path. If a command's output stream doesn't resolve promptly after the shell reports it finished (e.g. a backgrounded/detached child process keeping the terminal's stream open), drain() never resolves, entry.readDone never resolves, and endSub's handler would await forever — so the command never gets a row, even though the shell already reported a definite exit code.

Fix

Extracted the bounded-wait pattern into a small exported helper, waitAtMost(promise, timeoutMs), which races a promise against a timeout and resolves either way (swallowing rejection, since callers only care that the wait ended). endSub now does await waitAtMost(entry.readDone, 1000) instead of an unbounded await.

Timeout chosen: 1000ms, matching closeSub's existing bound exactly. Both handlers are protecting against the same underlying risk (read() not resolving), and there's no reason the normal-completion path should tolerate a longer or shorter stall than the terminal-closed path already treats as reasonable.

closeSub itself is untouched — it still uses its own inline Promise.race([...]) exactly as before, per the issue's suggested fix. (Its neighboring comment "Bounded, unlike the end handler's wait" is now slightly stale since endSub is bounded too, but I left closeSub's code as-is rather than touch a working, unrelated path — happy to update that comment in a follow-up if preferred.)

Testing

• npm run compile passes cleanly with no errors.
• Added src/test/suite/endSubTimeout.test.ts, a new regression-test file that imports waitAtMost directly (same pattern outputTruncation.test.ts already uses for sliceUtf8) and exercises it against:
  • an already-resolved promise (resolves fast)
  • an already-rejected promise (resolves fast, doesn't throw)
  • a promise that never settles at all — the exact shape of a stuck read() drain — asserting the wait still returns within the 1000ms bound instead of hanging forever, and doesn't return before the timeout either.
  • a rejecting promise raced against a real timeout, asserting waitAtMost never rejects.
• I did not attempt to reproduce a genuinely hung TerminalShellExecution.read() through a real terminal/command. This test harness has no mocking library, and vscode.window's terminal-shell-execution events aren't something I can reliably fake or force to hang from outside VS Code's own shell integration implementation (the same class of limitation noted for the drain() catch-block path in Fix: diagnostic notes silently dropped once output is already truncated (#15) #16). Instead, this test proves the exact bounded-wait mechanism endSub now relies on doesn't hang on a promise that never resolves — which is precisely the failure mode described in endSub has no timeout on entry.readDone, unlike closeSub — a stuck output stream silently drops the row #17 — deterministically and without needing a real terminal at all.
• Honest caveat, same as Fix: diagnostic notes silently dropped once output is already truncated (#15) #16: I could not run npm test (the Electron-hosted VS Code integration suite) in this environment. It's a Windows sandbox with no GUI/xvfb, and Code.exe doesn't launch as a real GUI app here regardless of shell — this matches this repo's own CI design, where the Windows/macOS jobs are compile-only and only ubuntu-latest under xvfb runs the integration suite. I did separately sanity-check the exact waitAtMost algorithm in a standalone plain-Node script (bypassing the blocked Electron host) and confirmed its timing behavior matches what the new test asserts. This PR's CI run will be the first real execution of the new test file inside the actual suite.