fix: MCP UI calls marshal onto the app-owned Textual context — no more NoActiveAppError

[hellices]

Closes #165

Problem

MCP UI-drive calls (and the fire-and-forget follow mirrors) run in tasks created from the ASGI request context, which does not carry Textual's active_app ContextVar. Composing a new widget tree there — DescribeScreen's with VerticalScroll(): — raised NoActiveAppError and terminated the whole app. The issue's audit defines the crash class precisely: composing/mounting new widget trees from a foreign context; mounted-widget updates (navigate/filter/log-pane) were safe only by accident, and log-stream tasks silently carried the MCP request context for the stream's lifetime (a delayed-crash hazard).

Fix — central, at the single boundary

KorvidApp snapshots its execution context in on_mount (inside Textual's message pump, so the snapshot carries active_app + pump vars). AppUIBridge._dispatch then runs every bridge coroutine in a fresh copy of that snapshot (snapshot.run(contextvars.copy_context) — per-call copies because a Context cannot be entered concurrently).

Per the issue's requirements:

• Central boundary — every foreign caller (direct MCP tools, follow mirrors, proposals) crosses AppUIBridge; no per-screen special-casing, no DescribeScreen changes.
• No private-API poking — Textual's active_app is never set from MCP code; the snapshot is taken where Textual itself established it.
• Fire-and-forget preserved — the server-side detach (_follow_tasks) is untouched; MCP responses still never wait on UI rendering.
• Serialization + guards preserved — the _UIBridgeProxy lock wraps the dispatch; approval-dialog and describe-screen user-priority guards run inside the dispatched call as before.
• Downstream-task hazard fixed — tasks spawned inside a dispatched call (log streams) inherit the app context (Task.get_context() proves it).
• Cancellation propagates — a cancelled bridge call cancels and reaps its inner task; shutdown strands nothing (warnings-as-errors in tests would flag stale tasks).

Testing (tests/ui/test_mcp_ui_context.py, RED-first)

Test	Pins
test_describe_from_a_foreign_context_mounts_without_crashing	the issue's minimal repro: agent_open_describe from contextvars.Context() mounts, app survives (was: app-terminating NoActiveAppError)
test_every_ui_tool_crosses_the_boundary_safely	navigate / set_filter / logs from empty contexts
test_downstream_log_tasks_carry_the_app_context	the audit's latent defect: _log_tasks members' Task.get_context() carries active_app (was: MCP request context)
test_concurrent_bridge_calls_from_foreign_contexts_stay_serialized	proxy ordering under the marshaling
test_real_mcp_http_open_describe_and_follow_mirror	full Streamable HTTP round-trip against a running Textual test app: direct open_describe and a follow-mirrored get_resource both mount their describe UI

Full gate green: ruff, mypy --strict, tach, 3040 passed / 21 skipped, coverage ≥ 80%.

[Copilot]

Pull request overview

Marshals MCP-driven UI operations onto Textual’s app-owned context to prevent NoActiveAppError.

Changes:

• Captures the Textual context during app mounting.
• Dispatches all AppUIBridge calls through copied app contexts.
• Adds direct, concurrent, downstream-task, and HTTP regression tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/korvid/ui/app.py	Adds app-context capture and bridge dispatching.
tests/ui/test_mcp_ui_context.py	Adds MCP context-safety regression coverage.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[my-reviewer-agent]

issue #165 수정 승인합니다. 단일 경계(AppUIBridge._dispatch)에서 앱 소유 컨텍스트 스냅샷의 사본으로 모든 브리지 코루틴을 마샬링하는 설계가 정확합니다.

확인한 사항:

• snapshot.run(contextvars.copy_context)는 저장된 Context를 복사하는 올바른 관용구이고, per-call 사본이라 "Context cannot be entered concurrently" 제약을 회피합니다. snapshot.run 호출 자체는 동기라 단일 이벤트 루프 스레드에서 동시 진입이 불가능함도 확인.
• 브리지 10개 메서드 전부 _dispatch 경유로 전환 — 누락 없음.
• 취소 경로: 외부 await 취소 시 내부 task cancel + reap 후 재-raise, 좀비 task 없음.
• 테스트가 실질적: foreign-context 최소 재현, downstream Task.get_context()의 active_app 검증(지연 크래시 클래스), 프록시 직렬화 유지, 실제 Streamable HTTP 왕복(직접 open_describe + follow mirror 둘 다).

사소한 제안 (advisory, 게이트 아님):

1. _dispatch의 except asyncio.CancelledError는 외부 취소와 내부 코루틴이 스스로 CancelledError를 올리는 경우를 구분하지 않습니다. 현재 브리지 대상 메서드들이 자체적으로 CancelledError를 던질 일은 없어 실질 문제는 아니지만, task.cancelled() 확인을 추가하면 의도가 더 명확해집니다.
2. _app_context는 on_mount 시점 1회 스냅샷이라 이후 앱 레벨 ContextVar 변경은 반영되지 않습니다. active_app은 불변이므로 현재로선 안전하나, 향후 앱이 mount 이후 세팅하는 ContextVar에 의존하는 UI 경로가 생기면 스냅샷 갱신 시점을 재고해야 합니다.

APPROVE

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/korvid/ui/app.py:770

• This field comment still describes the old fallback behavior. _dispatch now closes pre-mount coroutines and returns ERROR: UI not ready; it never runs them directly, and production MCP calls can reach this state. Please update the comment so future changes do not reintroduce the unsafe fallback.

        #: None until then (pre-mount bridge calls run directly - they can
        #: only come from tests driving the app before it is ready).

src/korvid/ui/app.py:9839

• The new cancellation/reaping branch has no regression coverage: the HTTP test lets every bridge call finish before shutdown, and the pre-mount test never creates an inner task. The linked issue explicitly requires shutdown to cancel/reap queued UI work. Add a test that blocks an in-flight dispatched coroutine, cancels its foreign caller (or shuts down the server), and verifies the inner task's finally runs with no task left pending.

        except asyncio.CancelledError:
            task.cancel()
            with contextlib.suppress(asyncio.CancelledError):
                await task

tests/ui/test_mcp_ui_context.py:68

• Despite this test's “every UI tool” claim and the linked issue's explicit drill-down acceptance criterion, it never calls agent_drill_down from the empty context. Add a foreign-context drill-down case that exercises a real deployment-to-child transition, so the newly wrapped AppUIBridge.agent_drill_down cannot regress independently.

        nav = await _in_empty_context(bridge.agent_navigate("deployments"))
        assert nav.startswith("switched")
        flt = await _in_empty_context(bridge.agent_set_filter("web"))
        assert "filter set" in flt
        await _in_empty_context(bridge.agent_set_filter(""))
        back = await _in_empty_context(bridge.agent_navigate("pods"))
        assert back.startswith("switched")
        logs = await _in_empty_context(bridge.agent_open_logs("web-1", "default"))

[my-reviewer-agent]

재리뷰 (신규 커밋 1개) — 이전 라운드의 두 지적을 모두 정확히 해소했습니다.

1. pre-mount None-snapshot 폴백 제거: _dispatch가 스냅샷 부재 시 foreign ASGI 컨텍스트에서 코루틴을 그대로 실행하던 폴백이 실제로 프로덕션에서 도달 가능(_start_mcp_if_enabled가 app.run_async()보다 먼저 실행)하다는 점을 인정하고, 'UI not ready' 거부로 교체. coro.close()로 un-awaited 코루틴 경고까지 차단하고, 테스트에서 warnings.simplefilter("error")로 누수 없음을 실제로 고정한 점이 좋습니다.
2. 3.12 전용 Task.get_context() 의존 제거: downstream 상속 테스트가 스폰된 스트림 코루틴 내부에서 active_app.get(None)을 기록하는 방식으로 전환 — 3.11 CI에서도 동작하며 seen == [app] 단언으로 비공허(non-vacuous)합니다.

지적 사항 없음. APPROVE

[hellices]

라운드 2 suppressed 3건 모두 반영:

• _app_context 필드 주석을 현재의 refusal 계약으로 갱신 (직접 실행 폴백 서술 제거).
• 취소/회수 회귀 테스트 — in-flight dispatched 코루틴의 foreign caller를 취소하면 내부 태스크의 finally가 실행되고 잔류 태스크가 없음을 고정 (test_cancelled_foreign_caller_reaps_the_dispatched_task).
• drill-down 수용 기준 — 빈 컨텍스트에서 실제 deployments→replicasets 전환 케이스 추가 (test_drill_down_crosses_the_boundary_safely).

Full gate 그린 (3043 passed).

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

tests/ui/test_mcp_ui_context.py:232

• These fixed sleeps reintroduce the slow-CI flakiness that tests/ui/waits.py:1-4 explicitly avoids. Wait for the deployment watch data that agent_drill_down consumes instead of assuming it arrives within 100 ms.

        await pilot.pause(0.1)
        await app.on_navigate_command(NavigateCommand("deployments", None))
        await pilot.pause(0.1)

[my-reviewer-agent]

재리뷰 — 신규 커밋 1건 (f25c41f, 테스트/주석만).

• _app_context 주석이 실제 동작과 일치하도록 수정: pre-mount 브리지 호출은 'UI not ready' ERROR로 거부되며(프로덕션에서 MCP 엔드포인트가 app.run_async()보다 먼저 뜰 수 있음) foreign context에서 직접 실행되지 않음 — 지난 리뷰에서 확인한 동작을 문서가 정확히 반영하게 됨.
• 이슈 #165 수용 기준 테스트 2건 추가: (1) 빈 컨텍스트에서 실제 deployments→replicasets drill-down이 ERROR 없이 통과하고 current_kind가 전환됨을 until()로 확인, (2) foreign caller 취소 시 dispatch된 내부 task가 reap되어 finally가 실행됨을 Event 3개(entered/release/cleaned)로 비유사(vacuous)하지 않게 고정 — 취소가 전파되고(pytest.raises(CancelledError)) 정리가 timeout 내 완료됨을 모두 단언.

지적사항 없음. 프로덕션 로직 변경 없이 커버리지와 문서 정확성만 개선한 커밋입니다.

APPROVE

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[my-reviewer-agent]

재리뷰 (신규 커밋 1건) — 셧다운 레이스 마감 확인, 승인합니다.

• _reap_dispatches: _app_context = None을 취소 전에 동기적으로 설정하므로, _dispatch의 None 체크와 _dispatch_tasks.add(task)가 하나의 이벤트 루프 스텝에서 일어나는 구조와 맞물려 '거부도 안 되고 reap도 안 되는' 창이 존재하지 않음을 확인했습니다. gather(*self._dispatch_tasks)는 언팩 후 done_callback의 discard가 set을 변경해도 안전합니다.
• on_unmount 최상단에서 reap을 먼저 수행해 이후 teardown sweep(로그 스트림 취소 등)보다 앞서는 순서도 올바릅니다.
• 신규 테스트는 in-flight dispatch의 finally 실행(reaped), 호출자 CancelledError 전파, 셧다운 후 호출의 ERROR 거부까지 모두 단언하는 비공허(non-vacuous) 테스트입니다. 드릴다운 테스트의 fixed pause → until() 조건 폴링 전환도 waits.py 규칙에 부합합니다.

지적사항 없음.

APPROVE