Add supervised restart support to Gateway backend by awheeler294 · Pull Request #150 · cppalliance/runvil

awheeler294 · 2026-10-15T22:33:54Z

This is the second part of a series of improvements to config handling in the Gateway, the full plan is in vibe/2026-10-06-1-restart-to-apply.md

Add Supervised Restart Support to the Gateway Backend

Adds a support for the Gateway to restart itself in response to an http request. Gateway has a number of settings that require a restart after they're changed, this allows the admin UI to request that restart.
Adds a --supervised command line option which will be used to run the Gateway as a process under a lightweight supervisor executable that will be responsible for restart logic.
Defines a restart_requested signal and exit code and adds the necessary plumbing
Adds rollback config files so that on a restart a bad config can be rolled back automatically and won't prevent the Gateway (and its settings UI) from loading.

note: There are a couple large additions/deletions that are a result of splitting files, this repo enforces a 500 line limit on source code files. I'll try to point out where the actual changes in those files are.

awheeler294 · 2026-10-15T22:39:37Z

Most of this file is existing tests moved out of ‎crates/gateway/app/src/main-tests.rs‎.

The new tests start at supervised_option_parses_and_reaches_the_serve_options

vinniefalco · 2026-10-15T23:37:06Z

At first glance this looks solid. Plan is clear on what it does.

seanpar203 · 2026-10-16T12:45:28Z

 /// then one line per change, heartbeats in the gaps, until `shutdown`
 /// fires.
-fn progress_sse_response(hub: &ProgressHub, shutdown: shutdown::ShutdownSignal) -> Response {
+fn progress_sse_response(hub: &ProgressHub, shutdown: Arc<shutdown::ShutdownSignal>) -> Response {


Is there a reason we had to move this over to using an Arc?

Is there one created on server start and passed down into all callers type of thing?

Yes, this is created on boot and them passed to multiple threads. Personally I think an mpsc::channel would make more sense here, but I didn't want to re-work the existing shutdown code (yet).

seanpar203 · 2026-10-16T13:01:57Z

                files,
                restart_required,
            } => {
-                let applied = blocking(move || promote_captures(&files)).await??;
+                let applied = blocking(move || {
+                    write_rollback_files(&files)?;
+                    promote_captures(&files)
+                })
+                .await??;
                return Ok(Json(ApplyReply {
                    applied,
                    reloaded: false,
                    restart_required,
                }));
            }
-            ApplyPlan::Reload(snapshot) => snapshot,
+            ApplyPlan::Reload(snapshot) => {
+                let captures = snapshot.captures().to_vec();
+                blocking(move || write_rollback_files(&captures)).await??;
+                snapshot
+            }
        };
        let applied = snapshot.applied_names();
        let restart_required = snapshot.restart_required;


This code block should be a re-usable function that accepts state.

I know it's something you just added to and didn't create yourself.

It would make that entire code block easier to test independent of the remaining code in the admin_config_apply function.

let config_path = crate::admin::config_path(&state)?.to_path_buf(); let (enqueued, applied, restart_required) = { // The lock spans the census, the parse, and the capture (or the // inline promotion), so a save cannot land between them and the // snapshot is one the latest save validated whole. It is released // before the command runs: the queue serializes the reload itself. let _guard = state.apply.lock().await; let plan = blocking(move || capture_apply(&config_path)).await??; let snapshot = match plan { ApplyPlan::Inline { files, restart_required, } => { let applied = blocking(move || promote_captures(&files)).await??; return Ok(Json(ApplyReply { applied, reloaded: false, restart_required, })); } ApplyPlan::Reload(snapshot) => snapshot, }; let restart_required = snapshot.restart_required; let enqueued = state.commands.enqueue(Command::ApplyConfig { snapshot, token: CancellationToken::new(), }); (enqueued, snapshot.applied_names(), restart_required) };

That's not a bad idea, but I think to make that a useful change we would also have to write the tests, and I think it should be a separate PR

seanpar203 · 2026-10-16T13:03:26Z

 /// files relative to the config root, sorted.
 fn delete_all_shadows(config_path: &Path) -> Result<Vec<String>, GatewayError> {
-    let census = shadow_census(config_path)?;
+    let census = ShadowCensus::new(config_path)?;


Nice idiomatic change using ::new method on the struct itself instead of some kind of helper function constructor.

wtf is a ShadowCensus lol

Was written by you @vinniefalco !

pub(crate) struct ShadowCensus { /// Real files whose shadows exist, in canonical form, without duplicates. pub(crate) files: Vec<PathBuf>, /// Top-level sections whose merged value the shadows change, sorted and deduplicated. pub(crate) sections: Vec<String>, }

Looks like it's related to the configuration files and the individual sections.

Probably not the best name though.

Remind me of a Shadow Council.

something about lizard people controlling the government...

I agree it's a terrible name. It tracks changes that are being written to the .next config files, which the codebase refers to as shadow files.

seanpar203 · 2026-10-16T13:08:21Z

+) -> Result<StatusCode, GatewayError> {
+    if !state.supervised {
+        return Err(GatewayError::RestartUnavailable);
+    }


Ahh so the supervised field is used here.

seanpar203 · 2026-10-16T13:09:37Z

+
+    // As for `POST /shutdown`: cancel the active queue command so the
+    // drain, and the exit, stay prompt.
+    state.commands.cancel_active();


Does this interrupt active processes and forces a shutdown or is it graceful?

It starts a graceful shutdown

seanpar203 · 2026-10-16T13:20:35Z

@@ -22,8 +22,8 @@ impl GatewayError {
    pub(crate) fn switch_failed(
        stage: &'static str,
        source: impl std::error::Error + Send + Sync + 'static,
-    ) -> GatewayError {
-        GatewayError::SwitchFailed {
+    ) -> Self {
+        Self::SwitchFailed {
            stage,
            source: Box::new(source),
        }
@@ -32,17 +32,15 @@ impl GatewayError {
    /// Wraps a cache-operation failure, preserving the cause.
    #[cfg(feature = "local")]
    #[must_use]
-    pub(crate) fn cache(source: impl std::error::Error + Send + Sync + 'static) -> GatewayError {
-        GatewayError::Cache(Box::new(source))
+    pub(crate) fn cache(source: impl std::error::Error + Send + Sync + 'static) -> Self {
+        Self::Cache(Box::new(source))
    }

    /// Wraps a model-info read or parse failure, preserving the cause.
    #[cfg(feature = "local")]
    #[must_use]
-    pub(crate) fn model_info(
-        source: impl std::error::Error + Send + Sync + 'static,
-    ) -> GatewayError {
-        GatewayError::ModelInfo(Box::new(source))
+    pub(crate) fn model_info(source: impl std::error::Error + Send + Sync + 'static) -> Self {
+        Self::ModelInfo(Box::new(source))


I find this entire classify.rs file strange.

From a readability perspective, it seems like the implementation of a concrete struct should be in the same file as the struct itself.

GatewayError is defined in error.rs and then there's an error module that has classify which implements it.

I don't know.

And theres no other error structs in the error.rs .

I like the idea of there being a delineation and separation for each error:

/errors gateway.rs supervisor.rs cli.rs

Nothing to hold up this PR, but maybe something we want to revisit.

Yeah, I agree it's awkward and needs to be revisited.

seanpar203 · 2026-10-16T13:22:06Z

    /// Returns [`StartupError`] when serving failed or the gateway thread
    /// panicked.
-    pub fn shutdown(mut self) -> Result<(), StartupError> {
+    pub fn shutdown(mut self) -> Result<ShutdownReason, StartupError> {


Nice addition here.

seanpar203 · 2026-10-16T13:25:34Z

+pub(crate) fn parse_args(
+    args: impl IntoIterator<Item = OsString>,
+) -> Result<Invocation, ParseError> {
+    let mut args = args.into_iter();
+    let _binary = args.next();
+
+    // `diagnostics` is the only subcommand and must come first.
+    let mut args = args.peekable();
+    if args.peek().and_then(|arg| arg.to_str()) == Some("diagnostics") {
+        args.next();
+        return parse_diagnostics_args(args);
+    }
+
+    let mut profile: Option<ProfileName> = None;
+    let mut config_path: Option<PathBuf> = None;
+    let mut tray = true;
+    let mut login = false;
+    let mut print_url = false;
+    let mut browser = false;
+    let mut supervised = false;
+
+    while let Some(arg) = args.next() {
+        match arg.to_str() {
+            Some("--config") => {
+                let path = args
+                    .next()
+                    .ok_or_else(|| ParseError::Usage("--config requires a path".to_string()))?;
+                if config_path.is_some() {
+                    return Err(ParseError::Usage("--config accepts one path".to_string()));
+                }
+                config_path = Some(PathBuf::from(path));
+            }
+            Some("--profile") => {
+                let name = args
+                    .next()
+                    .ok_or_else(|| ParseError::Usage("--profile requires a name".to_string()))?;
+                let name = name.into_string().map_err(|_| {
+                    ParseError::Usage("--profile name must be valid UTF-8".to_string())
+                })?;
+                let name = ProfileName::parse(&name)
+                    .map_err(|error| ParseError::Usage(format!("invalid profile name: {error}")))?;
+                profile = Some(name);
+            }
+            Some("--no-tray") => tray = false,
+            Some("--login") => login = true,
+            Some("--print-url") => print_url = true,
+            Some("--browser") => browser = true,
+            Some("--supervised") => supervised = true,
+            Some("-h" | "--help") => return Err(ParseError::Help),
+            Some("--version") => return Err(ParseError::Version),
+            Some(other) if other.starts_with('-') => {
+                return Err(ParseError::Usage(format!("unknown flag {other}")));
+            }
+            _ => {
+                return Err(ParseError::Usage(format!(
+                    "unexpected argument {}",
+                    arg.to_string_lossy()
+                )));
+            }
+        }
+    }


Does Rust not have CLI framework tools to help with this?

Some sugar tools to make it a little bit easier to parse.

Like clap.

Just to make this a little more maintainable long term as our CLI requirements grow over time.

Yes, I would very much like to replace all this with clap, and it's on my todo list. I just decided this PR was already big enough :)

awheeler294 requested a review from seanpar203 October 15, 2026 22:33

awheeler294 added the Gateway A separate product used by PromptForge label Oct 15, 2026