refactor(sessions): give a session entry a host and a transport-supplied handle - #225
Merged
Merged
Conversation
…on handle
activeSessions entries and getPtyForSession assumed every session was a local
node-pty: trigger-watcher wrote into ptyProcess.write() and probed liveness
with process.kill(pid, 0), neither of which makes sense for a session whose
process lives on another host. getPtyForSession now attaches a handle --
{ write(data), isAlive() } -- to each entry: for a local one (session.host ==
null) it's deduced from session.pty exactly as before (same write, same
signal-0 probe, now named createLocalSessionHandle); a non-null host would
take session.handle as supplied instead, with no remote implementation added
here. trigger-watcher.js no longer touches ptyProcess.write or process.kill
directly, only the handle. activeSessions entries also carry host (null for
local) and kind, descriptive metadata nothing reads yet.
Refs #220
devsuitup
enabled auto-merge (squash)
September 8, 2026 12:11
This was referenced Sep 8, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #220.
activeSessionsandgetPtyForSessionassumed every session is a localnode-pty: a pid signalable withprocess.kill(pid, 0)and awrite()on alocal handle. Nothing in that shape can represent a session whose process runs on
another machine, so every remote capability above observation needs this resolved
first.
This opens the seam and nothing else. No remote implementation here.
An entry keeps the session id as its key and gains
host(null for local) andkind.getPtyForSessionreturns a handle —{ write, isAlive }— deduced fromthe
node-ptyfor a local entry, taken as given for a non-local one. Thewatcher no longer calls
ptyProcess.writeorprocess.killdirectly.Local behaviour is unchanged. No existing test was modified; the suite passes as
written.
Verification
Three mutations, each with the changed line printed before the run and the file
restored from an md5-verified copy:
writemade a no-op → 48 existing watcher tests red. The seam ison the real injection path, not beside it.
isAliveforced true → the new liveness test red.node-pty→ the new seam test red.Worth naming: no pre-existing test isolated the liveness branch, because every
hand-built ctx in
trigger-watcher.test.jssupplies its ownisPtyAlive, whichtakes precedence over the handle. A test was added rather than an existing one
adjusted.
The new seam test is the one that proves the seam is worth having: it drives a
session through the real
createTriggerContext, against a realactiveSessionsmap, with an entry carrying only a test-supplied handle — no
ptyfieldanywhere.
task check: 1124 tests, 1116 passed, 0 failed, 8 pre-existing skips. Lint 0errors.