feat(terminal): add /workspaces/:id/terminal/diagnose endpoint - #2445
Merged
Conversation
GET /workspaces/:id/terminal/diagnose runs the same per-stage pipeline as
/terminal (ssh-keygen → EIC send-key → tunnel → ssh) but non-interactively
and returns JSON. Each stage reports {name, ok, duration_ms, error,
detail}, plus a top-level first_failure naming the broken stage.
Why: when the canvas terminal silently disconnects ("Session ended" with
no error frame — the user-reported failure mode on hongmingwang's hermes
workspace), there is no remote-readable signal of WHICH stage failed.
The ssh client's stderr lives only in the workspace-server's stdout on
the tenant CP EC2 — invisible without shell access. /terminal can't
expose stderr cleanly because it has already upgraded to WebSocket
binary frames by the time ssh runs. /terminal/diagnose stays pure
HTTP/JSON, so the same auth (WorkspaceAuth + ADMIN_TOKEN fallback) gives
operators a one-call probe that splits "IAM broke" (send-ssh-public-key
fails) from "tunnel/SG broke" (wait-for-port fails) from "sshd auth
broke" (ssh-probe gets Permission denied) from "shell broke" (probe
exits non-zero with stderr).
Stages mirrored from handleRemoteConnect in terminal.go:
1. ssh-keygen ephemeral session keypair
2. send-ssh-public-key AWS EIC API push, IAM-gated
3. pick-free-port local port for the tunnel
4. open-tunnel aws ec2-instance-connect open-tunnel start
5. wait-for-port the tunnel actually listens (folds tunnel
stderr into Detail when it doesn't)
6. ssh-probe non-interactive `ssh ... 'echo MARKER'` that
confirms auth + bash + the marker round-trip
(CombinedOutput captures stderr verbatim —
this is the whole reason the endpoint exists)
Local Docker workspaces (no instance_id) get a smaller probe:
container-found + container-running. Same response shape so callers
don't need to branch.
Tests stub sendSSHPublicKey / openTunnelCmd / sshProbeCmd via the
existing package-level vars (same pattern as TestSSHCommandCmd_*) so
the test suite stays hermetic — no AWS, no network. The three new
tests pin: (a) routing to remote on instance_id present,
(b) routing to local on empty instance_id, (c) the operationally
critical case — full success through wait-for-port then a probe
failure surfaces ssh stderr in the ssh-probe step's Error/Detail
with first_failure="ssh-probe".
Auth: rides on existing WorkspaceAuth middleware. Operators with the
tenant ADMIN_TOKEN (fetched via /cp/admin/orgs/:slug/admin-token) can
probe any workspace without per-workspace token; same admin path as
the canvas dashboard reads workspace activity.
Response always returns HTTP 200 (success or step failure are both in
the JSON body) so callers don't need to branch on status code — the
endpoint either reports a first_failure or doesn't.
Resolves task #200, supports task #193 (workspace EC2 sshd
unresponsive — without this endpoint we couldn't pin the failure
stage from outside the tenant CP EC2).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
requested a review
from hongmingwang-moleculeai
as a code owner
May 1, 2026 04:10
…ture Two fixes from /code-review-and-quality on PR #2445: 1. **KI-005 hierarchy check parity with /terminal** HandleConnect runs the KI-005 cross-workspace guard before dispatch (terminal.go:85-106): when X-Workspace-ID is set and != :id, validate the bearer's workspace binding then call canCommunicateCheck. Without this, an org-level token holder in tenant Foo can probe any workspace's diagnostic state by guessing the UUID — same enumeration vector KI-005 closed for /terminal in #1609. Per-workspace bearer tokens are URL-bound by WorkspaceAuth, so the gap is org tokens within the same tenant. Fix: copy the same gate into HandleDiagnose, before the instance_id SELECT. Test: TestHandleDiagnose_KI005_RejectsCrossWorkspace stubs canCommunicateCheck=false and confirms 403 fires before the DB lookup (sqlmock's ExpectationsWereMet pins that we never reached the SELECT COALESCE). Mirrors the existing TestTerminalConnect_KI005_RejectsUnauthorizedCrossWorkspace. 2. **Race-free tunnel stderr capture (syncBuf)** strings.Builder isn't goroutine-safe. os/exec spawns a background goroutine that copies the subprocess's stderr fd to cmd.Stderr's Write, so reading the buffer's String() from the request goroutine on wait-for-port timeout while the tunnel may still be writing is a data race that `go test -race` flags. Worst-case impact in production is a garbled Detail string (not a crash), but the fix is small. Fix: wrap bytes.Buffer in a sync.Mutex (syncBuf type). Same io.Writer interface, no API changes elsewhere. 3. **Nit cleanup** - read-pubkey failure now reports as its own step name instead of a duplicated "ssh-keygen" entry — disambiguates two different failure modes that previously shared a name. - Replaced numToString hand-rolled int-to-string with strconv.Itoa in the test (no import savings reason existed). Suite: 4 diagnose tests pass with -race; full handlers suite passes in 3.95s. go vet clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
Serialized merge by gitea-merge-queue after current-main, genuine approvals, and required CI checks were green.
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.
Summary
Adds
GET /workspaces/:id/terminal/diagnose— a non-interactive HTTP/JSON sibling of/terminalthat runs the same EIC + ssh stages and reports per-stage status. Built specifically because the canvas terminal currently fails silently ("Session ended" with no error frame on hongmingwang's hermes workspace) and there's no remote-readable signal of which stage broke — the ssh client's stderr lives only in the workspace-server's stdout on the tenant CP EC2.The endpoint splits the failure modes that all look identical from the user's side:
first_failure=\"send-ssh-public-key\", error containsAccessDeniedExceptionfirst_failure=\"wait-for-port\", detail contains theaws ec2-instance-connect open-tunnelstderrfirst_failure=\"ssh-probe\", detail containsPermission deniedfirst_failure=\"ssh-probe\", detail containsConnection refusedor timeoutStages mirrored from
handleRemoteConnectssh-keygen— ephemeral session keypairsend-ssh-public-key— AWS EIC API push (IAM-gated)pick-free-port— local port for the tunnelopen-tunnel—aws ec2-instance-connect open-tunnelsubprocesswait-for-port— confirm tunnel actually binds (folds tunnel stderr into Detail when it doesn't)ssh-probe— non-interactive `ssh ... 'echo MOLECULE_TERMINAL_PROBE_OK'` that proves auth + bash + round-trip viaCombinedOutputLocal Docker workspaces get a smaller
container-found+container-runningprobe, same response shape so callers don't need to branch.Auth
Rides on existing
WorkspaceAuthmiddleware. Operators with the tenantADMIN_TOKEN(fetched via/cp/admin/orgs/:slug/admin-token) can probe any workspace without per-workspace token — same admin path the canvas dashboard uses.Response shape
Always HTTP 200 (success or stage failure both in the JSON body — no need to branch on status):
```json
{
"workspace_id": "...",
"instance_id": "i-...",
"remote": true,
"ok": false,
"first_failure": "ssh-probe",
"steps": [
{"name": "ssh-keygen", "ok": true, "duration_ms": 12},
{"name": "send-ssh-public-key", "ok": true, "duration_ms": 234},
{"name": "pick-free-port", "ok": true, "duration_ms": 1, "detail": "port=12345"},
{"name": "open-tunnel", "ok": true, "duration_ms": 456},
{"name": "wait-for-port", "ok": true, "duration_ms": 789},
{"name": "ssh-probe", "ok": false, "duration_ms": 11000,
"error": "exit status 255",
"detail": "Permission denied (publickey)."}
]
}
```
Test plan
New tests
Tests stub `sendSSHPublicKey`, `openTunnelCmd`, `sshProbeCmd` via existing package-level vars (same pattern as `TestSSHCommandCmd_*`) so the suite stays hermetic — no AWS calls, no network.
Related
🤖 Generated with Claude Code