feat!: agent module registration api

[TimoGlastra]

Signed-off-by: Timo Glastra [email protected]

Add agent module registration api. With this you can now register custom modules on the agent like this:

const agent = new Agent({
  config: { /* agent config */ },
  modules: {
    tenants: new TenantsModule(),
  },
  dependencies: agentDependencies,
})

This will register the tenants module and make the tenants api available. There's some nice typescript magic going on here that automatically infers the api type from the tenants module, allowing the agent to be fully typed with custom modules:

// agent.api.tenants is fully typed
const tenantAgent = await agent.api.tenants.getTenantAgent({ tenantId: 'the-tenant-id' })

// tenantAgent is also fully typed, but doesn't have the `agent.api.tenants` available
await tenantAgent.api.oob.createInvitation()

As you may have already noticed from the example above, all the module api's are now nested under the agent.api property. I've taken this approach for two reasons:

1. If we define them top level there's risk of clashing, and it's less clear what on the agent is a module and what is an agent property. E.g. if I add a module under the config key it wouldn't work because we already have a config property on the agent. We could work around this by disallowing properties defined on the agent, but that means everytime we add a property it means it could break an existing setup (e.g. we add agent.status to get the agent status, but someone wrote their own status module).
2. We can't dynamically modify the return type of a constructor parameter. We could work around this by using e.g. agent.create() instead of new Agent however.

You can define the api for a module using the api key on a module:

class MyModule {
  public api = MyApi

  public register(dependencyManager: DependencyManager) {
    dependencyManager.registerContextScoped(MyApi)
  }
}

I'm not particularly happy with the extra nesting under the api key, so I'm interested to hear opinions on this. We could stick with the top level api modules, we could also change the key to e.g. .m for module or .a for api, but to make it shorter.

All modules that were registered by default on the agent are still registered by default. So agent.api.oob is present, as well as agent.api.connections, etc... For now the default modules are still initialized based on the configuration in the agent config, but in a next PR I'll remove that logic and you will only be able to configure modules using the module config itself. If you want to provide custom configuration for a default module you can do so by registering the module. This will just register the module with you custom config, instead of using the default config:

const agent = new Agent({
  config: { /* agent config */ },
  modules: {
    // provide custom config for the default registered connections module.
    connections: new ConnectionsModule({
        autoAcceptConnections: true
    })
  },
  dependencies: agentDependencies,
})

---

BREAKING CHANGE: all modules have been moved to the .api namespace. Instead of using agent.xxx (e.g. agent.oob) you should now call agent.api.oob instead. In addition the agent constructor has been updated to a single options object that contains the config and dependencies properties. Instead of constructing the agent like this:

const agent = new Agent({ /* config */ }, agentDependencies)

You should now construct it like this:

const agent = new Agent({
  config: { /* config */ },
  dependencies: agentDependencies
})

This allows for the new custom modules to be defined in the agent constructor.

[codecov-commenter]

Codecov Report

Merging #955 (d6d524f) into 0.3.0-pre (273e353) will increase coverage by 0.03%. The diff coverage is 99.21%.

@@              Coverage Diff              @@
##           0.3.0-pre     #955      +/-   ##
=============================================
+ Coverage      88.20%   88.23%   +0.03%     
=============================================
  Files            636      637       +1     
  Lines          15036    15092      +56     
  Branches        2536     2542       +6     
=============================================
+ Hits           13262    13316      +54     
- Misses          1676     1678       +2     
  Partials          98       98              

Impacted Files	Coverage Δ
packages/core/src/index.ts	100.00% <ø> (ø)
packages/core/src/modules/oob/OutOfBandApi.ts	87.73% <ø> (ø)
packages/core/src/plugins/Module.ts	50.00% <ø> (ø)
packages/core/src/plugins/DependencyManager.ts	90.32% <90.90%> (-1.35%)	:arrow_down:
packages/core/src/agent/Agent.ts	97.34% <100.00%> (-0.26%)	:arrow_down:
packages/core/src/agent/AgentModules.ts	100.00% <100.00%> (ø)
packages/core/src/agent/BaseAgent.ts	95.40% <100.00%> (+0.05%)	:arrow_up:
.../src/modules/basic-messages/BasicMessagesModule.ts	100.00% <100.00%> (ø)
.../core/src/modules/connections/ConnectionsModule.ts	100.00% <100.00%> (ø)
.../core/src/modules/credentials/CredentialsModule.ts	100.00% <100.00%> (ø)
... and 14 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[karimStekelenburg]

Nice job @TimoGlastra! My only remark here is that the agent.api naming doesn't make total sense to me (maybe I'm seeing it wrong though). Wouldn't agent.modules make more sense?

[TimoGlastra]

My only remark here is that the agent.api naming doesn't make total sense to me (maybe I'm seeing it wrong though). Wouldn't agent.modules make more sense?

Yeah have been thinking about using module, but that also didn't fully make sense to me. It only exposes the api of the module, not the whole module object (so ConnectionsApi class instance, not the ConnectionsModule instance)

[karimStekelenburg]

Haha fair, this is a very semantic thingy.

My thought here is this: as a user I will pass modules to the agent constructor. Therefore, I already know that a module adds certain functionality. However, I might not be familiar with the internal structure of modules, so I might not be aware of the concept of FooApi classes.

Since I'm passing modules, I wouldn't be surprised to have to access their APIs through agent.modules.

Anyway, just my thought! I try to think about these things as an outsider, not a framework developer. This maybe is a good candidate for a WG discussion ;)

[TimoGlastra]

Input from WG call:

• add custom property that only contains the custom modules. Default modules will still be top level.
• use modules instead of api.

I think I agree with the agent.modules part over the agent.api. Not sure I'm a fan of the custom namespace. More feedback is welcome!

[berendsliedrecht]

Input from WG call:

• add custom property that only contains the custom modules. Default modules will still be top level.

• use modules instead of api.

I think I agree with the agent.modules part over the agent.api. Not sure I'm a fan of the custom namespace. More feedback is welcome!

I am slightly in favour of the top-level modules and custom modules defined in agent.modules. I think doing something like agent.modules.credentials.offerCredential becomes so long that it will be annoying to use. Custom modules are IMHO second-class and can be defined under a specific namespace.

Keeping the core-defined modules top level, which in the end might not be a lot, makes a cleaner API.

Having said that, doing agent.oob.XXX For an invitation and then agent.modules.credential.XXX For issuance is very weird.

I am fine with either for different reasons. Maybe we can look at different frameworks and how they solved this issue?

[berendsliedrecht]

I thought about this issue a bit more, and it might not be such a big issue to have breaking changes when we add new modules into core. Adding a module into core, after modularisation, is very unlikely to happen when we have the did core modules in there. Unless they expand their spec or we want add seperation, we would not have the add any modules. Of course, it might happen and bundling it with some other breaking changes would be fine IMHO.

Also, as we mentioned in the last WG call, the agent.module.XXX does not really solve the key-collision issue vs the agent.XXX. With this my preference is for agent.XXX unless I am missing something that was another purpose of agent.module.XXX.

[TimoGlastra]

Well we need to change the call to Agent.create (or something like that such as a separate method createAgent) instead of calling new Agent(), but otherwise there's no difference.

I'll go with the top level approach then!

[genaris]

I don't see any problem on using an agent creator method, especially considering that we currently need to call initialize method after the constructor to make it usable. Could be done anything there to simplify this initialization in cases where applicable?

A question that arises from this approach is if there would be a chance of overriding a core module. We currently do so with connections module to add some project-specific capabilities (we sub-class Agent and define a connections property assigned to a sub-class of ConnectionModule), and even if it feels a bit hacky, it works quite smoothly.

[berendsliedrecht]

I don't see any problem on using an agent creator method, especially considering that we currently need to call initialize method after the constructor to make it usable. Could be done anything there to simplify this initialization in cases where applicable?

I think await Agent.create({}).initialize() could also be await Agent.createAndInitialize({}). Seems quite convenient as I think that creating the Agent and initialising it afterwards is something new users might not know.

Also, again not a massive fan of bloating the config, but adding a value to the config with initializeOnCreation: Boolean might also avoid it. However, this might cause issues as Agent.create({}) is synchronous and Agent.initialize is asynchronous and I am not sure if we can handle this properly, return type wise (void | Promise), based on the config field.

A question that arises from this approach is if there would be a chance of overriding a core module. We currently do so with connections module to add some project-specific capabilities (we sub-class Agent and define a connections property assigned to a sub-class of ConnectionModule), and even if it feels a bit hacky, it works quite smoothly.

It would be possible to override a core module.

const agent = Agent.create({
  config: { /* agent config */ },
  modules: {
    CORE_MODULE: new CORE_MODULE({config: 'yes'}), // CORE_MODULE is a core module and here we can define a new one.
  },
  dependencies: agentDependencies,
})

[TimoGlastra]

Yes as berend mentioned it would be possible to override the core module by providing it in the agent config, however the typing is currently set to only allow the same module. You would have to write your CustomModule and register it under a different key. We could loosen the type a bit to allow different modules, but I think it makes sense to use a different key if you're not using the connections module. So agent.myConnections or something that contains your business name etc.. would be a bit more explicit maybe? Otherwise we can look at making it accept the same interface (as it extends the connections module)

[genaris]

Yes as berend mentioned it would be possible to override the core module by providing it in the agent config, however the typing is currently set to only allow the same module. You would have to write your CustomModule and register it under a different key. We could loosen the type a bit to allow different modules, but I think it makes sense to use a different key if you're not using the connections module. So agent.myConnections or something that contains your business name etc.. would be a bit more explicit maybe? Otherwise we can look at making it accept the same interface (as it extends the connections module)

Certainly, if it can accept the same interface (or an extension of it) it would be great to allow these 'tuned core modules'. Of course there is no problem on using other keys, but if I expose agent.connections and agent.myConnections I'll allow my consumers to use both, and probably that's not something I want.

Anyway, there are some tricks in TypeScript that allow hiding some keys from superclasses, and for sure it's possible to think things differently for using different keys in an elegant way, so it's not a big deal if this idea gets too complicated. Just a 'nice to have' from my point of view.

[TimoGlastra]

it's not a big deal if this idea gets too complicated. Just a 'nice to have' from my point of view.

I think it's doable, but maybe for a future PR. This can be added as a non-breaking change, so we could maybe push it to 0.3.1

[TimoGlastra]

This PR is now updated and should be ready to merge. I've updated the description of the PR but it boils down to:

• default modules are still nested under agent.xxx
• custom modules are nested under agent.modules.xxx.

I think this is one of the blockers for the 0.3.0 release, so let's get this merged :)