From d68a3621f304856a61a9a04b66ccaa8030f91a13 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Thu, 15 Oct 2026 10:58:05 +0200 Subject: [PATCH 1/6] docs(adr): 0030 optional modules share one contract and load only when turned on --- ...e-contract-and-load-only-when-turned-on.md | 306 ++++++++++++++++++ docs/internal/adr/README.md | 1 + 2 files changed, 307 insertions(+) create mode 100644 docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md diff --git a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md new file mode 100644 index 00000000..c57b8d4c --- /dev/null +++ b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md @@ -0,0 +1,306 @@ +# 0030. Optional modules share one contract and load only when turned on + +- **Status:** Proposed +- **Date:** 2026-10-15 +- **Issue:** [#591](https://github.com/callstackincubator/simlock/issues/591) +- **Supersedes:** nothing. Narrows [ADR + 0023](0023-agent-device-is-served-by-an-optional-edge-module.md) §3, the + line "reaped and restarted when the simlock daemon starts": a leftover + agent-device daemon is reaped only when simlock starts with agent-device + turned on (§2). Keeps [ADR + 0024](0024-leasing-calls-lease-end-participants-before-reclaim.md) as it + is: the lease-end port and its ceiling do not change; the host adapts a + module's hook to it (§3). Keeps [ADR + 0026](0026-agent-device-through-a-gateway-relays-over-the-uplink.md)'s + capabilities and relay as they are, and fixes where their names come + from (§1). +- **Depends on:** ADR 0023, 0024, 0026, architecture rules 5, 12, 18–21. + +## Context + +An optional module is a part of simlock a worker runs only when its config +turns it on. agent-device is the only one on `main`. Screen streaming (PR +#576) is being built as the second. + +Each one is wired by hand in `src/daemon/main.ts`. agent-device's index +exports 14 names. The start-up file builds its supervisor, its edge parts, +its capability source, its relay target, its lease-end participant, its +HTTP front on both roles, its gateway fleet port, and calls its stop steps +in the right order. #576 adds about 180 lines for streaming next to it. + +Nothing ties a module's parts together, and the second module already shows +the cost: + +- The name is typed out in several places. #576's capability is + `screen-stream` and its relay target is `stream`. +- A disabled agent-device reports `off`. A disabled stream reports nothing. +- agent-device's config lives in the core (`src/core/config.ts`), and + its doctor finding is a kind of its own in the wire contract. +- #576 re-declares the HTTP mount type, because a module that imports + `src/http` creates a dependency cycle. + +Architecture rules 19 and 20 say to add an extension point once a second +implementation exists, in one shape. Streaming is that second +implementation. This ADR fixes the shape before streaming lands, and moves +agent-device onto it first. + +```mermaid +flowchart LR + subgraph Before + M1[daemon/main.ts] -->|14 imports, ~12 wiring spots| AD1[agent-device] + M1 --> CC1[core/config.ts
agentDevice keys] + M1 --> D1[core doctor
agent-device kind] + end + subgraph After + M2[daemon/main.ts] --> REG[modules/registry] + REG --> DEF[agent-device/definition] + DEF -. load when on .-> AD2[agent-device] + AD2 --> T[modules types] + end +``` + +## Decision + +### 1. A module is one definition in one registry + +A new component, `src/modules/`, owns the extension point. Its `index.ts` +exports the types below. `src/modules/registry.ts` lists every module's +definition, one line each. The daemon and the CLI read the registry. No +other file names a module. + +```ts +export interface OptionalModuleDefinition { + readonly name: string; + readonly config: ModuleConfig; + load(): Promise<{ + createWorker(ctx: WorkerContext): WorkerModule; + createGateway?(ctx: GatewayContext): GatewayModule; + }>; +} +``` + +A module's directory has a small `definition.ts` that exports its +definition. It imports only types and its config schema, so reading the +registry loads no module code. + +`name` is the module's only name: + +- the capability a worker reports; +- the relay target a gateway sends to; +- the `module` field of its doctor finding (§6); +- its config key, in camelCase: `agent-device` is `agentDevice`. + +The name passes the capability name check (`^[a-z][a-z0-9-]{0,31}$`). A +test over the registry checks every name and its config key. + +### 2. A module is loaded only when it is turned on + +The host reads `.enabled`. When it is `false`, the host calls +nothing of the module and loads none of its code. It reports the module's +capability as `off` itself. + +When it is `true`, the host awaits `load()`, a dynamic `import()` of the +module's `index.ts`, and calls `createWorker` or `createGateway`. + +A module turned off runs nothing, so it cannot clean up after a crash. If +simlock crashes while agent-device is on and starts again with it off, the +agent-device daemon the crash left keeps running. It is reaped the next +time simlock starts with agent-device on, as today. This needs a crash and +a config change at once. We accept it. + +### 3. The handle: what a module provides, and hooks + +`createWorker` returns a handle. Fields the module provides are nouns. The +host reads them once, when it builds itself. Calls the host makes at a +moment are grouped under `hooks`. Each hook is named for the host step it +runs at. + +```ts +export interface WorkerModule { + readonly capability: CapabilitySource; + readonly relayTarget?: RelayTarget; + readonly mount?: (deps: MountDeps) => ModuleMount; + readonly findings?: () => readonly ModuleFinding[]; + readonly hooks?: WorkerHooks; +} + +export interface WorkerHooks { + readonly afterSocketClaimed?: () => void; + readonly beforeDeviceReclaim?: (lease: EndedLease, signal: AbortSignal) => Promise; + readonly beforeDrain?: () => Promise; + readonly beforeDriversDispose?: () => Promise; +} + +export interface GatewayModule { + readonly mount?: (deps: MountDeps) => ModuleMount; + readonly hooks?: { readonly beforeDrain?: () => Promise }; +} +``` + +Each hook's name is a promise the host keeps: + +| Hook | Runs | Awaited | Bounded by | +|---|---|---|---| +| `afterSocketClaimed` | once this daemon holds its socket; never in a daemon that lost the race | no | — | +| `beforeDeviceReclaim` | after leasing commits a lease's end, before it reclaims the device | yes, by leasing | leasing's lease-end ceiling (ADR 0024 §2) | +| `beforeDrain` | on stop, after new work is refused, before leasing settles | yes | the host's hook deadline | +| `beforeDriversDispose` | on stop, before any driver is disposed | yes | the host's hook deadline | + +The host keeps these rows in one table and runs every module's hooks +through it. It is the one place that bounds them and logs a hook that runs +out of time. The technical spec sets the deadlines from today's waits. +`beforeDeviceReclaim` reaches leasing through the lease-end port ADR 0024 +already defines; leasing does not change. + +Hooks are function-valued properties, not methods, so the host can call +them without binding `this`. + +```mermaid +sequenceDiagram + participant S as DaemonServer + participant H as module host + participant M as module hooks + participant L as leasing + participant D as drivers + + S->>H: stop: frontends + H->>M: beforeDrain() + alt finishes in time + M-->>H: done + else deadline passes + H->>H: log, stop waiting + end + S->>L: settle (drain in-flight reclaims) + S->>H: dispose + H->>M: beforeDriversDispose() + M-->>H: done or deadline + H->>D: dispose drivers +``` + +### 4. The module gets a context, and observes through the bus only + +`createWorker(ctx)` gets everything the module uses: its own config +section, a logger named for it, the clock, the ports it may use, the +drivers' platforms and lease environment, and `events.subscribe`. + +The event bus is for observing. A module may subscribe to business events +to watch them. A step that has to finish before the host goes on is a hook, +never an event handler. Bus handlers are not awaited, and ADR 0024 already +rejected an event handler for the lease-end step. + +`createGateway(ctx)` gets a fleet context bound to the module's name: + +```ts +export interface GatewayContext { + readonly config: C; + readonly instanceId: string; + readonly startedAt: number; + readonly requesterPrefix: string; + workers(): readonly WorkerView[]; + worker(id: string): WorkerView | undefined; + serving(): readonly WorkerView[]; + readonly leases: FleetLeases; + relay(workerId: string, request: Request, tenant: string, leaseId?: string): Promise; +} +``` + +`serving()` lists connected workers that report this module `available`. +`relay` sends to the relay target named after this module. A module cannot +send to another module's target. `FleetLeases` and the other fleet types +are declared in `src/modules` by shape, because a module may not import +`src/gateway`. + +agent-device's gateway fleet port moves out of `main.ts` into the module +and is built from this context. + +### 5. A module owns its config + +`ModuleConfig` holds the module's config schema, its defaults, the keys +a gateway reads, and a check that may read the rest of the merged config +(agent-device's `leaseTtlCapMs` must not exceed `lease.maxTtlMs`). + +`loadConfig` takes the registry's configs from its caller, the daemon or +the CLI. The core declares the shape it takes and imports nothing from a +module. Keys stay top-level, so no user's `config.json` changes. A gateway +warns about a module key it does not read, as today. + +### 6. Shared types live in `src/modules` + +`src/modules` declares `ModuleMount`, the shape the HTTP app mounts. +`src/http` imports it from there, so a module never imports `src/http`. + +`src/modules` imports only `src/ports`, `src/contract` and core types. A +dependency-cruiser rule enforces it. The core, leasing, the drivers and the +gateway import nothing from `src/modules` or from any module. + +### 7. One doctor finding for every module + +The doctor finding `agent-device-unavailable` becomes: + +```json +{ "kind": "module-unavailable", "module": "agent-device", "message": "…", "remedy": "…" } +``` + +A module's `findings()` returns it. The core doctor, the contract and the +CLI know the one kind, not any module. The protocol version moves from 36 +to 37 with no shim, and the changelog says so. + +### 8. Tests swap modules, not module parts + +`startDaemon` drops the `agentDeviceUpstream` and `agentDeviceHealth` +options. It takes `optionalModules`, which replaces the registry. A +module's `testing.ts` exports a definition backed by its fakes. A +conformance suite runs over the registry with `describe.each`: + +- a module turned off is not loaded and reports `off`; +- each hook's row in §3 holds; +- the name and config key pass §1's checks. + +## Consequences + +- Adding an optional module is a directory, a `definition.ts` and one + registry line. `main.ts`, the core config, the doctor and the contract do + not change. +- `main.ts` names no module. agent-device's 14 imports and its gateway + fleet port leave it. +- Every module turned off reports `off`, and its code is never loaded. +- A script matching `agent-device-unavailable` breaks. The protocol bump + makes an old client refuse a new daemon rather than misread it. +- A leftover agent-device daemon is not reaped while agent-device is off. + `docs/CONFIGURATION.md`'s line "A daemon a killed Simlock left behind is + stopped at the next start" gains "with `agentDevice.enabled` `true`". +- The supervisor's path for `enabled: false` is deleted, with its test. +- Streaming lands on this contract when it becomes a feature. Its + capability and relay target then share one name. + +## Alternatives considered + +- **Hooks as event-bus events** (`daemon.stopping`, `drivers.disposing`). + Rejected: bus handlers are not awaited, so `settle` could start while + agent-device's waiting allocates are still open, and the Android driver + could stop the adb server under agent-device. Bus events are also public + facts in `EVENTS.md`. +- **One `onInit(ctx)` that registers everything on the context** + (`ctx.mount(…)`, `ctx.beforeDrain(…)`). Rejected: nothing checks that a + module registered what it must, a late registration does nothing + silently, and the type no longer shows what a module provides. +- **Generic names** (`onInit`, `onDispose`). Rejected: agent-device has two + start moments and two stop moments with different guarantees. A name that + fits both hides the one it needs. +- **A definition-level cleanup hook, or a `loadWhenOff` flag,** so a module + turned off can reap a crash's leftovers. Rejected: it serves only a crash + followed by a config change, and splits agent-device's start order + between host and module. +- **The host records and reaps every detached process modules start.** + Rejected: a shared mechanism with one user. +- **Gateway adapters stay in `main.ts`, one per module.** Rejected: about + 120 lines of per-module code stay in the start-up file, and a module's + code is split across two places. +- **Config under `modules.`.** Rejected: it breaks every existing + `config.json` for a tidier layout. +- **The core keeps every module's config, checked by a test.** Rejected: + each module would still edit the core. +- **A doctor finding kind per module.** Rejected: each module would edit + the contract, the core doctor and the CLI. +- **Always load every module.** Rejected: a module turned off should cost + nothing, and streaming's capture code has no reason to load when off. diff --git a/docs/internal/adr/README.md b/docs/internal/adr/README.md index 81c22a51..511da2b2 100644 --- a/docs/internal/adr/README.md +++ b/docs/internal/adr/README.md @@ -68,3 +68,4 @@ the status is stale. | [0027](0027-dependency-cruiser-checks-every-component-boundary.md) | dependency-cruiser checks every component's boundary and the module graph | Accepted | | [0028](0028-the-uplink-carries-byte-channels-and-small-messages-go-first.md) | The uplink carries byte channels beside its messages, and small messages go first | Accepted | | [0029](0029-host-load-is-an-optional-module-whose-readings-ride-on-status.md) | Host load is an optional module whose readings ride on status | Accepted | +| [0030](0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md) | Optional modules share one contract and load only when turned on | Proposed | From fc047be151fbf11590ed6a9e77ce028be72132f0 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Thu, 15 Oct 2026 11:00:02 +0200 Subject: [PATCH 2/6] docs(adr): 0030 accepted --- ...modules-share-one-contract-and-load-only-when-turned-on.md | 2 +- docs/internal/adr/README.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md index c57b8d4c..f345d062 100644 --- a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md +++ b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md @@ -1,6 +1,6 @@ # 0030. Optional modules share one contract and load only when turned on -- **Status:** Proposed +- **Status:** Accepted — not yet implemented - **Date:** 2026-10-15 - **Issue:** [#591](https://github.com/callstackincubator/simlock/issues/591) - **Supersedes:** nothing. Narrows [ADR diff --git a/docs/internal/adr/README.md b/docs/internal/adr/README.md index 511da2b2..884b5353 100644 --- a/docs/internal/adr/README.md +++ b/docs/internal/adr/README.md @@ -61,11 +61,11 @@ the status is stale. | [0020](0020-a-requester-may-choose-its-lease-id.md) | A requester may choose its lease id, and the gateway passes it through bare | Accepted | | [0021](0021-a-gateway-dispatch-is-a-probe-and-names-its-fleet-request.md) | A gateway dispatch is a probe, and the gateway records every fleet request's outcome | Accepted — partly implemented: §5 (usage figures) is not built | | [0022](0022-a-physical-device-is-a-second-device-type-the-core-knows.md) | A physical device is a second device type the core knows | Proposed | -| [0023](0023-agent-device-is-served-by-an-optional-edge-module.md) | agent-device is served by an optional edge module, and the simlock lease is the only clock | Accepted; §1 narrowed by 0026 for gateway mode | +| [0023](0023-agent-device-is-served-by-an-optional-edge-module.md) | agent-device is served by an optional edge module, and the simlock lease is the only clock | Accepted; §1 narrowed by 0026 for gateway mode; §3 narrowed by 0030 (no reap while turned off) | | [0024](0024-leasing-calls-lease-end-participants-before-reclaim.md) | Leasing calls lease-end participants before it reclaims a device | Accepted | | [0025](0025-simlock-ships-android-profiles-and-a-model-pairs-only-with-images-that-fit.md) | Simlock ships Android device profiles, and a model pairs only with images that fit it | Accepted | | [0026](0026-agent-device-through-a-gateway-relays-over-the-uplink.md) | agent-device through a gateway: workers advertise capabilities, and the host link relays over the uplink | Accepted; §2 narrowed by 0028 for relayed bodies | | [0027](0027-dependency-cruiser-checks-every-component-boundary.md) | dependency-cruiser checks every component's boundary and the module graph | Accepted | | [0028](0028-the-uplink-carries-byte-channels-and-small-messages-go-first.md) | The uplink carries byte channels beside its messages, and small messages go first | Accepted | | [0029](0029-host-load-is-an-optional-module-whose-readings-ride-on-status.md) | Host load is an optional module whose readings ride on status | Accepted | -| [0030](0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md) | Optional modules share one contract and load only when turned on | Proposed | +| [0030](0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md) | Optional modules share one contract and load only when turned on | Accepted — not yet implemented | From a8784b38303df585f514e76db54c19b415910a89 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Thu, 15 Oct 2026 12:11:29 +0200 Subject: [PATCH 3/6] docs(adr): 0030 config sits under capabilities, and stop hooks share a 30 s deadline --- ...e-contract-and-load-only-when-turned-on.md | 76 ++++++++++++++----- 1 file changed, 57 insertions(+), 19 deletions(-) diff --git a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md index f345d062..c19b7559 100644 --- a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md +++ b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md @@ -87,15 +87,16 @@ registry loads no module code. - the capability a worker reports; - the relay target a gateway sends to; -- the `module` field of its doctor finding (§6); -- its config key, in camelCase: `agent-device` is `agentDevice`. +- the `module` field of its doctor finding (§7); +- its key under `capabilities` in the config (§5). The name passes the capability name check (`^[a-z][a-z0-9-]{0,31}$`). A -test over the registry checks every name and its config key. +test over the registry checks every name. ### 2. A module is loaded only when it is turned on -The host reads `.enabled`. When it is `false`, the host calls +The host reads `capabilities..enabled`. When it is `false` or the +section is missing, the host calls nothing of the module and loads none of its code. It reports the module's capability as `off` itself. @@ -143,12 +144,14 @@ Each hook's name is a promise the host keeps: |---|---|---|---| | `afterSocketClaimed` | once this daemon holds its socket; never in a daemon that lost the race | no | — | | `beforeDeviceReclaim` | after leasing commits a lease's end, before it reclaims the device | yes, by leasing | leasing's lease-end ceiling (ADR 0024 §2) | -| `beforeDrain` | on stop, after new work is refused, before leasing settles | yes | the host's hook deadline | -| `beforeDriversDispose` | on stop, before any driver is disposed | yes | the host's hook deadline | +| `beforeDrain` | on stop, after new work is refused, before leasing settles | yes | the host's hook deadline, 30 s | +| `beforeDriversDispose` | on stop, before any driver is disposed | yes | the host's hook deadline, 30 s | The host keeps these rows in one table and runs every module's hooks through it. It is the one place that bounds them and logs a hook that runs -out of time. The technical spec sets the deadlines from today's waits. +out of time. Both stop hooks share one deadline of 30 s. It sits above +agent-device's slowest stop: a start still in progress, then 10 s after +SIGTERM and 2 s after SIGKILL. `beforeDeviceReclaim` reaches leasing through the lease-end port ADR 0024 already defines; leasing does not change. @@ -213,16 +216,41 @@ are declared in `src/modules` by shape, because a module may not import agent-device's gateway fleet port moves out of `main.ts` into the module and is built from this context. -### 5. A module owns its config +### 5. A module owns its config, under `capabilities` -`ModuleConfig` holds the module's config schema, its defaults, the keys -a gateway reads, and a check that may read the rest of the merged config -(agent-device's `leaseTtlCapMs` must not exceed `lease.maxTtlMs`). +Every module's config sits under one key, `capabilities`, keyed by the +module's name: + +```json +{ "capabilities": { "agent-device": { "enabled": true, "command": "agent-device" } } } +``` + +Users meet one word: they turn a capability on here, see its state in +status, and name it in a lease request's `requires`. The top-level +`agentDevice` key goes. agent-device's integration has not been released, +so nothing migrates it: an old `agentDevice` key is an unknown key, warned +about and ignored like any other. + +`ModuleConfig` holds the module's validator, its defaults, the keys a +gateway reads, and a check that may read the rest of the merged config +(agent-device's `leaseTtlCapMs` must not exceed `lease.maxTtlMs`). It +follows the shape capacity strategies already use: a validator and +defaults beside the definition. `loadConfig` takes the registry's configs from its caller, the daemon or the CLI. The core declares the shape it takes and imports nothing from a -module. Keys stay top-level, so no user's `config.json` changes. A gateway -warns about a module key it does not read, as today. +module. A section for a name no module has is an unknown key. A gateway +warns about a module key it does not read, as it does today. + +`config.get` returns the same layout. The contract types the section by +shape, so it never changes for a new module: + +```ts +capabilities: z.record(capabilityNameSchema, z.object({ enabled: z.boolean() }).catchall(z.unknown())), +``` + +A client reads `enabled` typed and the rest as `unknown`. The daemon has +already checked each section with its module's validator. ### 6. Shared types live in `src/modules` @@ -242,8 +270,8 @@ The doctor finding `agent-device-unavailable` becomes: ``` A module's `findings()` returns it. The core doctor, the contract and the -CLI know the one kind, not any module. The protocol version moves from 36 -to 37 with no shim, and the changelog says so. +CLI know the one kind, not any module. The protocol version moves up with +no shim, and the changelog says so. ### 8. Tests swap modules, not module parts @@ -254,13 +282,16 @@ conformance suite runs over the registry with `describe.each`: - a module turned off is not loaded and reports `off`; - each hook's row in §3 holds; -- the name and config key pass §1's checks. +- the name passes §1's check. ## Consequences - Adding an optional module is a directory, a `definition.ts` and one registry line. `main.ts`, the core config, the doctor and the contract do not change. +- `agentDevice` in `config.json` becomes `capabilities.agent-device`, and + `config.get` and the client's `SimlockConfig` change with it. The + protocol version moves up. - `main.ts` names no module. agent-device's 14 imports and its gateway fleet port leave it. - Every module turned off reports `off`, and its code is never loaded. @@ -268,7 +299,8 @@ conformance suite runs over the registry with `describe.each`: makes an old client refuse a new daemon rather than misread it. - A leftover agent-device daemon is not reaped while agent-device is off. `docs/CONFIGURATION.md`'s line "A daemon a killed Simlock left behind is - stopped at the next start" gains "with `agentDevice.enabled` `true`". + stopped at the next start" gains "with + `capabilities.agent-device.enabled` `true`". - The supervisor's path for `enabled: false` is deleted, with its test. - Streaming lands on this contract when it becomes a feature. Its capability and relay target then share one name. @@ -296,8 +328,14 @@ conformance suite runs over the registry with `describe.each`: - **Gateway adapters stay in `main.ts`, one per module.** Rejected: about 120 lines of per-module code stay in the start-up file, and a module's code is split across two places. -- **Config under `modules.`.** Rejected: it breaks every existing - `config.json` for a tidier layout. +- **One top-level key per module (`agentDevice`), typed in the contract + and checked by a test.** Rejected: every module would add a contract + entry, and the key would need a camelCase rule to match the name. +- **The namespace called `modules`.** Rejected: users see `capabilities` + in status, `requires` and `CAPABILITY_MISSING`, and would meet a second + word for the same thing. +- **Any extra top-level key let through untyped (`catchall`).** Rejected: + the core's own keys would lose their strictness. - **The core keeps every module's config, checked by a test.** Rejected: each module would still edit the core. - **A doctor finding kind per module.** Rejected: each module would edit From ca3198f0745ac3bbd655c7eb96dc022cc0ace166 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Thu, 15 Oct 2026 12:29:12 +0200 Subject: [PATCH 4/6] docs(adr): 0030 split loaders, own registry component, routes under the name, concurrent stop steps --- ...e-contract-and-load-only-when-turned-on.md | 84 +++++++++++++------ 1 file changed, 59 insertions(+), 25 deletions(-) diff --git a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md index c19b7559..4c3a9e54 100644 --- a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md +++ b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md @@ -63,25 +63,27 @@ flowchart LR ### 1. A module is one definition in one registry -A new component, `src/modules/`, owns the extension point. Its `index.ts` -exports the types below. `src/modules/registry.ts` lists every module's -definition, one line each. The daemon and the CLI read the registry. No -other file names a module. +A new component, `src/modules/`, owns the extension point: its +`index.ts` exports the types below and the host. A second component, +`src/module-registry/`, lists every module's definition, one line each. +The daemon and the CLI read the registry. No other file names a module. +The registry is its own component so that the registry, a module's +definition and `src/modules` form no import cycle. ```ts export interface OptionalModuleDefinition { readonly name: string; readonly config: ModuleConfig; - load(): Promise<{ - createWorker(ctx: WorkerContext): WorkerModule; - createGateway?(ctx: GatewayContext): GatewayModule; - }>; + loadWorker(): Promise<{ createWorker(ctx: WorkerContext): WorkerModule }>; + loadGateway?(): Promise<{ createGateway(ctx: GatewayContext): GatewayModule }>; } ``` A module's directory has a small `definition.ts` that exports its -definition. It imports only types and its config schema, so reading the -registry loads no module code. +definition. It imports its own module only as types, or through the +dynamic `import()` inside a loader, so reading the registry loads no +module code. A module with no `loadGateway` has no gateway side, and a +gateway never loads it. Each side loads only its own code. `name` is the module's only name: @@ -100,8 +102,11 @@ section is missing, the host calls nothing of the module and loads none of its code. It reports the module's capability as `off` itself. -When it is `true`, the host awaits `load()`, a dynamic `import()` of the -module's `index.ts`, and calls `createWorker` or `createGateway`. +When it is `true`, the host awaits `loadWorker()` on a worker, or +`loadGateway()` on a gateway, and calls `createWorker` or `createGateway`. +Creating a module starts nothing; a module starts in its +`afterSocketClaimed` hook. A start that fails after some modules were +created leaves them created and not started, with nothing to stop. A module turned off runs nothing, so it cannot clean up after a crash. If simlock crashes while agent-device is on and starts again with it off, the @@ -120,7 +125,7 @@ runs at. export interface WorkerModule { readonly capability: CapabilitySource; readonly relayTarget?: RelayTarget; - readonly mount?: (deps: MountDeps) => ModuleMount; + readonly mount?: (deps: MountDeps) => ModuleHandler; readonly findings?: () => readonly ModuleFinding[]; readonly hooks?: WorkerHooks; } @@ -133,7 +138,7 @@ export interface WorkerHooks { } export interface GatewayModule { - readonly mount?: (deps: MountDeps) => ModuleMount; + readonly mount?: (deps: MountDeps) => ModuleHandler; readonly hooks?: { readonly beforeDrain?: () => Promise }; } ``` @@ -149,7 +154,9 @@ Each hook's name is a promise the host keeps: The host keeps these rows in one table and runs every module's hooks through it. It is the one place that bounds them and logs a hook that runs -out of time. Both stop hooks share one deadline of 30 s. It sits above +out of time. For one step, every module's hook starts at once, and the step +waits at most its deadline in total. Both stop steps share one deadline of +30 s. It sits above agent-device's slowest stop: a start still in progress, then 10 s after SIGTERM and 2 s after SIGKILL. `beforeDeviceReclaim` reaches leasing through the lease-end port ADR 0024 @@ -158,6 +165,14 @@ already defines; leasing does not change. Hooks are function-valued properties, not methods, so the host can call them without binding `this`. +A module's HTTP routes sit under its name. `mount` returns only a handler; +the host serves it at `/v1/` and `/v1//*`. A module cannot pick +a route that clashes with another module's or with a core route. + +A module's `findings()` returns `{ message, remedy }`. The host adds the +finding's kind and the module's name (§7), so the name in a finding cannot +drift from the module's. + ```mermaid sequenceDiagram participant S as DaemonServer @@ -191,7 +206,9 @@ to watch them. A step that has to finish before the host goes on is a hook, never an event handler. Bus handlers are not awaited, and ADR 0024 already rejected an event handler for the lease-end step. -`createGateway(ctx)` gets a fleet context bound to the module's name: +`createGateway(ctx)` gets a fleet context bound to the module's name. It +also carries the gateway's granted devices, which agent-device's fleet port +reads: ```ts export interface GatewayContext { @@ -254,12 +271,17 @@ already checked each section with its module's validator. ### 6. Shared types live in `src/modules` -`src/modules` declares `ModuleMount`, the shape the HTTP app mounts. -`src/http` imports it from there, so a module never imports `src/http`. +`src/modules` declares the handler shape a module's routes have. The host +turns each handler into the mount the HTTP app already takes, so a module +never imports `src/http`, and `src/http` does not change. -`src/modules` imports only `src/ports`, `src/contract` and core types. A -dependency-cruiser rule enforces it. The core, leasing, the drivers and the -gateway import nothing from `src/modules` or from any module. +`src/modules` imports only `src/ports`, `src/contract` and `src/core`. A +dependency-cruiser rule enforces it. Types it needs from elsewhere (the +capability source, the relay target, the lease-end participant, the bus's +`subscribe`, the daemon's dispatch, the gateway's views and lease index) +are declared by shape, and a test checks each copy against the original. +The core, leasing, the drivers, the gateway and the contract import +nothing from `src/modules` or from any module. ### 7. One doctor finding for every module @@ -269,8 +291,9 @@ The doctor finding `agent-device-unavailable` becomes: { "kind": "module-unavailable", "module": "agent-device", "message": "…", "remedy": "…" } ``` -A module's `findings()` returns it. The core doctor, the contract and the -CLI know the one kind, not any module. The protocol version moves up with +The host builds it from what a module's `findings()` returns and the +module's name. The core doctor, the contract and the CLI know the one kind, +not any module. The protocol version moves up with no shim, and the changelog says so. ### 8. Tests swap modules, not module parts @@ -281,8 +304,9 @@ module's `testing.ts` exports a definition backed by its fakes. A conformance suite runs over the registry with `describe.each`: - a module turned off is not loaded and reports `off`; -- each hook's row in §3 holds; -- the name passes §1's check. +- creating a module starts nothing; +- each hook it declares settles within its row's deadline in §3; +- the name passes §1's check and no two modules share one. ## Consequences @@ -340,5 +364,15 @@ conformance suite runs over the registry with `describe.each`: each module would still edit the core. - **A doctor finding kind per module.** Rejected: each module would edit the contract, the core doctor and the CLI. +- **One `load()` for both sides.** Rejected: a gateway would have to load + a module to learn it has no gateway side, and would load its worker code. +- **The registry inside `src/modules`.** Rejected: the registry imports + each definition and each definition imports `src/modules`, a cycle + between components. +- **A module picks its own route prefix.** Rejected: a second name per + module, and a prefix could clash with a core route. +- **Hooks for one step run one after another.** Rejected: a hanging module + either holds up every module after it or makes the stop grow by 30 s per + module. - **Always load every module.** Rejected: a module turned off should cost nothing, and streaming's capture code has no reason to load when off. From 42122c405509d319abc1bd3fa1f8d55acf3eb9bc Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Thu, 15 Oct 2026 12:46:20 +0200 Subject: [PATCH 5/6] docs(adr): 0030 core route names are reserved, and modules may use http helpers and bus types --- ...e-contract-and-load-only-when-turned-on.md | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md index 4c3a9e54..f5271929 100644 --- a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md +++ b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md @@ -167,7 +167,10 @@ them without binding `this`. A module's HTTP routes sit under its name. `mount` returns only a handler; the host serves it at `/v1/` and `/v1//*`. A module cannot pick -a route that clashes with another module's or with a core route. +a route that clashes with another module's or with a core route: the +contract lists the first segments of the core routes, a test keeps that +list true to the HTTP app, and the host refuses a module named after one +at start, whether or not HTTP is on. A module's `findings()` returns `{ message, remedy }`. The host adds the finding's kind and the module's name (§7), so the name in a finding cannot @@ -272,14 +275,16 @@ already checked each section with its module's validator. ### 6. Shared types live in `src/modules` `src/modules` declares the handler shape a module's routes have. The host -turns each handler into the mount the HTTP app already takes, so a module -never imports `src/http`, and `src/http` does not change. - -`src/modules` imports only `src/ports`, `src/contract` and `src/core`. A -dependency-cruiser rule enforces it. Types it needs from elsewhere (the -capability source, the relay target, the lease-end participant, the bus's -`subscribe`, the daemon's dispatch, the gateway's views and lease index) -are declared by shape, and a test checks each copy against the original. +turns each handler into the mount the HTTP app already takes, so being +mounted needs nothing from `src/http`, and `src/http` does not change. A +module may still use `src/http`'s helpers to build its own front, as +agent-device does. + +`src/modules` imports only `src/ports`, `src/contract`, `src/core`, and +types from `src/bus`. A dependency-cruiser rule enforces it. Types it needs +from elsewhere (the capability source, the relay target, the lease-end +participant, the daemon's dispatch, the gateway's lease index) are declared +by shape, and a test checks each copy against the original. The core, leasing, the drivers, the gateway and the contract import nothing from `src/modules` or from any module. From 057484b56800b8b9c79f2a7892333d687d741810 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Thu, 15 Oct 2026 12:58:16 +0200 Subject: [PATCH 6/6] docs(adr): 0030 a start that fails after the socket is claimed runs the stop hooks --- ...dules-share-one-contract-and-load-only-when-turned-on.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md index f5271929..6be54109 100644 --- a/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md +++ b/docs/internal/adr/0030-optional-modules-share-one-contract-and-load-only-when-turned-on.md @@ -105,8 +105,10 @@ capability as `off` itself. When it is `true`, the host awaits `loadWorker()` on a worker, or `loadGateway()` on a gateway, and calls `createWorker` or `createGateway`. Creating a module starts nothing; a module starts in its -`afterSocketClaimed` hook. A start that fails after some modules were -created leaves them created and not started, with nothing to stop. +`afterSocketClaimed` hook. A start that fails before the socket is claimed +leaves the modules created and not started, with nothing to stop. One that +fails after it (convergence, the HTTP gateway) stops the daemon through the +normal stop, which runs every module's stop hooks. A module turned off runs nothing, so it cannot clean up after a crash. If simlock crashes while agent-device is on and starts again with it off, the