Skip to content

Commit 4bec022

Browse files
EhabYEhabY
committed
Address review comments
1 parent a26acbc commit 4bec022

File tree

4 files changed

+158
-105
lines changed

4 files changed

+158
-105
lines changed

‎src/remote/remote.ts‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -741,21 +741,24 @@ export class Remote {
741741
featureSet: FeatureSet,
742742
): vscode.Disposable {
743743
return vscode.workspace.onDidChangeConfiguration((e) => {
744-
if (e.affectsConfiguration("coder.proxyLogDirectory")) {
745-
const newLogDir = this.getLogDir(featureSet);
746-
if (newLogDir !== currentLogDir) {
747-
vscode.window
748-
.showInformationMessage(
749-
"Log directory configuration changed. Reload window to apply.",
750-
"Reload",
751-
)
752-
.then((action) => {
753-
if (action === "Reload") {
754-
vscode.commands.executeCommand("workbench.action.reloadWindow");
755-
}
756-
});
757-
}
744+
if (!e.affectsConfiguration("coder.proxyLogDirectory")) {
745+
return;
758746
}
747+
const newLogDir = this.getLogDir(featureSet);
748+
if (newLogDir === currentLogDir) {
749+
return;
750+
}
751+
752+
vscode.window
753+
.showInformationMessage(
754+
"Log directory configuration changed. Reload window to apply.",
755+
"Reload",
756+
)
757+
.then((action) => {
758+
if (action === "Reload") {
759+
vscode.commands.executeCommand("workbench.action.reloadWindow");
760+
}
761+
});
759762
});
760763
}
761764

‎src/remote/sshProcess.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,7 @@ export class SshProcessMonitor implements vscode.Disposable {
135135

136136
/**
137137
* Searches for the SSH process indefinitely until found or disposed.
138-
* Tries port-based discovery first (more accurate), falls back to hostname-based.
139-
* When found, starts monitoring.
138+
* Starts monitoring when it finds the process through the port.
140139
*/
141140
private async searchForProcess(): Promise<void> {
142141
const { pollInterval, logger, sshHost } = this.options;

‎src/util.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,11 @@ export interface AuthorityParts {
1414
export const AuthorityPrefix = "coder-vscode";
1515

1616
// Regex patterns to find the SSH port from Remote SSH extension logs.
17-
// `ms-vscode-remote.remote-ssh`: `-> socksPort <port> ->`
17+
// `ms-vscode-remote.remote-ssh`: `-> socksPort <port> ->` or `between local port <port>`
1818
// `codeium.windsurf-remote-openssh`, `jeanp413.open-remote-ssh`, `google.antigravity-remote-openssh`: `=> <port>(socks) =>`
19-
// Windows `ms-vscode-remote.remote-ssh`: `between local port <port>`
2019
// `anysphere.remote-ssh`: `Socks port: <port>`
2120
export const RemoteSSHLogPortRegex =
22-
/(?:-> socksPort (\d+) ->|=> (\d+)\(socks\) =>|between local port (\d+)|Socks port: (\d+))/g;
21+
/(?:-> socksPort (\d+) ->|between local port (\d+)|=> (\d+)\(socks\) =>|Socks port: (\d+))/g;
2322

2423
/**
2524
* Given the contents of a Remote - SSH log file, find the most recent port
@@ -34,7 +33,11 @@ export function findPort(text: string): number | null {
3433
return null;
3534
}
3635

36+
// Get the last match, which is the most recent port.
3737
const lastMatch = allMatches.at(-1)!;
38+
// Each capture group corresponds to a different Remote SSH extension log format:
39+
// [0] full match, [1] and [2] ms-vscode-remote.remote-ssh,
40+
// [3] windsurf/open-remote-ssh/antigravity, [4] anysphere.remote-ssh
3841
const portStr = lastMatch[1] || lastMatch[2] || lastMatch[3] || lastMatch[4];
3942
if (!portStr) {
4043
return null;

‎test/unit/util.test.ts‎

Lines changed: 134 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -5,99 +5,104 @@ import {
55
countSubstring,
66
escapeCommandArg,
77
expandPath,
8+
findPort,
89
parseRemoteAuthority,
910
toSafeHost,
1011
} from "@/util";
1112

12-
it("ignore unrelated authorities", () => {
13-
const tests = [
14-
"vscode://ssh-remote+some-unrelated-host.com",
15-
"vscode://ssh-remote+coder-vscode",
16-
"vscode://ssh-remote+coder-vscode-test",
17-
"vscode://ssh-remote+coder-vscode-test--foo--bar",
18-
"vscode://ssh-remote+coder-vscode-foo--bar",
19-
"vscode://ssh-remote+coder--foo--bar",
20-
];
21-
for (const test of tests) {
22-
expect(parseRemoteAuthority(test)).toBe(null);
23-
}
24-
});
25-
26-
it("should error on invalid authorities", () => {
27-
const tests = [
28-
"vscode://ssh-remote+coder-vscode--foo",
29-
"vscode://ssh-remote+coder-vscode--",
30-
"vscode://ssh-remote+coder-vscode--foo--",
31-
"vscode://ssh-remote+coder-vscode--foo--bar--",
32-
];
33-
for (const test of tests) {
34-
expect(() => parseRemoteAuthority(test)).toThrow("Invalid");
35-
}
36-
});
37-
38-
it("should parse authority", () => {
39-
expect(
40-
parseRemoteAuthority("vscode://ssh-remote+coder-vscode--foo--bar"),
41-
).toStrictEqual({
42-
agent: "",
43-
host: "coder-vscode--foo--bar",
44-
label: "",
45-
username: "foo",
46-
workspace: "bar",
47-
});
48-
expect(
49-
parseRemoteAuthority("vscode://ssh-remote+coder-vscode--foo--bar--baz"),
50-
).toStrictEqual({
51-
agent: "baz",
52-
host: "coder-vscode--foo--bar--baz",
53-
label: "",
54-
username: "foo",
55-
workspace: "bar",
56-
});
57-
expect(
58-
parseRemoteAuthority(
59-
"vscode://ssh-remote+coder-vscode.dev.coder.com--foo--bar",
60-
),
61-
).toStrictEqual({
62-
agent: "",
63-
host: "coder-vscode.dev.coder.com--foo--bar",
64-
label: "dev.coder.com",
65-
username: "foo",
66-
workspace: "bar",
67-
});
68-
expect(
69-
parseRemoteAuthority(
70-
"vscode://ssh-remote+coder-vscode.dev.coder.com--foo--bar--baz",
71-
),
72-
).toStrictEqual({
73-
agent: "baz",
74-
host: "coder-vscode.dev.coder.com--foo--bar--baz",
75-
label: "dev.coder.com",
76-
username: "foo",
77-
workspace: "bar",
78-
});
79-
expect(
80-
parseRemoteAuthority(
81-
"vscode://ssh-remote+coder-vscode.dev.coder.com--foo--bar.baz",
82-
),
83-
).toStrictEqual({
84-
agent: "baz",
85-
host: "coder-vscode.dev.coder.com--foo--bar.baz",
86-
label: "dev.coder.com",
87-
username: "foo",
88-
workspace: "bar",
13+
describe("parseRemoteAuthority", () => {
14+
it("ignore unrelated authorities", () => {
15+
const tests = [
16+
"vscode://ssh-remote+some-unrelated-host.com",
17+
"vscode://ssh-remote+coder-vscode",
18+
"vscode://ssh-remote+coder-vscode-test",
19+
"vscode://ssh-remote+coder-vscode-test--foo--bar",
20+
"vscode://ssh-remote+coder-vscode-foo--bar",
21+
"vscode://ssh-remote+coder--foo--bar",
22+
];
23+
for (const test of tests) {
24+
expect(parseRemoteAuthority(test)).toBe(null);
25+
}
26+
});
27+
28+
it("should error on invalid authorities", () => {
29+
const tests = [
30+
"vscode://ssh-remote+coder-vscode--foo",
31+
"vscode://ssh-remote+coder-vscode--",
32+
"vscode://ssh-remote+coder-vscode--foo--",
33+
"vscode://ssh-remote+coder-vscode--foo--bar--",
34+
];
35+
for (const test of tests) {
36+
expect(() => parseRemoteAuthority(test)).toThrow("Invalid");
37+
}
38+
});
39+
40+
it("should parse authority", () => {
41+
expect(
42+
parseRemoteAuthority("vscode://ssh-remote+coder-vscode--foo--bar"),
43+
).toStrictEqual({
44+
agent: "",
45+
host: "coder-vscode--foo--bar",
46+
label: "",
47+
username: "foo",
48+
workspace: "bar",
49+
});
50+
expect(
51+
parseRemoteAuthority("vscode://ssh-remote+coder-vscode--foo--bar--baz"),
52+
).toStrictEqual({
53+
agent: "baz",
54+
host: "coder-vscode--foo--bar--baz",
55+
label: "",
56+
username: "foo",
57+
workspace: "bar",
58+
});
59+
expect(
60+
parseRemoteAuthority(
61+
"vscode://ssh-remote+coder-vscode.dev.coder.com--foo--bar",
62+
),
63+
).toStrictEqual({
64+
agent: "",
65+
host: "coder-vscode.dev.coder.com--foo--bar",
66+
label: "dev.coder.com",
67+
username: "foo",
68+
workspace: "bar",
69+
});
70+
expect(
71+
parseRemoteAuthority(
72+
"vscode://ssh-remote+coder-vscode.dev.coder.com--foo--bar--baz",
73+
),
74+
).toStrictEqual({
75+
agent: "baz",
76+
host: "coder-vscode.dev.coder.com--foo--bar--baz",
77+
label: "dev.coder.com",
78+
username: "foo",
79+
workspace: "bar",
80+
});
81+
expect(
82+
parseRemoteAuthority(
83+
"vscode://ssh-remote+coder-vscode.dev.coder.com--foo--bar.baz",
84+
),
85+
).toStrictEqual({
86+
agent: "baz",
87+
host: "coder-vscode.dev.coder.com--foo--bar.baz",
88+
label: "dev.coder.com",
89+
username: "foo",
90+
workspace: "bar",
91+
});
8992
});
9093
});
9194

92-
it("escapes url host", () => {
93-
expect(toSafeHost("https://foobar:8080")).toBe("foobar");
94-
expect(toSafeHost("https://ほげ")).toBe("xn--18j4d");
95-
expect(toSafeHost("https://test.😉.invalid")).toBe("test.xn--n28h.invalid");
96-
expect(toSafeHost("https://dev.😉-coder.com")).toBe(
97-
"dev.xn---coder-vx74e.com",
98-
);
99-
expect(() => toSafeHost("invalid url")).toThrow("Invalid URL");
100-
expect(toSafeHost("http://ignore-port.com:8080")).toBe("ignore-port.com");
95+
describe("toSafeHost", () => {
96+
it("escapes url host", () => {
97+
expect(toSafeHost("https://foobar:8080")).toBe("foobar");
98+
expect(toSafeHost("https://ほげ")).toBe("xn--18j4d");
99+
expect(toSafeHost("https://test.😉.invalid")).toBe("test.xn--n28h.invalid");
100+
expect(toSafeHost("https://dev.😉-coder.com")).toBe(
101+
"dev.xn---coder-vx74e.com",
102+
);
103+
expect(() => toSafeHost("invalid url")).toThrow("Invalid URL");
104+
expect(toSafeHost("http://ignore-port.com:8080")).toBe("ignore-port.com");
105+
});
101106
});
102107

103108
describe("countSubstring", () => {
@@ -190,3 +195,46 @@ describe("expandPath", () => {
190195
expect(expandPath("~/${userHome}/foo")).toBe(`${home}/${home}/foo`);
191196
});
192197
});
198+
199+
describe("findPort", () => {
200+
it.each([[""], ["some random log text without ports"]])(
201+
"returns null for <%s>",
202+
(input) => {
203+
expect(findPort(input)).toBe(null);
204+
},
205+
);
206+
207+
it.each([
208+
[
209+
"ms-vscode-remote.remote-ssh",
210+
"[10:30:45] SSH established -> socksPort 12345 -> ready",
211+
12345,
212+
],
213+
[
214+
"ms-vscode-remote.remote-ssh[2]",
215+
"Forwarding between local port 54321 and remote",
216+
54321,
217+
],
218+
[
219+
"windsurf/open-remote-ssh/antigravity",
220+
"[INFO] Connection => 9999(socks) => target",
221+
9999,
222+
],
223+
[
224+
"anysphere.remote-ssh",
225+
"[DEBUG] Initialized Socks port: 8888 proxy",
226+
8888,
227+
],
228+
])("finds port from %s log format", (_name, input, expected) => {
229+
expect(findPort(input)).toBe(expected);
230+
});
231+
232+
it("returns most recent port when multiple matches exist", () => {
233+
const log = `
234+
[10:30:00] Starting connection -> socksPort 1111 -> initialized
235+
[10:30:05] Reconnecting => 2222(socks) => retry
236+
[10:30:10] Final connection Socks port: 3333 established
237+
`;
238+
expect(findPort(log)).toBe(3333);
239+
});
240+
});

0 commit comments

Comments
 (0)