Check compatibility when autostarting, not before autostart

web/packages/teleterm/src/ui/ConnectMyComputer/CompatibilityPromise.tsx

@@ -49,7 +49,9 @@ export function checkAgentCompatibility(
     : 'incompatible';
 }
 
-export function CompatibilityError(): JSX.Element {
+export function CompatibilityError(props: {
+  hideAlert?: boolean;
+}): JSX.Element {
   const { proxyVersion, appVersion } = useVersions();
 
   const clusterMajorVersion = getMajorVersion(proxyVersion);
@@ -91,7 +93,11 @@ export function CompatibilityError(): JSX.Element {
 
   return (
     <>
-      <Alert>Detected an incompatible agent version.</Alert>
+      {!props.hideAlert && (
+        <Alert>
+          The agent version is not compatible with the cluster version
+        </Alert>
+      )}
       <Text>
         The cluster is on version {proxyVersion} while Teleport Connect is on
         version {appVersion}. Per our{' '}

web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.story.tsx

@@ -27,7 +27,10 @@ import { MockAppContext } from 'teleterm/ui/fixtures/mocks';
 import { AgentProcessState } from 'teleterm/mainProcess/types';
 import { makeRuntimeSettings } from 'teleterm/mainProcess/fixtures/mocks';
 
-import { ConnectMyComputerContextProvider } from '../connectMyComputerContext';
+import {
+  AgentCompatibilityError,
+  ConnectMyComputerContextProvider,
+} from '../connectMyComputerContext';
 
 import { DocumentConnectMyComputerStatus } from './DocumentConnectMyComputerStatus';
 
@@ -107,6 +110,24 @@ export function AgentVersionTooNew() {
   );
 }
 
+export function AgentVersionTooNewWithFailedAutoStart() {
+  const appContext = new MockAppContext({ appVersion: '17.0.0' });
+
+  appContext.connectMyComputerService.downloadAgent = () =>
+    Promise.reject(new AgentCompatibilityError('incompatible'));
+  appContext.connectMyComputerService.isAgentConfigFileCreated = () =>
+    Promise.resolve(true);
+
+  return (
+    <ShowState
+      agentProcessState={{ status: 'not-started' }}
+      appContext={appContext}
+      proxyVersion={'16.3.0'}
+      autoStart={true}
+    />
+  );
+}
+
 // Shows only cluster upgrade instructions.
 // Downgrading the app would result in installing a version that doesn't support 'Connect My Computer'.
 // DELETE IN 17.0.0 (gzdunek): by the time 17.0 releases, 14.x will no longer be
@@ -151,6 +172,7 @@ function ShowState(props: {
   agentProcessState: AgentProcessState;
   appContext?: AppContext;
   proxyVersion?: string;
+  autoStart?: boolean;
 }) {
   const cluster = makeRootCluster({
     proxyVersion: props.proxyVersion || makeRuntimeSettings().appVersion,
@@ -194,11 +216,27 @@ function ShowState(props: {
     await wait(3_000);
     throw new Error('TIMEOUT. Cannot find node.');
   };
+  appContext.configService.set('feature.connectMyComputer', true);
   appContext.clustersService.state.clusters.set(cluster.uri, cluster);
   appContext.workspacesService.setState(draftState => {
     draftState.rootClusterUri = cluster.uri;
+    draftState.workspaces = {
+      [cluster.uri]: {
+        localClusterUri: cluster.uri,
+        documents: [],
+        location: '/docs/1234',
+        accessRequests: undefined,
+      },
+    };
   });
 
+  if (props.autoStart) {
+    appContext.workspacesService.setConnectMyComputerAutoStart(
+      cluster.uri,
+      true
+    );
+  }
+
   return (
     <MockAppContextProvider appContext={appContext}>
       <MockWorkspaceContextProvider rootClusterUri={cluster.uri}>

web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.tsx

@@ -248,7 +248,18 @@ export function DocumentConnectMyComputerStatus(
       {prettyCurrentAction.logs && <Logs logs={prettyCurrentAction.logs} />}
 
       {isAgentIncompatible ? (
-        <CompatibilityError />
+        <CompatibilityError
+          // Hide the alert if the current action has failed. downloadAgent and startAgent already
+          // return an error message related to compatibility.
+          //
+          // Basically, we have to cover two use cases:
+          //
+          // * Auto start has failed due to compatibility promise, so the downloadAgent failed with
+          // an error.
+          // * Auto start wasn't enabled, so the current action has no errors, but the user should
+          // not be able to start the agent due to compatibility issues.
+          hideAlert={!!prettyCurrentAction.error}
+        />
       ) : (
         <>
           <Text mb={4} mt={1}>

web/packages/teleterm/src/ui/ConnectMyComputer/NavigationMenu.tsx

@@ -29,8 +29,6 @@ import {
   useConnectMyComputerContext,
 } from './connectMyComputerContext';
 
-import type { AgentCompatibility } from './CompatibilityPromise';
-
 /**
  * IndicatorStatus combines a couple of different states into a single enum which dictates the
  * decorative look of NavigationMenu.
@@ -41,16 +39,11 @@ export function NavigationMenu() {
   const iconRef = useRef();
   const [isMenuOpened, setIsMenuOpened] = useState(false);
   const { documentsService, rootClusterUri } = useWorkspaceContext();
-  const {
-    isAgentConfiguredAttempt,
-    agentCompatibility,
-    currentAction,
-    canUse,
-  } = useConnectMyComputerContext();
+  const { isAgentConfiguredAttempt, currentAction, canUse } =
+    useConnectMyComputerContext();
   const indicatorStatus = getIndicatorStatus(
     currentAction,
-    isAgentConfiguredAttempt,
-    agentCompatibility
+    isAgentConfiguredAttempt
   );
 
   if (!canUse) {
@@ -119,28 +112,15 @@ export function NavigationMenu() {
 
 function getIndicatorStatus(
   currentAction: CurrentAction,
-  isAgentConfiguredAttempt: Attempt<boolean>,
-  agentCompatibility: AgentCompatibility
+  isAgentConfiguredAttempt: Attempt<boolean>
 ): IndicatorStatus {
   if (isAgentConfiguredAttempt.status === 'error') {
     return 'error';
   }
 
-  const isAgentConfigured =
-    isAgentConfiguredAttempt.status === 'success' &&
-    isAgentConfiguredAttempt.data;
-
-  if (!isAgentConfigured) {
-    return '';
-  }
-
   if (currentAction.kind === 'observe-process') {
     switch (currentAction.agentProcessState.status) {
       case 'not-started': {
-        if (agentCompatibility === 'incompatible') {
-          return 'error';
-        }
-
         return '';
       }
       case 'error': {

web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.test.tsx

@@ -33,6 +33,7 @@ import {
 import { createMockConfigService } from 'teleterm/services/config/fixtures/mocks';
 
 import {
+  AgentCompatibilityError,
   AgentProcessError,
   ConnectMyComputerContextProvider,
   useConnectMyComputerContext,
@@ -167,7 +168,7 @@ test('starting the agent flips the workspace autoStart flag to true', async () =
 
   expect(
     appContext.workspacesService.getConnectMyComputerAutoStart(rootCluster.uri)
-  ).toBeTruthy();
+  ).toBe(true);
 });
 
 test('killing the agent flips the workspace autoStart flag to false', async () => {
@@ -182,7 +183,44 @@ test('killing the agent flips the workspace autoStart flag to false', async () =
 
   expect(
     appContext.workspacesService.getConnectMyComputerAutoStart(rootCluster.uri)
-  ).toBeFalsy();
+  ).toBe(false);
+});
+
+test('failed autostart flips the workspace autoStart flag to false', async () => {
+  const { appContext, rootCluster } = getMocksWithConnectMyComputerEnabled();
+
+  let currentAgentProcessState: AgentProcessState = {
+    status: 'not-started',
+  };
+  jest
+    .spyOn(appContext.mainProcessClient, 'getAgentState')
+    .mockImplementation(() => currentAgentProcessState);
+  jest
+    .spyOn(appContext.connectMyComputerService, 'isAgentConfigFileCreated')
+    .mockResolvedValue(true);
+  jest
+    .spyOn(appContext.connectMyComputerService, 'downloadAgent')
+    .mockRejectedValue(new AgentCompatibilityError('incompatible'));
+
+  appContext.workspacesService.setConnectMyComputerAutoStart(
+    rootCluster.uri,
+    true
+  );
+
+  const { result, waitFor } = renderUseConnectMyComputerContextHook(
+    appContext,
+    rootCluster
+  );
+
+  await waitFor(
+    () =>
+      result.current.currentAction.kind === 'download' &&
+      result.current.currentAction.attempt.status === 'error'
+  );
+
+  expect(
+    appContext.workspacesService.getConnectMyComputerAutoStart(rootCluster.uri)
+  ).toBe(false);
 });
 
 test('starts the agent automatically if the workspace autoStart flag is true', async () => {
@@ -215,9 +253,10 @@ test('starts the agent automatically if the workspace autoStart flag is true', a
       return { cleanup: () => eventEmitter.off('', listener) };
     });
 
-  jest
-    .spyOn(appContext.workspacesService, 'getConnectMyComputerAutoStart')
-    .mockReturnValue(true);
+  appContext.workspacesService.setConnectMyComputerAutoStart(
+    rootCluster.uri,
+    true
+  );
 
   const { result, waitFor } = renderUseConnectMyComputerContextHook(
     appContext,

web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx

@@ -159,18 +159,27 @@ export const ConnectMyComputerContextProvider: FC<{
       }
   );
 
+  const checkCompatibility = useCallback(() => {
+    if (agentCompatibility !== 'compatible') {
+      throw new AgentCompatibilityError(agentCompatibility);
+    }
+  }, [agentCompatibility]);
+
   const [downloadAgentAttempt, downloadAgent, setDownloadAgentAttempt] =
     useAsync(
       useCallback(async () => {
         setCurrentActionKind('download');
+        checkCompatibility();
         await connectMyComputerService.downloadAgent();
-      }, [connectMyComputerService])
+      }, [connectMyComputerService, checkCompatibility])
     );
 
   const [startAgentAttempt, startAgent] = useAsync(
     useCallback(async () => {
       setCurrentActionKind('start');
 
+      checkCompatibility();
+
       await connectMyComputerService.runAgent(rootClusterUri);
 
       const abortController = createAbortController();
@@ -207,15 +216,19 @@ export const ConnectMyComputerContextProvider: FC<{
       rootClusterUri,
       usageService,
       workspacesService,
+      checkCompatibility,
     ])
   );
 
   const downloadAndStartAgent = useCallback(async () => {
-    const [, error] = await downloadAgent();
+    let [, error] = await downloadAgent();
     if (error) {
-      return;
+      throw error;
+    }
+    [, error] = await startAgent();
+    if (error) {
+      throw error;
     }
-    await startAgent();
   }, [downloadAgent, startAgent]);
 
   const [killAgentAttempt, killAgent] = useAsync(
@@ -355,17 +368,32 @@ export const ConnectMyComputerContextProvider: FC<{
   const agentIsNotStarted =
     currentAction.kind === 'observe-process' &&
     currentAction.agentProcessState.status === 'not-started';
-  const isAgentCompatible = agentCompatibility === 'compatible';
+  const isAgentCompatibilityKnown = agentCompatibility !== 'unknown';
 
   useEffect(() => {
     const shouldAutoStartAgent =
       isAgentConfigured &&
       canUse &&
-      isAgentCompatible &&
+      // Agent compatibility is known only after we fetch full cluster details, so we have to wait
+      // for that until we attempt to autostart the agent. Otherwise startAgent would return an
+      // error.
+      isAgentCompatibilityKnown &&
       workspacesService.getConnectMyComputerAutoStart(rootClusterUri) &&
       agentIsNotStarted;
+
     if (shouldAutoStartAgent) {
-      downloadAndStartAgent();
+      (async () => {
+        try {
+          await downloadAndStartAgent();
+        } catch (error) {
+          // Turn off autostart if it fails, otherwise the user wouldn't be able to turn it off by
+          // themselves.
+          workspacesService.setConnectMyComputerAutoStart(
+            rootClusterUri,
+            false
+          );
+        }
+      })();
     }
   }, [
     canUse,
@@ -374,7 +402,7 @@ export const ConnectMyComputerContextProvider: FC<{
     isAgentConfigured,
     rootClusterUri,
     workspacesService,
-    isAgentCompatible,
+    isAgentCompatibilityKnown,
   ]);
 
   return (
@@ -468,6 +496,33 @@ export class NodeWaitJoinTimeout extends Error {
   }
 }
 
+export class AgentCompatibilityError extends Error {
+  constructor(
+    public readonly agentCompatibility: Exclude<
+      AgentCompatibility,
+      'compatible'
+    >
+  ) {
+    let message: string;
+    switch (agentCompatibility) {
+      case 'incompatible': {
+        message =
+          'The agent version is not compatible with the cluster version';
+        break;
+      }
+      case 'unknown': {
+        message = 'The compatibility of the agent could not be established';
+        break;
+      }
+      default: {
+        throw assertUnreachable(agentCompatibility);
+      }
+    }
+    super(message);
+    this.name = 'AgentCompatibilityError';
+  }
+}
+
 /**
  * wait is like wait from the shared package, but it works with TshAbortSignal.
  * TODO(ravicious): Refactor TshAbortSignal so that its interface is the same as AbortSignal.

web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx

@@ -93,9 +93,10 @@ export function DocumentsRenderer(props: {
               )}
               {workspace.rootClusterUri ===
                 workspacesService.getRootClusterUri() &&
+                props.topBarContainerRef.current &&
                 createPortal(
                   <ConnectMyComputerNavigationMenu />,
-                  props.topBarContainerRef?.current
+                  props.topBarContainerRef.current
                 )}
             </ConnectMyComputerContextProvider>
           </WorkspaceContextProvider>