Feat/ts tools#130
Merged
luarss merged 44 commits intoJun 30, 2026
Merged
Conversation
Collaborator
Author
ea30af4 to
23b020b
Compare
Collaborator
|
@kartikloops Please rebase when you can |
…xecution from python
…ave the pty marked alive
…ent double kill and stale exit codes
…r is not discarded
…pe sequences is not reinterpreted
…rtup failure cannot leave a creating zombie
… creates are not lost
…lock all creation
…annot be bypassed via catch/if/foreach or set x [exec ls]
…ion is not leaked
…ches in TerminateSessionTool
…ot silently dropped
…te-transition side effect
… the dead session from the map
…flect post-loop state
…eady empties the map
…s not retried with SIGTERM
… does not produce a silent 256x256 output
… commands are not exempt
0e27dec to
461407a
Compare
Collaborator
Author
|
@luarss re-based, please have a look |
luarss
requested changes
Jun 26, 2026
… } or mid-word " cannot hide a trailing statement from the verb check
…le cannot escape the contained run directory
…eue/buffer/chunk sizes, command/idle timeouts) so a 0 cannot silently disable sessions or output
… so a bracket inside a # comment is not rejected as an exec
…l is not misreported dead, only ESRCH counts as gone
luarss
requested changes
Jun 27, 2026
…ry other domain model so snake_case is produced only by toSnakeCase at the wire boundary, not hand-written
…scated command like \socket or \x73ocket is matched as the command tcl actually runs, not the literal backslash form
…ituted command ($x or nested [[...]]) is surfaced and rejected by the read-only tool instead of running exec via runtime variable substitution
…rror message is inserted literally instead of being reinterpreted as a replacement pattern
42f3652
into
The-OpenROAD-Project:feat/ts-migration
1 check passed
kartikloops
added a commit
to kartikloops/openroad-mcp
that referenced
this pull request
Jul 3, 2026
* add base tool class with camelcase to snake case serialization boundary * add interactive tool classes porting session management and command execution from python * add report image tools using sharp for webp compression and path traversal validation * add tools barrel export * add tests for interactive shell and session management tools * add tests for report image tools covering path traversal and platform validation * verify process liveness with kill(0) so a missed exit event cannot leave the pty marked alive * serialize session terminate and cleanup with a lifecycle lock to prevent double kill and stale exit codes * drop redundant cleanup call in terminateSession so final output buffer is not discarded * use function replacement in annotate and preserve modes so $& in escape sequences is not reinterpreted * transition session to terminated from any non-terminal state so a startup failure cannot leave a creating zombie * ignore non-positive history limit so a negative value cannot drop recent commands * track session death time and use it for force-cleanup timing instead of last activity * skip null session placeholders in terminateAllSessions so in-progress creates are not lost * reject infinite and negative float settings so a timeout cannot be silently disabled * reject negative integer settings so a negative session limit cannot block all creation * backfill execution_time for all commands batched into a single readOutput * preserve exit code across cleanup so late waitForExit callers get the real code * use a live pid in the pty mock so the kill(0) liveness probe sees an active process * reject whitespace-only path segments in validatePathSegment * bound per-session command history so long-lived sessions do not leak memory * block body-eval builtins and bracket substitution so the query tool cannot be bypassed via catch/if/foreach or set x [exec ls] * terminate auto-created session when executeCommand throws so the session is not leaked * derive wasAlive from info.isAlive and propagate it through error branches in TerminateSessionTool * discard queued commands and warn on terminate so pending inputs are not silently dropped * rename isAlive to checkAlive on InteractiveSession to surface its state-transition side effect * wrap non-force cleanup in finally so a throwing cleanup still removes the dead session from the map * snapshot session counts after the async metrics loop so the totals reflect post-loop state * remove dead session loop in cleanupAll since terminateAllSessions already empties the map * call terminateProcess with force=true in cleanup so a stuck process is not retried with SIGTERM * guard against null image dimensions before resize so missing metadata does not produce a silent 256x256 output * update integration_check to call checkAlive instead of the renamed isAlive * extend bracket-scan regex to match [::exec ls] so namespace-qualified commands are not exempt * code cleanup * add interactive tool wire-format snapshots so CI snapshot tests pass * clamp brace depth and gate quotes on word boundaries so an unbalanced } or mid-word " cannot hide a trailing statement from the verb check * walk the report tree manually and skip symlinks so a linked dir or file cannot escape the contained run directory * reject zero for env settings where it is degenerate (max sessions, queue/buffer/chunk sizes, command/idle timeouts) so a 0 cannot silently disable sessions or output * scan bracket substitutions per statement and skip comment/blank lines so a bracket inside a # comment is not rejected as an exec * treat EPERM from the liveness probe as alive so a pid we cannot signal is not misreported dead, only ESRCH counts as gone * author the metrics and command-history payloads in camelCase like every other domain model so snake_case is produced only by toSnakeCase at the wire boundary, not hand-written * resolve tcl backslash escapes in the verb and bracket scan so an obfuscated command like \socket or \x73ocket is matched as the command tcl actually runs, not the literal backslash form * capture the bracket command word up to a delimiter so a dynamic substituted command ($x or nested [[...]]) is surfaced and rejected by the read-only tool instead of running exec via runtime variable substitution * use function replacement in _detectErrors so $& or $1 in a captured error message is inserted literally instead of being reinterpreted as a replacement pattern
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
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.
The full business logic layer is complete. This covers the PTY session management (spawning OpenROAD processes, reading output, handling termination), all 10 MCP tools with identical input/output schemas to the Python server, the command whitelist with security fixes that go beyond the Python baseline, report image listing and compression, settings, ANSI output cleaning, and path validation.
402 unit tests cover all of the above, plus an integration script that validates the full lifecycle against a real OpenROAD process.