🐞 Windmill and Harvest fail to start on production deployments with AWS S3

[Findeton]

Parent issue: https://github.com/sequentech/meta/issues/11570

[Copilot]

Pull request overview

Fixes production startup failures for Windmill/Harvest when using AWS S3 by making plugin loading and S3 listing compatible with bucket-hosted S3 endpoints, and by surfacing initialization failures instead of silently swallowing them.

Changes:

• Propagate PluginManager initialization/loading errors with proper anyhow::Context instead of ignoring them.
• Add S3 endpoint “bucket-hosted URL” parsing + bucket/prefix rewriting so list-based operations (plugin discovery, bulk deletes, exports) work against AWS S3.
• Improve Celery app initialization failure visibility by logging the error before panicking.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
packages/windmill/src/services/plugins_manager/plugin_manager.rs	Avoids swallowing plugin manager init/load failures; modernizes Option handling.
packages/windmill/src/services/celery_app.rs	Logs Celery initialization errors before panicking to aid diagnosis in production.
packages/sequent-core/src/util/aws.rs	Introduces named env constants and clarifies the S3 config selection flag.
packages/sequent-core/src/services/s3.rs	Implements AWS bucket-hosted endpoint rewriting for list operations and adds focused unit tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

This PR addresses production startup failures for Windmill/Harvest deployments using AWS S3 by improving initialization error propagation and making S3 “list-style” operations work with AWS bucket-hosted endpoint URIs.

Changes:

• Propagate PluginManager initialization/load failures (instead of silently ignoring them) with clearer error context.
• Improve Celery app initialization failures by logging the full error before panicking.
• Refactor S3 list/delete helpers to correctly handle AWS bucket-hosted endpoints by rewriting the endpoint + mapping “logical bucket” to a key prefix; adds unit tests for the endpoint parsing/rewriting logic.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
packages/windmill/src/services/plugins_manager/plugin_manager.rs	Improves plugin loading flow and ensures plugin-manager initialization errors are not swallowed.
packages/windmill/src/services/celery_app.rs	Logs Celery app init errors before panicking during singleton initialization.
packages/sequent-core/src/util/aws.rs	Introduces env-var constants and updates AWS/S3 config helpers/docstrings.
packages/sequent-core/src/services/s3.rs	Adds AWS bucket-hosted endpoint parsing + list target resolution; updates list/delete code paths and adds tests.

Comments suppressed due to low confidence (2)

packages/windmill/src/services/plugins_manager/plugin_manager.rs:235

• get_plugin_manager now propagates errors from init_plugin_manager(). Because init_plugin_manager() uses a non-atomic get() check followed by set(), concurrent calls can race: one task may successfully set, while the other fails with "PluginManager already initialized" and get_plugin_manager returns an error even though the singleton is ready. Consider using an async-aware tokio::sync::OnceCell::get_or_try_init (or similar) or treating the "already initialized" set failure as success and then returning PLUGIN_MANAGER.get().unwrap().

pub async fn get_plugin_manager() -> Result<&'static PluginManager> {
    let plugin_manager = match PLUGIN_MANAGER.get() {
        Some(manager) => manager,
        _ => {
            init_plugin_manager()
                .await
                .context("Failed to initialize PluginManager")?;
            PLUGIN_MANAGER
                .get()
                .expect("PluginManager should be initialized")
        }

packages/sequent-core/src/util/aws.rs:20

• The new docstring for get_region says it "keeps the default chain as a fallback", but the implementation still hard-errors if AWS_REGION is missing (std::env::var("AWS_REGION")?). Either update the docs to reflect that AWS_REGION is required, or change the implementation to truly fall back (e.g., avoid erroring on missing AWS_REGION and let RegionProviderChain::default_provider() / or_else handle it).

/// Resolves the AWS region from the environment and keeps the default chain
/// as a fallback so local and deployed runtimes share the same lookup flow.
pub fn get_region() -> Result<RegionProviderChain> {
    let region = RegionProviderChain::first_try(Region::new(
        std::env::var("AWS_REGION")
            .map_err(|err| anyhow!("AWS_REGION env var missing: {err}"))?,
    ))
    .or_default_provider()
    .or_else(Region::new("us-east-1"));
    Ok(region)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.