fix: add authority diagnostics for workspace open

Author: EhabY

src/commands.ts

@@ -1348,7 +1348,6 @@ export class Commands {
 		if (!vscode.workspace.workspaceFolders?.length) {
 			newWindow = false;
 		}
-
 		if (!folderPath && useDefaultDirectory) {
 			folderPath = agent.expanded_directory;
 		}
@@ -1366,7 +1365,6 @@ export class Commands {
 				// reference a workspace without an agent name, which will be missed.
 				(opened) => opened.folderUri?.authority === remoteAuthority,
 			);
-
 			// openRecent will always use the most recent.  Otherwise, if there are
 			// multiple we ask the user which to use.
 			if (opened.length === 1 || (opened.length > 1 && openRecent)) {
@@ -1399,6 +1397,13 @@ export class Commands {
 				this.logger.error(`IPC ping failed for ${remoteAuthority}`, err);
 			});
 
+		this.logger.info("Opening remote workspace", {
+			remoteAuthority,
+			workspace: `${workspace.owner_name}/${workspace.name}`,
+			agent: agent.name,
+			handoff: folderPath ? "folder" : "empty_window",
+		});
+
 		if (folderPath) {
 			await vscode.commands.executeCommand(
 				"vscode.openFolder",

src/remote/remote.ts

@@ -135,13 +135,23 @@ export class Remote {
 		startupMode: StartupMode,
 		remoteSshExtensionId: string,
 	): Promise<RemoteDetails | undefined> {
-		const parts = parseRemoteAuthority(remoteAuthority);
+		let parts: AuthorityParts | null;
+		try {
+			parts = parseRemoteAuthority(remoteAuthority);
+		} catch (error) {
+			this.logger.warn("Failed to parse remote authority", {
+				remoteAuthority,
+				error: toError(error).message,
+			});
+			throw error;
+		}
 		if (!parts) {
 			// Not a Coder host.
 			return;
 		}
 
-		this.logger.debug("Setting up remote connection", {
+		this.logger.info("Setting up remote connection", {
+			remoteAuthority,
 			hostname: parts.safeHostname,
 			workspace: `${parts.username}/${parts.workspace}`,
 			agent: parts.agent || "(default)",

src/uri/uriHandler.ts

@@ -22,6 +22,7 @@ interface UriRouteContext extends UriHandlerDeps {
 }
 
 type UriRouteHandler = (ctx: UriRouteContext) => Promise<void>;
+type CoderUriRoute = "open" | "openDevContainer";
 
 const routes: Readonly<Record<string, UriRouteHandler>> = {
 	"/open": handleOpen,
@@ -48,7 +49,10 @@ export function registerUriHandler(deps: UriHandlerDeps): vscode.Disposable {
 				});
 			} catch (error) {
 				const message = errToStr(error, "No error message was provided");
-				output.warn(`Failed to handle URI ${uri.toString()}: ${message}`);
+				output.warn("Failed to handle URI", {
+					...summarizeUri(uri),
+					error: message,
+				});
 				vscodeProposed.window.showErrorMessage("Failed to handle URI", {
 					detail: message,
 					modal: true,
@@ -67,6 +71,17 @@ function getRequiredParam(params: URLSearchParams, name: string): string {
 	return value;
 }
 
+function summarizeUri(uri: vscode.Uri): Record<string, string | boolean> {
+	const params = new URLSearchParams(uri.query);
+	return {
+		path: uri.path,
+		hasOwner: params.has("owner"),
+		hasWorkspace: params.has("workspace"),
+		hasAgent: params.has("agent"),
+		hasToken: params.has("token"),
+	};
+}
+
 async function handleOpen(ctx: UriRouteContext): Promise<void> {
 	const { params, serviceContainer, deploymentManager, commands } = ctx;
 
@@ -78,7 +93,7 @@ async function handleOpen(ctx: UriRouteContext): Promise<void> {
 		params.has("openRecent") &&
 		(!params.get("openRecent") || params.get("openRecent") === "true");
 
-	await setupDeployment(params, serviceContainer, deploymentManager);
+	await setupDeployment("open", params, serviceContainer, deploymentManager);
 
 	await commands.open({
 		workspaceOwner: owner,
@@ -108,7 +123,12 @@ async function handleOpenDevContainer(ctx: UriRouteContext): Promise<void> {
 		);
 	}
 
-	await setupDeployment(params, serviceContainer, deploymentManager);
+	await setupDeployment(
+		"openDevContainer",
+		params,
+		serviceContainer,
+		deploymentManager,
+	);
 
 	await commands.openDevContainer(
 		owner,
@@ -126,6 +146,7 @@ async function handleOpenDevContainer(ctx: UriRouteContext): Promise<void> {
  * and token storage. Throws if user cancels URL input or login fails.
  */
 async function setupDeployment(
+	route: CoderUriRoute,
 	params: URLSearchParams,
 	serviceContainer: ServiceContainer,
 	deploymentManager: Pick<DeploymentManager, "setDeployment">,
@@ -154,6 +175,14 @@ async function setupDeployment(
 	}
 
 	const safeHostname = toSafeHost(url);
+	const owner = params.get("owner") ?? "";
+	const workspace = params.get("workspace") ?? "";
+	serviceContainer.getLogger().info("Handling Coder URI", {
+		route,
+		safeHostname,
+		workspace: owner && workspace ? `${owner}/${workspace}` : "",
+		agent: params.get("agent") ?? "(unspecified)",
+	});
 
 	const token: string | undefined = params.get("token") ?? undefined;
 	const result = await authTelemetry.traceLogin("uri", () =>

src/util.ts

@@ -58,9 +58,9 @@ export function findPort(text: string): number | null {
  * SSH host names created by this extension match the format:
  *   coder-vscode.<safeHostname>--<username>--<workspace>(.<agent?>)
  *
- * If this is not a Coder host, return null.
+ * If this is not a full Coder authority, return null.
  *
- * Throw an error if the host is invalid.
+ * Throw an error if a full Coder authority is invalid.
  */
 export function parseRemoteAuthority(authority: string): AuthorityParts | null {
 	const authorityParts = authority.split("+");
@@ -75,7 +75,7 @@ export function parseRemoteAuthority(authority: string): AuthorityParts | null {
 	}
 
 	if (parts.length < 3) {
-		throw new Error(invalidAuthorityMessage);
+		return null;
 	}
 
 	// Parse from the right because safe hostnames can contain "--".

test/unit/uri/uriHandler.test.ts

@@ -363,11 +363,43 @@ describe("uriHandler", () => {
 			await handleUri(createMockUri("/open", "workspace=w"));
 
 			expect(logger.warn).toHaveBeenCalledWith(
-				expect.stringContaining("Failed to handle URI"),
+				"Failed to handle URI",
+				expect.objectContaining({
+					error: expect.stringContaining("owner must be specified"),
+				}),
 			);
 			expect(showErrorMessage).toHaveBeenCalled();
 		});
 
+		it("does not log URI tokens on failure", async () => {
+			const { handleUri, commands, logger } = createTestContext();
+			commands.open.mockRejectedValue(new Error("Connection failed"));
+
+			await handleUri(
+				createMockUri(
+					"/open",
+					`owner=o&workspace=w&url=${encodeURIComponent(TEST_URL)}&token=secret-token`,
+				),
+			);
+
+			expect(logger.warn).toHaveBeenCalledWith(
+				"Failed to handle URI",
+				expect.objectContaining({
+					error: "Connection failed",
+				}),
+			);
+			const logged = [
+				...vi.mocked(logger.debug).mock.calls,
+				...vi.mocked(logger.info).mock.calls,
+				...vi.mocked(logger.warn).mock.calls,
+			]
+				.map((call) => JSON.stringify(call))
+				.join("\n");
+			expect(logged).not.toContain("secret-token");
+			expect(logged).not.toContain("token=");
+			expect(logged).not.toContain("vscode://coder.coder-remote");
+		});
+
 		it("propagates command errors", async () => {
 			const { handleUri, commands, showErrorMessage } = createTestContext();
 			commands.open.mockRejectedValue(new Error("Connection failed"));

test/unit/util.test.ts

@@ -12,6 +12,7 @@ import {
 	openInBrowser,
 	parseRemoteAuthority,
 	resolveUiUrl,
+	toRemoteAuthority,
 	toSafeHost,
 } from "@/util";
 
@@ -36,19 +37,23 @@ describe("parseRemoteAuthority", () => {
 			input: remoteAuthority("coder-vscode-test--foo--bar"),
 		},
 		{ label: "wrong prefix", input: remoteAuthority("coder--foo--bar") },
+		{
+			label: "incomplete Coder host",
+			input: remoteAuthority("coder-vscode.dev.coder.com"),
+		},
+		{
+			label: "incomplete Coder host with username",
+			input: remoteAuthority("coder-vscode.dev.coder.com--foo"),
+		},
+		{
+			label: "manual non-Coder host with Coder prefix",
+			input: remoteAuthority("coder-vscode.personal-host"),
+		},
 	])("ignores unrelated authority: $label", ({ input }) => {
 		expect(parseRemoteAuthority(input)).toBe(null);
 	});
 
 	it.each([
-		{
-			label: "missing user and workspace",
-			sshHost: "coder-vscode.dev.coder.com",
-		},
-		{
-			label: "missing workspace",
-			sshHost: "coder-vscode.dev.coder.com--foo",
-		},
 		{
 			label: "empty username",
 			sshHost: "coder-vscode.dev.coder.com----bar",
@@ -88,6 +93,26 @@ describe("parseRemoteAuthority", () => {
 		username?: string;
 	}
 
+	it("round trips generated remote authorities", () => {
+		const authority = toRemoteAuthority(
+			"https://ほげ",
+			"alice",
+			"workspace",
+			"main",
+		);
+
+		expect(authority).toBe(
+			"ssh-remote+coder-vscode.xn--18j4d--alice--workspace.main",
+		);
+		expect(parseRemoteAuthority(authority)).toStrictEqual({
+			agent: "main",
+			sshHost: "coder-vscode.xn--18j4d--alice--workspace.main",
+			safeHostname: "xn--18j4d",
+			username: "alice",
+			workspace: "workspace",
+		} satisfies AuthorityParts);
+	});
+
 	it.each<ParseCase>([
 		{
 			label: "hostname without agent",