fix(wp): restore interactive WebSocket shell

[sjinks]

Description

Running vip @app.environment wp against a WebSocket environment sent an empty command to the API instead of opening the interactive shell. Restore the Go shell loop so commands are triggered only after input, retain the prompt between commands, and print API errors once without GraphQL location prefixes.

Update Node WebSocket completion so explicit remote exit events return to the shell and preserve delayed nonzero statuses. Add terminal editing, safe stdin handoff, Ctrl-C delivery to remote stdin, and terminal restoration on interruption. Replace the obsolete WebSocket redirect fixture with real Node-versus-Go subprocess coverage.

Fixes #3096.

Browser-launch test stubbing is intentionally excluded and will be handled separately.

Changelog Description

Fixed

• WP-CLI: Restore the interactive WebSocket shell in VIP CLI 5 and keep the shell available after command failures.
• WP-CLI: Avoid duplicate API errors and retain remote command exit statuses.

Pull request checklist

• Add automated regression tests for both runtimes.
• Build both runtimes and run relevant checks.
• Manually test against a live VIP environment
• No new environment variables or documented command syntax changes.

Steps to Test

1. Run npm run build and make build.
2. Run npm run jest -- --runTestsByPath __tests__/commands/wp-ssh.ts __tests__/lib/cli/command-wp-matrix.test.js --runInBand.
3. Run go test -race ./cmd/vip-next/commands ./internal/wpshell ./internal/wpstream -run 'TestWP|TestREPL|TestInput|TestEngineIO|TestClient|TestRun|TestStream|TestSocket' -count=1 with ambient proxy variables cleared.
4. Run NODE_VIP_BIN=$PWD/dist/bin/vip.js go test -tags=parity -count=1 ./internal/parity -run '^(TestWPWebsocket|TestEverySurfaceScenarioIsClassified|TestSurfaceClassificationIsExclusive)' -v.
5. Run npm run check-types, npm run cmd:lint -- src/bin/vip-wp.js, and make lint.

These checks passed locally. The focused subprocess checks also passed with hostile ambient credentials, API-host, and proxy settings; no tests skipped. They cover opening and exiting the shell, invalid input, API errors, successive successful commands, stdout EOF before a nonzero exit event, one-shot exit codes, PTY interaction, remote Ctrl-C byte delivery, and terminal restoration after SIGTERM.

Ref: PLTFRM-3036

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[Copilot]

🟡 Changes recommended

Cancellation can lose prompt input or hang indefinitely while waiting for blocked output.

2 open findings

• Cancellation can hang while waiting for blocked stdout writer · New
• Cancellation can consume the first byte of the next prompt · New

What changed in this PR

Restores interactive WebSocket WP-CLI shells across Node.js and Go with improved exit handling, terminal input, and runtime parity coverage.

Changes:

• Restores the Go interactive shell and terminal handling.
• Preserves delayed WebSocket exit statuses in both runtimes.
• Replaces redirect fixtures with subprocess parity tests.

File	Description
src/​bin/​vip-wp.js	Handles delayed and explicit remote exits.
cmd/​vip-next/​commands/​wp.go	Implements the Go WebSocket shell loop.
cmd/​vip-next/​commands/​wp_test.go	Tests shell prompting and API errors.
internal/​wpshell/​repl.go	Adds editable terminal line input.
internal/​wpshell/​input.go	Coordinates prompt and remote stdin.
internal/​wpshell/​input_test.go	Tests stdin forwarding and handoff.
internal/​wpstream/​run.go	Waits for output shutdown on cancellation.
internal/​parity/​wp_shell_differential_test.go	Adds Node-versus-Go shell tests.
internal/​parity/​surface_differential_test.go	Classifies the new parity scenario.
testdata/​parity/​wp-websocket-shell.yaml	Defines the interactive shell scenario.
testdata/​parity/​wp-websocket-redirect.yaml	Removes the obsolete redirect scenario.
testdata/​parity/​recordings/​wp-websocket-shell/​resolve-app.json	Adds WebSocket environment fixture data.
testdata/​parity/​recordings/​wp-websocket-redirect/​resolve-app.json	Removes obsolete fixture data.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The new parity test file cannot compile on Windows because its build constraint includes Unix-only PTY code.

1 open finding

• Exclude Windows from parity PTY test · New

2 resolved since last review

• Cancellation can hang while waiting for blocked stdout writer
• Cancellation can consume the first byte of the next prompt

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Reconnection timers and listeners can terminate resumed jobs or duplicate exit handling, while repeated remote interruptions exit the shell unexpectedly.

0 open findings

1 resolved since last review

• Exclude Windows from parity PTY test

Previously missed (1)

In code that hasn't changed since last review

Cumulative interrupt count cancels later commands on first Ctrl-C

cmd/​vip-next/​commands/​wp.go:174

interrupts is cumulative for the entire shell session. After Ctrl-C is forwarded to one remote command, it remains 1, so the first Ctrl-C during any later command satisfies interrupts > 1, cancels the whole shell, and is never delivered remotely. Reset the counter when each command starts, and mirror that reset for Node's countSIGINT to preserve cross-runtime behavior.

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

SIGTERM handling changes exit semantics, and one-shot Node.js completion can lose telemetry.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Preserve SIGTERM exit semantics instead of returning context.Canceled

cmd/​vip-next/​commands/​wp.go:161

signal.NotifyContext consumes SIGTERM, but the resulting context.Canceled is returned from the command and main prints it as an error and exits with code 1. Consequently kill -TERM no longer preserves normal signal termination semantics and emits a spurious Error: context canceled. Handle the signal explicitly so terminal restoration runs, then restore/re-deliver the signal (or otherwise preserve its exit status) instead of returning the context error.

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Reconnect timing can terminate a resumed Node command, and multiline Go input renders incorrect prompts.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Suppress repeated prompts while reading multiline commands

internal/​wpshell/​repl.go:32

In terminal mode, term.Terminal.ReadLine renders its prompt on every call. A multiline quoted WP command leaves the parser incomplete and loops back here, so Go prints another full shell prompt for each continuation line, while the Node shell only prompts initially and after a completed command. The line-reader API needs a continuation mode that reads the next physical line without rendering the normal prompt.

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Go derives remote stdin normalization from stdout TTY state, breaking interactive input when stdout is redirected.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Derive IsTTY from stdin instead of stdout

cmd/​vip-next/​commands/​wp.go:505

IsTTY controls CR→LF conversion in wpstream, but it is still derived from os.Stdout while this path now supplies stdin independently. With terminal stdin and redirected stdout (for example, vip … wp >log), the shell enters raw mode but remote stdin receives \r instead of \n, unlike Node which keys normalization off stdin. Determine TTY status from cmd.InOrStdin() before passing this reader.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Reconnection can drop remote stdin bytes or retry after an already-delivered exit status.

1 open finding

• Disconnect can override a queued command exit status · New

Previously missed (1)

In code that hasn't changed since last review

Reconnect can drop the first byte of stdin

cmd/​vip-next/​commands/​wp.go:217

This reconnect path can drop the first byte entered after reconnection. Each runOnce starts an io.Copy from the same opts.Stdin; abortAll only aborts the old destination stream, so when the old copier is blocked in Read, the new copier waits behind it. The next byte is consumed by the old copier and then discarded when its write hits the aborted stream. Use a single stdin pump that can switch active streams, or otherwise cancel and fully join the prior attempt without consuming input before starting the next attempt.

🧠 Review effort: Balanced

[sjinks]

Addressed both reconnect findings in 58b81bc: a single stdin pump now spans reconnect attempts, and queued exit packets/status take priority over transport failure. Added unit regressions and real Node/Go parity coverage. Also isolated release-promotion test HTTP transports in 9cae212 to prevent parallel httptest server shutdown from interrupting asset uploads. Validation: 37 focused Node tests, Node build/type-check, focused Go race tests, 100 race-enabled release-promotion runs, 11 WP differential tests without skips, five repeated runs of both new differential tests, Go vet, and Windows cross-compilation. Full Go tests were not run locally because existing unrelated tests open a browser; CI has been triggered by the push.

[Copilot]

🟡 Changes recommended

Leading-whitespace shell commands currently produce different API commands in the Go and Node.js runtimes.

1 open finding

• Node CLI fails to strip leading whitespace before wp command · New

1 resolved since last review

• Disconnect can override a queued command exit status

🧠 Review effort: Balanced

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

🔵 Needs a closer look

Cross-runtime terminal, signal, and concurrent stream lifecycle changes warrant final human review.

0 open findings

1 resolved since last review

• Node CLI fails to strip leading whitespace before wp command

🧠 Review effort: Balanced