Skip to content

feat(connectors): emit connector.activation.changed for CLI/SDK activation writes (#1226)#1309

Open
alexey-tyurin wants to merge 1 commit into
amd:mainfrom
alexey-tyurin:feat/emit-activation-changed-events
Open

feat(connectors): emit connector.activation.changed for CLI/SDK activation writes (#1226)#1309
alexey-tyurin wants to merge 1 commit into
amd:mainfrom
alexey-tyurin:feat/emit-activation-changed-events

Conversation

@alexey-tyurin
Copy link
Copy Markdown
Contributor

Summary

CLI and SDK activation toggles now drive the same live connector.activation.changed SSE update that the HTTP router already emitted, so the Agent UI's "Active for" panel reflects them without a manual refresh.

Why

Activation writes from gaia connectors activations activate/deactivate (CLI) and direct SDK calls landed in ~/.gaia/connectors/activations.json silently — only the HTTP PUT/DELETE handlers emitted connector.activation.changed. With the Agent UI Settings tab open, a CLI-driven toggle took effect on disk but the panel kept showing stale state until the user navigated away and back to force a refetch, making CLI ↔ UI workflows confusing. This was filed as an explicit follow-up on #1219.

Linked issue

Closes #1226

Changes

  • Centralized the emit (src/gaia/connectors/api.py). activate() / deactivate() now emit connector.activation.changed ({connector_id, agent_id, active}) after the ledger write, so HTTP, SDK, and any in-process caller all notify through one path — mirroring how feat(connectors): per-agent MCP tool-visibility activations (#1005) #1219 centralized the MCP-only guard in api.py.
  • New emit_change() helper (src/gaia/connectors/events.py). A sync, loop-aware fire-and-forget wrapper so synchronous callers (api, CLI) can publish without being async. Inside the server loop it schedules on that loop and fans out to SSE subscribers; in a bare process it runs against the registered emitter (the no-op logging emitter). Failures are logged, never swallowed.
  • Server-side ledger watcher (src/gaia/connectors/activation_watcher.py, new). The CLI runs in a separate process from the UI server, where the SSE bus lives. A background watcher polls activations.json, diffs against a snapshot, and emits one event per changed (connector, agent) pair (~1 s) so cross-process CLI/SDK writes reach connected UI clients. In-process writes call note_local_write(...) to advance the snapshot per-pair, so they don't double-emit — and a concurrent change to a different pair is still caught.
  • Watcher lifecycle (src/gaia/ui/server.py). Started/stopped in the FastAPI lifespan, mirroring the existing DocumentMonitor.
  • Router cleanup + guard fix (src/gaia/ui/routers/connectors.py). Removed the now-duplicate inline emits from PUT/DELETE; DELETE now routes through api.deactivate instead of the bare ledger call, closing a pre-existing MCP-only-guard bypass on that route.
  • Tests. New tests/unit/connectors/test_activation_watcher.py (diff, poll-emit, dedup, don't-mask-other-pair) and extended tests/unit/connectors/test_activation_api.py (CLI→SSE path: emits on success, nothing on failure/rejection).
  • Docs (docs/sdk/infrastructure/connectors.mdx). Updated the activation SSE section to describe the centralized emit + watcher. No frontend changes — useConnectorsSSE.ts already consumes connector.activation.changed.

Test plan

  • python util/lint.py --all — all quality checks pass (Black, isort, Pylint, Flake8 green; pre-existing MyPy/Bandit/agent-convention warnings only, none in touched files).
  • python -m pytest tests/unit/connectors/test_activation_api.py tests/unit/connectors/test_activation_watcher.py tests/unit/connectors/test_router_connectors.py — 77 passed.
  • python -m pytest tests/unit/connectors/ — 436 passed, 3 skipped.
  • Manual cross-process live update: start the UI server, open Settings → Connectors → expand an MCP connector's "Active for"; in a second terminal run gaia connectors activations activate mcp-github builtin:connectors-demo --scopes use → panel flips to active with no refresh; gaia connectors activations deactivate mcp-github builtin:connectors-demo → flips back live.

Follow-ups (not in this PR — gaps found while doing this work)

  • DispatchQueue worker logs after the test stream closes (pre-existing). Any UI test that boots a TestClient prints repeated --- Logging error --- ValueError: I/O operation on closed file blocks pointing at src/gaia/ui/server.py:267 (_import_modules logging the faiss-load line). Root cause: DispatchQueue.shutdown (src/gaia/ui/dispatch.py:104-105) calls self._executor.shutdown(wait=False), so a boot-init worker thread (the slow faiss import) is still running and logs after pytest closed the captured stream. Cosmetic (tests pass, exit 0) and reproduces on main with this PR stashed. Suggested fix: have shutdown drain in-flight boot jobs with a bounded wait, e.g. await loop.run_in_executor(None, lambda: self._executor.shutdown(wait=True, cancel_futures=True)) under a timeout, so no worker logs post-teardown.
  • StarletteDeprecationWarning: Using httpx with starlette.testclient is deprecated; install httpx2 (pre-existing, repo-wide). Emitted by any test using the shared ui_api_client TestClient fixture (origin tests/conftest.py:355); pytest dedupes it to a single "1 warning" line. Not introduced here (reproduces on main with this PR stashed). Suggested fix: bump to httpx2 per Starlette's guidance, or add a scoped filterwarnings entry in pyproject.toml.

Checklist

  • I have linked a GitHub issue above (Closes #1226).
  • I have described why this change is being made, not just what changed.
  • I have run linting and tests locally (python util/lint.py --all, pytest tests/unit/).
  • I have updated documentation if user-visible behavior changed (see CONTRIBUTING.md).

@github-actions github-actions Bot added documentation Documentation changes tests Test changes labels May 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Solid, focused PR that closes a real UX gap cleanly. The centralization strategy is correct, the dedup mechanism is provably race-free, and the tests cover all the edge cases that matter. Three cosmetic nits below, none blocking.


Summary

CLI/SDK activation writes now flow through api.activate/deactivate, which emit connector.activation.changed through a single path. The file watcher bridges the remaining cross-process gap (CLI is a separate process from the SSE bus). The note_local_write dedup is sound: because emit_change only schedules a task and note_local_write runs synchronously in the same call chain, the event loop can't interleave the watcher's poll_once between them — no double-emit race is possible.


Issues Found

🟢 _RecordingEmitter + recording_emitter fixture duplicated across two test files
(test_activation_api.py:73–99, test_activation_watcher.py:21–37)

Both files define identical classes and fixtures. These belong in tests/unit/connectors/conftest.py so future tests get them for free.

# tests/unit/connectors/conftest.py  (add alongside existing autouse fixtures)

class _RecordingEmitter:
    def __init__(self):
        self.events = []

    async def emit(self, event_type, payload):
        self.events.append((event_type, dict(payload)))


@pytest.fixture
def recording_emitter():
    """Capture events published through the connectors event bus."""
    from gaia.connectors import events

    rec = _RecordingEmitter()
    events.set_emitter(rec)
    try:
        yield rec
    finally:
        events.reset_emitter()

Then remove the duplicate class + fixture from both test files and import nothing — pytest discovers the conftest fixture automatically.


🟢 Issue references in inline comments (activation_watcher.py:48, test_activation_watcher.py:7)

CLAUDE.md: "Don't reference the current task, fix, or callers inline … they belong in the PR description and commit body." The (#1226) tags in the module docstring and test header comment will rot as the code evolves.

"""Background watcher that emits ``connector.activation.changed`` for
out-of-process activation writes.
"""Unit tests for the activation file-watcher.

🟢 deactivate() missing a logger.info that activate() has (src/gaia/connectors/api.py:347–353)

Pre-existing omission (not introduced here), but since this PR is already touching both functions it's a good moment to make them symmetric. No suggestion required — just noting it for awareness.


Strengths

  • emit_change handles both contexts correctly. Loop-present → create_task (non-blocking, logged on failure). Loop-absent → asyncio.run with exception logging. Neither path can break the caller's write.
  • Dedup is elegant and argument-free. note_local_write advances only the written pair, so a concurrent out-of-process change to a different pair is still detected — as test_note_local_write_does_not_mask_other_pair proves explicitly.
  • Test suite covers all meaningful scenarios — diff logic, poll-emit, dedup suppression, partial-pair isolation, failure-path emits nothing — with clean separation between TestDiffLedgers (pure) and TestPollOnce (async + I/O).
  • Router cleanup is a genuine improvement. delete_activation now routes through api.deactivate instead of bypassing the centralized guard, consistent with put_activation.

Verdict

Approve. The nits are cosmetic and don't affect correctness or behavior. The core implementation is well-reasoned, the dedup analysis is correct, and the test coverage is thorough. The fixture consolidation is a nice-to-have that can be done as a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Documentation changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(connectors): emit connector.activation.changed for CLI/SDK activation writes

1 participant