[flow] Back out D104362011 (restart server on flowconfig/package.json changes)

Summary:
D104362011 changed the monitor's response to `Flowconfig_changed` from "exit along with the server" to "respawn the server in-place". The respawned server comes up in lazy mode, so a subsequent CI `flow status --lazy-mode none` connects to it and never runs a full check — D105744590 (`[react-native-view-shot] Upgrade to 5.1.0`) landed this way.

Back out the entirety of D104362011 except for the `experimental.restart_on_flowconfig_change` config option, which is kept as a parsed-but-inert option so flowconfigs that set it (per D105897416) continue to validate after this binary is released.

Changelog: [internal]

Reviewed By: SamChou19815

Differential Revision: D105899555

fbshipit-source-id: 9d3dfc828a92da4253f868a7a5497189cff69bec

Author: panagosg7

rust_port/crates/flow_cli/src/command_utils.rs

@@ -2109,7 +2109,6 @@ pub(super) fn make_options(
                 relay_integration_excludes,
                 relay_integration_module_prefix,
                 relay_integration_module_prefix_includes,
-                restart_on_flowconfig_change,
                 root_name,
                 saved_state_direct_serialization,
                 saved_state_parallel_decompress,
@@ -2755,7 +2754,6 @@ pub(super) fn make_options(
         relay_integration_excludes,
         relay_integration_module_prefix: relay_integration_module_prefix.map(FlowSmolStr::new),
         relay_integration_module_prefix_includes,
-        restart_on_flowconfig_change,
         root: Arc::new(root),
         root_name: root_name.map(FlowSmolStr::new),
         saved_state_direct_serialization,

rust_port/crates/flow_common/src/options.rs

@@ -248,7 +248,6 @@ pub struct Options {
     pub relay_integration_excludes: Arc<[Regex]>,
     pub relay_integration_module_prefix: Option<FlowSmolStr>,
     pub relay_integration_module_prefix_includes: Arc<[Regex]>,
-    pub restart_on_flowconfig_change: bool,
     pub root: Arc<PathBuf>,
     pub root_name: Option<FlowSmolStr>,
     pub saved_state_direct_serialization: bool,

rust_port/crates/flow_common_exit_status/src/lib.rs

@@ -33,7 +33,7 @@ pub enum FlowExitStatus {
     NoServerRunning,
     // Ran out of retries
     OutOfRetries,
-    // Invalid .flowconfig (parse error, version mismatch). Fatal — monitor exits.
+    // Invalid .flowconfig
     InvalidFlowconfig,
     // Provided path is not a file as required
     PathIsNotAFile,
@@ -52,7 +52,7 @@ pub enum FlowExitStatus {
     ServerOutOfDate,
     // When the shared memory is missing space (e.g. full /dev/shm)
     OutOfSharedMemory,
-    // The .flowconfig or package.json changed at runtime. Monitor restarts the server.
+    // The .flowconfig has changed and we're out of date
     FlowconfigChanged,
     // Failed to parse the command line or misuse of command line arguments
     CommandlineUsageError,

rust_port/crates/flow_config/src/flowconfig.rs

@@ -185,7 +185,6 @@ pub mod opts {
         pub relay_integration_excludes: Vec<String>,
         pub relay_integration_module_prefix: Option<String>,
         pub relay_integration_module_prefix_includes: Vec<String>,
-        pub restart_on_flowconfig_change: bool,
         pub root_name: Option<String>,
         pub saved_state_direct_serialization: bool,
         pub saved_state_parallel_decompress: bool,
@@ -348,7 +347,6 @@ pub mod opts {
             relay_integration_module_prefix_includes: vec![ocaml_str_to_rust_regex(
                 "<PROJECT_ROOT>/.*",
             )],
-            restart_on_flowconfig_change: true,
             root_name: None,
             saved_state_direct_serialization: false,
             saved_state_parallel_decompress: false,
@@ -2179,6 +2177,7 @@ pub mod opts {
             "experimental.projects_path_mapping",
             "experimental.records",
             "experimental.records.includes",
+            "experimental.restart_on_flowconfig_change",
             "experimental.strict_es6_import_export",
             "experimental.ts_syntax",
             "experimental.allow_variance_keywords",
@@ -2549,14 +2548,12 @@ pub mod opts {
                         config,
                     ))
                 }
-                "experimental.restart_on_flowconfig_change" => Some(parse_boolean(
-                    |opts, v| {
-                        opts.restart_on_flowconfig_change = v;
-                        Ok(())
-                    },
-                    values,
-                    config,
-                )),
+                // Parsed for backwards compatibility with flowconfigs that still set
+                // `experimental.restart_on_flowconfig_change`. Has no runtime effect;
+                // kept so those flowconfigs do not error out.
+                "experimental.restart_on_flowconfig_change" => {
+                    Some(parse_boolean(|_, _| Ok(()), values, config))
+                }
                 "experimental.strict_es6_import_export" => {
                     Some(strict_es6_import_export_parser(values, config))
                 }

rust_port/crates/flow_server_monitor/src/flow_server_monitor_server.rs

@@ -996,8 +996,6 @@ mod keep_alive_loop {
         if monitor_options.no_restart {
             return (true, None, false);
         }
-        let restart_on_flowconfig_change =
-            monitor_options.server_options.restart_on_flowconfig_change;
         use flow_common_exit_status::FlowExitStatus;
         match exit_status {
             //*** Things the server might exit with that implies that the monitor should exit too ***
@@ -1009,6 +1007,8 @@ mod keep_alive_loop {
             // Parse/version/etc error. Server will never start correctly.
             | FlowExitStatus::PathIsNotAFile
             // Required a file but privided path was not a file
+            | FlowExitStatus::FlowconfigChanged
+            // We could survive some config changes, but it's too hard to tell
             | FlowExitStatus::InvalidSavedState
             // The saved state file won't automatically recover by restarting
             | FlowExitStatus::UnusedServer
@@ -1055,13 +1055,7 @@ mod keep_alive_loop {
                 false,
             ),
             FlowExitStatus::KilledByMonitor /* The server died because we asked it to die */ => (false, None, false),
-            FlowExitStatus::FlowconfigChanged if !restart_on_flowconfig_change => {
-                // Killswitch: revert to the legacy behavior where the monitor exits on
-                // flowconfig/package.json changes. Remove once the new behavior is fully rolled out.
-                (true, None, false)
-            }
-            FlowExitStatus::Restart /* The server asked to be restarted */
-            | FlowExitStatus::FlowconfigChanged /* .flowconfig or package.json changed; restart with new config */ => (
+            FlowExitStatus::Restart /* The server asked to be restarted */ => (
                 false,
                 Some(flow_server_env::server_status::RestartReason::Restart),
                 false,
@@ -1095,12 +1089,10 @@ mod keep_alive_loop {
             let msg = flow_server_env::lsp_prot::MessageFromServer::NotificationFromServer(
                 flow_server_env::lsp_prot::NotificationFromServer::ServerExit(exit_type),
             );
-            // Atomically write the ServerExit notification then close. Going through
-            // `WriteAndClose` keeps wire ordering correct: the command loop drains the
-            // write before the close fires. Using `write` + `try_flush_and_close` would
-            // race — `try_flush_and_close` aborts the read loop, whose catch handler
-            // closes the socket before the queued write reaches the wire.
-            conn.write_and_close(msg);
+            // it's ok if the stream is already closed, we must be shutting down already
+            conn.write(msg);
+            // it's also ok if the flush fails because the socket is already closed
+            conn.try_flush_and_close();
         }
     }
 

rust_port/crates/flow_server_monitor/src/socket_acceptor.rs

@@ -196,22 +196,8 @@ fn create_persistent_connection(
 
     Server::notify_new_persistent_connection(client_id, lsp_init_params);
 
-    // Hold a clone of the underlying TCP stream so the close callback can
-    // shut it down. The outer `close` from `socket_acceptor_loop` was disarmed
-    // when we accepted the persistent client, so on its own it no longer
-    // closes the wire — and the LSP would never see EOF. Shutting down here
-    // sends FIN to the LSP, which is what triggers its disconnect handling.
-    let shutdown_slot = std::sync::Arc::new(std::sync::Mutex::new(Some(
-        client_stream
-            .try_clone()
-            .expect("clone of TcpStream for persistent shutdown failed"),
-    )));
-
     let outer_close = close;
     let close: Arc<dyn Fn() + Send + Sync> = Arc::new(move || {
-        if let Some(s) = shutdown_slot.lock().unwrap().take() {
-            self::close(s);
-        }
         Server::notify_dead_persistent_connection(client_id);
         outer_close();
     });

rust_port/crates/flow_server_rechecker/src/recheck_updates.rs

@@ -91,10 +91,7 @@ fn get_updated_flowconfig(config_path: &str) -> Result<(FlowConfig, String), Err
     }
 }
 
-fn assert_compatible_flowconfig_version(
-    options: &Options,
-    config: &FlowConfig,
-) -> Result<(), Error> {
+fn assert_compatible_flowconfig_version(config: &FlowConfig) -> Result<(), Error> {
     let not_satisfied = |version_constraint: &str| -> bool {
         match semver::satisfies(Some(true), version_constraint, flow_version::version()) {
             Ok(result) => !result,
@@ -108,14 +105,10 @@ fn assert_compatible_flowconfig_version(
                 version_constraint,
                 flow_version::version(),
             );
-            // Killswitch: under the legacy behavior, version mismatches surfaced as
-            // FlowconfigChanged (exit code 16). Remove once the new behavior is fully rolled out.
-            let exit_status = if options.restart_on_flowconfig_change {
-                FlowExitStatus::InvalidFlowconfig
-            } else {
-                FlowExitStatus::FlowconfigChanged
-            };
-            Err(Error::Unrecoverable { msg, exit_status })
+            Err(Error::Unrecoverable {
+                msg,
+                exit_status: FlowExitStatus::FlowconfigChanged,
+            })
         }
         _ => Ok(()),
     }
@@ -131,7 +124,7 @@ fn assert_compatible_flowconfig_change(options: &Options, config_path: &str) ->
             ".flowconfig changed (hash {} -> {}); server must restart",
             old_hash, new_hash
         );
-        assert_compatible_flowconfig_version(options, &new_config)?;
+        assert_compatible_flowconfig_version(&new_config)?;
         Err(Error::Unrecoverable {
             msg,
             exit_status: FlowExitStatus::FlowconfigChanged,

src/commands/commandUtils.ml

@@ -1689,7 +1689,6 @@ let make_options
         ~f:(fun pattern ->
           pattern |> Files.expand_project_root_token_as_absolute ~root |> Str.regexp)
         (FlowConfig.relay_integration_module_prefix_includes flowconfig);
-    opt_restart_on_flowconfig_change = FlowConfig.restart_on_flowconfig_change flowconfig;
     opt_root = root;
     opt_root_name = FlowConfig.root_name flowconfig;
     opt_saved_state_fetcher;

src/commands/config/flowConfig.ml

@@ -153,7 +153,6 @@ module Opts = struct
     relay_integration_excludes: string list;
     relay_integration_module_prefix: string option;
     relay_integration_module_prefix_includes: string list;
-    restart_on_flowconfig_change: bool;
     root_name: string option;
     saved_state_fetcher: Options.saved_state_fetcher;
     saved_state_direct_serialization: bool;
@@ -324,7 +323,6 @@ module Opts = struct
       relay_integration_excludes = [];
       relay_integration_module_prefix = None;
       relay_integration_module_prefix_includes = ["<PROJECT_ROOT>/.*"];
-      restart_on_flowconfig_change = true;
       root_name = None;
       saved_state_fetcher = Options.Dummy_fetcher;
       saved_state_direct_serialization = false;
@@ -1240,9 +1238,10 @@ module Opts = struct
           ~multiple:true
           (fun opts v -> Ok { opts with records_includes = v :: opts.records_includes })
       );
-      ( "experimental.restart_on_flowconfig_change",
-        boolean (fun opts v -> Ok { opts with restart_on_flowconfig_change = v })
-      );
+      (* Parsed for backwards compatibility with flowconfigs that still set
+         [experimental.restart_on_flowconfig_change]. Has no runtime effect;
+         kept so those flowconfigs do not error out. *)
+      ("experimental.restart_on_flowconfig_change", boolean (fun opts _ -> Ok opts));
       ("experimental.strict_es6_import_export", strict_es6_import_export_parser);
       ("experimental.ts_syntax", boolean (fun opts v -> Ok { opts with ts_syntax = v }));
       ( "experimental.allow_variance_keywords",
@@ -2245,8 +2244,6 @@ let relay_integration_module_prefix_includes c =
 
 let required_version c = c.version
 
-let restart_on_flowconfig_change c = c.options.Opts.restart_on_flowconfig_change
-
 let root_name c = c.options.Opts.root_name
 
 let saved_state_fetcher c = c.options.Opts.saved_state_fetcher

src/commands/config/flowConfig.mli

@@ -287,8 +287,6 @@ val relay_integration_module_prefix_includes : config -> string list
 
 val required_version : config -> string option
 
-val restart_on_flowconfig_change : config -> bool
-
 val root_name : config -> string option
 
 val saved_state_fetcher : config -> Options.saved_state_fetcher