agent-config: allow providing config in clients

[mschuwalow]

resolves #2952

Also cleans up the wit a bit to be consistent with the naming in other parts of the code

[vigoo]

Are you going to apply the same changes to the client constructors in the generated bridges separately?

[mschuwalow]

Yes, the bridge sdks

Are you going to apply the same changes to the client constructors in the generated bridges separately?

The generated bridges currently only allow invoking, right? The local config can only be provided when explicitly creating the agent (same as args, env, wasi:config), not when invoking. So I guess we should do one of these things (not just for the agent config, but for the other ones as well).

1. Optionally allow them to be passed to each invocation
2. Expose an explicit create command in the bridges that allows starting an agent (not just invoking it)

TLDR: no, didn't plan for exposing it in the bridges currently

[vigoo]

The bridges SDKs are having the same API as the RPC. So if we add a new parameter to the RPC constructors here, we should do the same in the bridge SDKs.

[vigoo]

My biggest concern (maybe misundestanding the changes):

this adds each config parameter as a parameter to the generated RPC constructor methods (get, etc)? I think by default we should not pass them and let it set from the agent/environment/etc levels.

Maybe there should be separate variants of the constructor methods that take custom config? (This also relates to my previous comments about being in sync with the bridge api - if we have separate new constructors, then it's not breaking the parity, just a new thing to be implemented in the bridge)

[mschuwalow]

The get methods in the sdk take a generated version of the config with the following rules applied:

• all leaf fields made optional
• all secrets removed

e.g. the following configuration

#[derive(ConfigSchema)]
pub struct NestedConfig {
    #[config_schema(secret)]
    pub nested_secret: Secret<i32>,
    pub a: bool,
    pub b: Vec<i32>,
}

#[derive(ConfigSchema, Serialize)]
pub struct AliasedNestedConfig {
    #[serde(skip_serializing_if = "Option::is_none")]
    pub c: Option<i32>
}

#[derive(ConfigSchema)]
pub struct ConfigAgentConfig {
  pub foo: i32,
  pub bar: String,
  #[config_schema(secret)]
  pub secret: Secret<String>,
  #[config_schema(nested)]
  pub nested: NestedConfig,
  #[config_schema(nested)]
  pub aliased_nested: AliasedNestedConfig,
}

Will generate the following type for getting / rpc

#[derive(Default)]
pub struct ConfigAgentConfigRpc {
    pub foo: ::std::option::Option<i32>,
    pub bar: ::std::option::Option<String>,
    pub nested: <NestedConfig as golem_rust::agentic::ConfigSchema>::RpcType,
    pub aliased_nested: <AliasedNestedConfig as golem_rust::agentic::ConfigSchema>::RpcType,
}

This allows us to just use Default::default in rust and {} in js to create an agent with the default configuration. To me it feels ergonomic enough, but I can also introduce separate versions.
But we would also need a version for get_phantom, new_phantom if we go this way.

[mschuwalow]

Followup to☝️

Example of usage:

#[agent_implementation]
impl RpcLocalConfigAgent for RpcLocalConfigAgentImpl {
    fn new(name: String, #[agent_config] config: Config<RpcLocalConfigAgentConfig>) -> Self {
        Self { name, config }
    }

    async fn echo_local_config(&self) -> String {
        let config = self.config.get();
        let client = LocalConfigAgentClient::get(
            self.name.clone(),
            LocalConfigAgentConfigRpc {
                foo: Some(config.foo.clone()),
                nested: NestedLocalAgentConfigRpc {
                    a: config.nested_a,
                    ..Default::default()
                },
                ..Default::default()
            }
        );
        client.echo_local_config().await
    }
}

[mschuwalow]

Tests failing due to #3000

[vigoo]

I think it is important that the get (and phantom friends) constructors remain as they are - only getting the constructor parameters, corresponding to the agent identity. That is the parameter list you see when you list / examine agents in the CLI or any future UI. This way the parameter list in the code / in the repl corresponds (almost 100%) to these rendered IDs.

Per-agent config overrides are a very special case I think and we should not sacrifice the easy of use of the generated clients and REPL (note that as I wrote above, the REPL - that builds on the generated bridges - should be always in sync with the clients, providing the same syntax!).

So I think we should have special variants for config override (get_with_config etc) and have them both here and in the SDK (and I think it is a good idea to change hte bridge client constructors to actually create the worker - that will make the REPL .get call have the side-effect of upserting the agent, which I always missed)