Show Remote SSH Output panel on workspace start #627

EhabY · 2025-10-17T13:12:09Z

Closes #626

src/extension.ts

EhabY · 2025-10-24T09:40:43Z

src/remote/remote.ts

-						title: "Waiting for the agent to connect...",
-						location: vscode.ProgressLocation.Notification,
+						progressTitle: "Waiting for the agent to connect...",
+						timeoutMs: 3 * 60 * 1000,


We used to potentially wait forever, should we set a timeout for this? What's a good value 🤔 ?

I think there is no good timeout value really, agents can take an arbitrary amount of time to connect. As long as there is no error or network disconnect, we should keep waiting.

If we did need a timeout, it should probably be something like where we send a ping and if we do not get a pong back in N seconds after M attempts then we error. I think maybe websockets already do something like that though?

Ultimately though as long as the connection looks good, and we are communicating the status to the user, we should let the user decide whether they want to wait, IMO.

Yeah fair enough, timing out for something that can potentially take arbitrary amount of time and would lead to the product being unusable is bad. Removed the timeout!

This actually brings up a question, should we make the progress indicator cancellable? Currently it's not which might make sense since cancelling means we close the remote connection.

src/remote/remote.ts

code-asher

I have not had a chance to try it yet, but it is looking really good!

code-asher · 2025-10-24T20:55:59Z

src/api/workspace.ts

-					}
-				});
+				.filter((line) => line !== "");
+			for (const line of lines) {


I see a few places have been changed from forEach to a for of loop, was there a specific reason for this or just preference?

I installed sonarqube which has some default warnings, see https://next.sonarqube.com/sonarqube/coding_rules?open=javascript%3AS7728&rule_key=javascript%3AS7728

It's not really that significant but it's better I suppose

code-asher · 2025-10-24T20:59:39Z

src/api/workspace.ts

+	// This fetches the initial bunch of logs.
+	const logs = await client.getWorkspaceAgentLogs(agent.id);
+	for (const log of logs) {
+		writeEmitter.fire(log.output + "\r\n");
+	}


The watch endpoint already streams all the logs, I think (including previous ones, since the default for the after query param is zero which means all logs), so I believe we would not need the extra getWorkspaceAgentLogs call.

Is this also true for the waitForBuild? because there we also do:

// This fetches the initial bunch of logs. const logs = await client.getWorkspaceBuildLogs(workspace.latest_build.id); for (const log of logs) { writeEmitter.fire(log.output + "\r\n"); } const socket = await client.watchBuildLogsByBuildId( workspace.latest_build.id, logs, );

Seems to be the case, I've removed the initial fetching and the logs seem to be complete for both requests 🤔

code-asher · 2025-10-24T21:04:11Z

src/remote/remote.ts

-						title: "Waiting for the agent to connect...",
-						location: vscode.ProgressLocation.Notification,
+						progressTitle: "Waiting for the agent to connect...",
+						timeoutMs: 3 * 60 * 1000,


I think there is no good timeout value really, agents can take an arbitrary amount of time to connect. As long as there is no error or network disconnect, we should keep waiting.

If we did need a timeout, it should probably be something like where we send a ping and if we do not get a pong back in N seconds after M attempts then we error. I think maybe websockets already do something like that though?

Ultimately though as long as the connection looks good, and we are communicating the status to the user, we should let the user decide whether they want to wait, IMO.

src/remote/remote.ts

code-asher · 2025-10-24T21:35:31Z

src/remote/remote.ts

+					try {
+						terminalSession = new TerminalSession("Agent Log");
+						const { socket: agentSocket, completion: logsCompletion } =
+							await streamAgentLogs(


The await here I think means we could possibly miss a state change in between the previous watch and this? Maybe we could store the current state in monitor and check it before registering the onChange.

But, I do feel like we have this state machine that is broken up into separate sections and it could make a lot of sense to put it all together. A single machine that can take a workspace object, explicitly lists out all the possible states, and does the right thing.

For example what if the workspace is stopped while we wait for the scripts? We are only checking the lifecycle state so we would miss that change I think. The web socket would close eventually, although I think we might not be watching for that either (I think this may have already been an issue though, or I am misreading the code haha).

Do you mean that we have one big state machine that checks the workspace state AND the life cycle state of the agents?

So for example, if we are starting the workspace then handle that as before, if we switch to started then we handle the agent life cycle all in one big switch that is executed on every onChange

Done, added an agent status/lifecycle state machine and made sure to throw an error if at any point the workspace is not running

src/remote/remote.ts

…tial ones beforehand

EhabY force-pushed the show-remote-ssh-output-on-start branch from bbb0391 to 92b1788 Compare October 17, 2025 13:13

EhabY commented Oct 17, 2025

View reviewed changes

src/extension.ts Outdated Show resolved Hide resolved

johnstcn assigned EhabY Oct 20, 2025

johnstcn requested a review from code-asher October 20, 2025 14:11

EhabY force-pushed the show-remote-ssh-output-on-start branch from 92b1788 to 719de6f Compare October 22, 2025 08:41

Show Remote SSH Output panel on workspace start

fe6d1dc

EhabY force-pushed the show-remote-ssh-output-on-start branch from 719de6f to fe6d1dc Compare October 23, 2025 10:31

EhabY added 3 commits October 23, 2025 16:49

Wait for startup script when launching the workspace

c49dc60

Refactoring and cleanup of agent startup script waiting

e6bb283

More refactoring

a34f9c6

EhabY commented Oct 24, 2025

View reviewed changes

src/remote/remote.ts Outdated Show resolved Hide resolved

code-asher reviewed Oct 24, 2025

View reviewed changes

EhabY added 3 commits October 28, 2025 11:33

Review comments

07a10a8

Add agent state machine

3313619

Rely on the websocket to get all the logs instead of fetching the ini…

878e2ae

…tial ones beforehand

Uh oh!

Show Remote SSH Output panel on workspace start #627

Are you sure you want to change the base?

Show Remote SSH Output panel on workspace start #627

Conversation

EhabY commented Oct 17, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EhabY Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EhabY Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

EhabY Oct 27, 2025 •

edited

Loading

EhabY Oct 27, 2025 •

edited

Loading