diff --git a/README.md b/README.md index 219baa6..b037761 100644 --- a/README.md +++ b/README.md @@ -1084,6 +1084,7 @@ Primary references: - [Security checklist](docs/security_checklist.md) - [Config surface ADR](docs/adr/ADR-0001-config-surface-unification.md) - [Product boundary ADR](docs/adr/ADR-0002-product-boundary-and-packaging-contract.md) +- [Connector extraction ADR](docs/adr/ADR-0003-connector-extraction-feasibility-and-seams.md) - [Config and secrets contract](docs/release/config_secrets_contract.md) - [Advanced registry and transforms](docs/advanced_registry_and_transforms.md) - [Connector guide](docs/connector.md) diff --git a/api/connector_contracts.py b/api/connector_contracts.py index ea7235c..208abb5 100644 --- a/api/connector_contracts.py +++ b/api/connector_contracts.py @@ -25,6 +25,9 @@ except ImportError: # pragma: no cover if __package__ and "." in __package__: from ..services.access_control import require_admin_token, resolve_token_info + from ..services.connector_extraction_contract import ( + get_connector_extraction_contract, + ) from ..services.connector_installation_registry import ( get_connector_installation_registry, ) @@ -33,6 +36,9 @@ if __package__ and "." in __package__: else: # pragma: no cover from services.access_control import require_admin_token # type: ignore from services.access_control import resolve_token_info # type: ignore + from services.connector_extraction_contract import ( # type: ignore + get_connector_extraction_contract, + ) from services.connector_installation_registry import ( # type: ignore get_connector_installation_registry, ) @@ -241,3 +247,22 @@ async def connector_installation_audit_handler(request): {"ok": False, "error": exc.code, "message": str(exc)}, status=403, ) + + +@endpoint_metadata( + auth=AuthTier.ADMIN, + risk=RiskTier.LOW, + summary="Connector extraction contract", + description="Returns the machine-readable connector extraction feasibility contract.", + audit="connector.extraction_contract.get", + plane=RoutePlane.ADMIN, +) +async def connector_extraction_contract_handler(request): + if (guard := _require_admin(request)) is not None: + return guard + return web.json_response( + { + "ok": True, + "contract": get_connector_extraction_contract(), + } + ) diff --git a/api/route_registrars.py b/api/route_registrars.py index c8500ce..d1490a2 100644 --- a/api/route_registrars.py +++ b/api/route_registrars.py @@ -226,6 +226,11 @@ def build_connector_installation_route_specs( f"{prefix}/connector/installations", handlers["connector_installations_list_handler"], ), + RouteSpec( + "GET", + f"{prefix}/connector/extraction-contract", + handlers["connector_extraction_contract_handler"], + ), RouteSpec( "GET", f"{prefix}/connector/installations/resolve", diff --git a/api/routes.py b/api/routes.py index f88172e..63dd61f 100644 --- a/api/routes.py +++ b/api/routes.py @@ -67,6 +67,7 @@ remote_admin_page_handler = None # type: ignore # F61 security_doctor_handler = None # type: ignore # S30 connector_installations_list_handler = connector_installation_get_handler = None # type: ignore connector_installation_resolve_handler = connector_installation_audit_handler = None # type: ignore +connector_extraction_contract_handler = None # type: ignore templates_list_handler = None # type: ignore rewrite_recipes_list_handler = rewrite_recipe_get_handler = None # type: ignore rewrite_recipe_create_handler = rewrite_recipe_update_handler = None # type: ignore @@ -96,6 +97,7 @@ if web is not None: connector_installation_get_handler, connector_installation_resolve_handler, connector_installation_audit_handler, + connector_extraction_contract_handler, ) = import_attrs_dual( __package__, "..api.connector_contracts", @@ -105,6 +107,7 @@ if web is not None: "connector_installation_get_handler", "connector_installation_resolve_handler", "connector_installation_audit_handler", + "connector_extraction_contract_handler", ), ) ( @@ -919,6 +922,7 @@ def register_routes(server) -> None: if connector_installations_list_handler: connector_installation_handlers = { "connector_installations_list_handler": connector_installations_list_handler, + "connector_extraction_contract_handler": connector_extraction_contract_handler, "connector_installation_resolve_handler": connector_installation_resolve_handler, "connector_installation_audit_handler": connector_installation_audit_handler, "connector_installation_get_handler": connector_installation_get_handler, diff --git a/docs/adr/ADR-0003-connector-extraction-feasibility-and-seams.md b/docs/adr/ADR-0003-connector-extraction-feasibility-and-seams.md new file mode 100644 index 0000000..8df04bc --- /dev/null +++ b/docs/adr/ADR-0003-connector-extraction-feasibility-and-seams.md @@ -0,0 +1,89 @@ +# ADR-0003: Connector Extraction Feasibility And Split-Package Seams + +- Status: Accepted +- Date: 2026-04-24 +- Owners: OpenClaw maintainers +- Related roadmap items: `R162` with prior boundary decision in `ADR-0002` + +## Context + +ADR-0002 established that the connector is an **optional attached subsystem**, not the primary published artifact of this repository. The remaining question is whether that attached subsystem should now be extracted into a separately packaged connector or separate repo. + +Current code structure still mixes: + +- connector platform adapters and runtime +- shared installation/token lifecycle services +- shared callback signing and replay-protection contracts +- backend delivery/result APIs that the connector calls locally +- tenant/config/auth boundaries that remain owned by the core package + +That means extraction is no longer a purely packaging question. It is a shared-contract question. + +## Decision + +OpenClaw adopts a **no-go-for-split-now** decision for connector extraction. + +Current recommendation: + +1. Keep the connector **in-repo** as an **optional attached subsystem**. +2. Treat a future **optional extra package** as the only plausible next extraction target. +3. Treat both **sidecar-only distribution** and **separate repo / primary connector package** as **no-go now** options. + +## Minimum Stable Seams Required Before Any Split + +Any future extraction must first stabilize these seam families: + +1. **installation registry and token refs** + - workspace/account binding records + - tenant-scoped token-reference ownership + - installation diagnostics and fail-closed resolution + +2. **interactive callback security contract** + - signed callback envelopes + - timestamp / replay / idempotency checks + - action-policy mapping and approval downgrade semantics + +3. **delivery and result bridge** + - submission/result polling contract + - callback delivery expectations + - backend result payload compatibility + +4. **config/auth and tenant boundary** + - connector runtime config contract + - admin token / auth expectations + - tenant header behavior + - server-side secret/state ownership + +## Why Separate Packaging Is A No-Go Now + +Current blockers are concrete, not theoretical: + +- shared services import connector types and connector adapters import shared services, so extraction would currently create unstable bidirectional package seams +- installation/token/state ownership still lives in shared repo services rather than a versioned connector-boundary package +- connector API/client flows still assume in-repo backend evolution instead of a versioned external backend contract +- `services/sidecar` still imports connector config/client modules directly, so even a packaging-only split would not isolate ownership yet + +## Consequences + +Positive: + +- maintainers now have one explicit go/no-go answer instead of repeatedly re-litigating extraction +- future connector extraction work can target named seam families instead of rediscovering coupling ad hoc +- admin diagnostics can expose the same contract to future packaging or release automation + +Trade-offs: + +- the repo intentionally keeps connector and core package code together for now +- packaging hygiene remains a future concern rather than a solved distribution problem +- extraction pressure is deferred until shared contracts are versionable on their own + +## Rejected Alternatives + +1. Extract connector into a separate repo now + - Rejected because current coupling would move instability across package boundaries instead of reducing it. + +2. Publish connector as a sidecar-only primary distribution now + - Rejected because current operator workflows still assume the embedded OpenClaw package/runtime remains primary. + +3. Leave extraction as an undocumented future possibility + - Rejected because future packaging work needs an explicit seam map and a clear no-go baseline. diff --git a/docs/connector.md b/docs/connector.md index a45db29..47febe8 100644 --- a/docs/connector.md +++ b/docs/connector.md @@ -4,6 +4,8 @@ The **OpenClaw Connector** (`connector`) is a standalone process that allows you Per the product boundary contract, the connector is an **optional attached subsystem**. The primary published artifact of this repo remains the **ComfyUI custom node pack**, and the connector augments that package rather than replacing it. +Current extraction decision: keep the connector **in-repo** as an optional attached subsystem for now. OpenClaw does **not** currently treat a standalone connector package/repo as a supported distribution; see [ADR-0003](adr/ADR-0003-connector-extraction-feasibility-and-seams.md). + ## How It Works The connector runs alongside ComfyUI on your machine. diff --git a/docs/sidecar.md b/docs/sidecar.md index d79dc37..76a45f8 100644 --- a/docs/sidecar.md +++ b/docs/sidecar.md @@ -16,4 +16,5 @@ - Bridge APIs and connector runtime are available. - Connector/sidecar runtime remains an optional attached subsystem; the primary package artifact is the ComfyUI custom node pack. +- Connector extraction remains a no-go-for-split-now decision until the shared installation/callback/delivery/config seams are independently versioned; see `docs/adr/ADR-0003-connector-extraction-feasibility-and-seams.md`. - Standalone sidecar/gateway evolution is tracked in `.planning/roadmap.md`. diff --git a/services/connector_extraction_contract.py b/services/connector_extraction_contract.py new file mode 100644 index 0000000..5b4f2f9 --- /dev/null +++ b/services/connector_extraction_contract.py @@ -0,0 +1,143 @@ +""" +Machine-readable connector extraction feasibility contract. +""" + +from __future__ import annotations + +import copy +from typing import Any, Dict + +CONNECTOR_EXTRACTION_CONTRACT_VERSION = 1 + +_CONNECTOR_EXTRACTION_CONTRACT: Dict[str, Any] = { + "version": CONNECTOR_EXTRACTION_CONTRACT_VERSION, + "decision": { + "id": "stay_in_repo_attached_subsystem", + "status": "recommended_now", + "go_no_go": "no_go_for_split_now", + "summary": ( + "Keep the connector in-repo as an optional attached subsystem for now; " + "do not split it into a standalone package or separate repo yet." + ), + "future_candidate": "optional_extra_package_after_shared_contract_extraction", + }, + "candidate_packaging_options": [ + { + "id": "stay_in_repo_attached_subsystem", + "status": "recommended_now", + "summary": "Current attached-subsystem model under the ComfyUI package boundary.", + }, + { + "id": "optional_extra_package_after_shared_contract_extraction", + "status": "future_candidate", + "summary": ( + "Potential future split after installation, callback, delivery, and " + "config/auth seams are independently versioned." + ), + }, + { + "id": "sidecar_only_distribution", + "status": "no_go_now", + "summary": ( + "Not currently viable because operator workflows still depend on the " + "embedded OpenClaw package/runtime and its local APIs." + ), + }, + { + "id": "separate_repo_or_primary_connector_package", + "status": "no_go_now", + "summary": ( + "Not currently viable because connector and shared services still " + "have bidirectional runtime coupling." + ), + }, + ], + "minimum_stable_seam_families": [ + { + "id": "installation_registry_and_token_refs", + "summary": ( + "Workspace/account installation lifecycle, token-reference ownership, " + "tenant scoping, and diagnostics must stay stable before extraction." + ), + "entrypoints": [ + "services/connector_installation_registry.py", + "connector/platforms/slack_installation_manager.py", + "connector/platforms/feishu_installation_manager.py", + "api/connector_contracts.py", + ], + }, + { + "id": "interactive_callback_security_contract", + "summary": ( + "Signed callback envelopes, replay/idempotency checks, action-policy " + "mapping, and installation resolution must stay shared." + ), + "entrypoints": [ + "services/connector_callback_contract.py", + "connector/security_profile.py", + "connector/transport_contract.py", + "connector/platforms/feishu_webhook.py", + ], + }, + { + "id": "delivery_and_result_bridge", + "summary": ( + "Connector submission, result polling, and callback delivery depend " + "on stable backend APIs and result shapes." + ), + "entrypoints": [ + "connector/openclaw_client.py", + "connector/results_poller.py", + "services/callback_delivery.py", + "api/webhook_submit.py", + ], + }, + { + "id": "config_auth_and_tenant_boundary", + "summary": ( + "Connector runtime config, admin token expectations, tenant header " + "behavior, and server-side secret ownership must remain explicit." + ), + "entrypoints": [ + "connector/config.py", + "services/runtime_config.py", + "services/tenant_context.py", + "services/secret_store.py", + ], + }, + ], + "current_blockers": [ + { + "id": "bidirectional_runtime_imports", + "summary": ( + "Shared services import connector transport/config types while " + "connector adapters import shared service contracts." + ), + }, + { + "id": "shared_secret_and_state_ownership", + "summary": ( + "Installation token refs, tenant-aware secret store usage, and " + "connector state persistence still live in shared repo services." + ), + }, + { + "id": "local_backend_api_contract_not_versioned_for_external_package", + "summary": ( + "Connector client/result flows still assume in-repo backend API " + "evolution rather than a separately versioned public package contract." + ), + }, + { + "id": "sidecar_runtime_still_imports_connector_package_directly", + "summary": ( + "The sidecar runtime under `services/sidecar` still imports connector " + "config/client modules directly." + ), + }, + ], +} + + +def get_connector_extraction_contract() -> Dict[str, Any]: + return copy.deepcopy(_CONNECTOR_EXTRACTION_CONTRACT) diff --git a/tests/test_api_connector_contracts.py b/tests/test_api_connector_contracts.py index 218990c..c83c69e 100644 --- a/tests/test_api_connector_contracts.py +++ b/tests/test_api_connector_contracts.py @@ -113,6 +113,26 @@ class TestAPIConnectorContracts(unittest.IsolatedAsyncioTestCase): self.assertEqual(resp.status, 403) + async def test_extraction_contract_handler_success(self): + request = AsyncMock() + request.query = {} + + with ( + patch("api.connector_contracts.check_rate_limit", return_value=True), + patch( + "api.connector_contracts.require_admin_token", return_value=(True, None) + ), + ): + resp = await mod.connector_extraction_contract_handler(request) + + self.assertEqual(resp.status, 200) + body = json.loads(resp.body) + self.assertTrue(body["ok"]) + self.assertEqual( + body["contract"]["decision"]["id"], + "stay_in_repo_attached_subsystem", + ) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_r151_route_registrars.py b/tests/test_r151_route_registrars.py index f247381..8d0a852 100644 --- a/tests/test_r151_route_registrars.py +++ b/tests/test_r151_route_registrars.py @@ -102,13 +102,15 @@ class TestR151RouteRegistrars(unittest.TestCase): "/openclaw", { "connector_installations_list_handler": sentinel.list_handler, + "connector_extraction_contract_handler": sentinel.contract_handler, "connector_installation_resolve_handler": sentinel.resolve_handler, "connector_installation_audit_handler": sentinel.audit_handler, "connector_installation_get_handler": sentinel.get_handler, }, ) keys = {(spec.method, spec.path) for spec in specs} - self.assertEqual(4, len(specs)) + self.assertEqual(5, len(specs)) + self.assertIn(("GET", "/openclaw/connector/extraction-contract"), keys) self.assertIn(("GET", "/openclaw/connector/installations/audit"), keys) self.assertIn( ("GET", "/openclaw/connector/installations/{installation_id}"), keys diff --git a/tests/test_r162_connector_extraction_contract.py b/tests/test_r162_connector_extraction_contract.py new file mode 100644 index 0000000..9c35b54 --- /dev/null +++ b/tests/test_r162_connector_extraction_contract.py @@ -0,0 +1,58 @@ +from pathlib import Path +import unittest + +from services.connector_extraction_contract import get_connector_extraction_contract + +ROOT = Path(__file__).resolve().parents[1] +ADR_PATH = ( + ROOT / "docs" / "adr" / "ADR-0003-connector-extraction-feasibility-and-seams.md" +) +CONNECTOR_DOC_PATH = ROOT / "docs" / "connector.md" + + +class TestR162ConnectorExtractionContract(unittest.TestCase): + def test_recommendation_and_options_are_stable(self): + contract = get_connector_extraction_contract() + + self.assertEqual( + contract["decision"]["id"], "stay_in_repo_attached_subsystem" + ) + self.assertEqual(contract["decision"]["go_no_go"], "no_go_for_split_now") + self.assertEqual( + contract["decision"]["future_candidate"], + "optional_extra_package_after_shared_contract_extraction", + ) + self.assertEqual( + [option["id"] for option in contract["candidate_packaging_options"]], + [ + "stay_in_repo_attached_subsystem", + "optional_extra_package_after_shared_contract_extraction", + "sidecar_only_distribution", + "separate_repo_or_primary_connector_package", + ], + ) + + def test_seam_entrypoints_exist(self): + contract = get_connector_extraction_contract() + + for seam in contract["minimum_stable_seam_families"]: + for rel_path in seam["entrypoints"]: + self.assertTrue((ROOT / rel_path).exists(), rel_path) + + def test_docs_align_with_decision_terms(self): + adr_text = ADR_PATH.read_text(encoding="utf-8") + connector_text = CONNECTOR_DOC_PATH.read_text(encoding="utf-8") + + for phrase in ( + "optional attached subsystem", + "no-go-for-split-now", + "optional extra package", + ): + self.assertIn(phrase, adr_text) + + self.assertIn("in-repo", connector_text) + self.assertIn("ADR-0003", connector_text) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_s33r76r78_auth_hardening.py b/tests/test_s33r76r78_auth_hardening.py index b7a2e88..945386b 100644 --- a/tests/test_s33r76r78_auth_hardening.py +++ b/tests/test_s33r76r78_auth_hardening.py @@ -76,6 +76,7 @@ AUTH_CLASS_BY_ROUTE = { ("POST", "/tools/{name}/run"): "admin", ("POST", "/pnginfo"): "admin", ("GET", "/connector/installations"): "admin", + ("GET", "/connector/extraction-contract"): "admin", ("GET", "/connector/installations/resolve"): "admin", ("GET", "/connector/installations/audit"): "admin", ("GET", "/connector/installations/{installation_id}"): "admin",