fix(ap): bound interactive AP read calls so a wedged pipeline degrades instead of hanging

[cdeust]

What

The AP MCP client runs with callTimeoutMs=0 — deliberately unbounded, because a fresh index_codebase of a large tree legitimately exceeds any fixed wall clock (see mcp_call_timeout.py). The only backstop is the 600s wedge-silence window.

That's correct for ingestion but wrong for the interactive read path. search_codebase (behind unified_search) and the get_symbol / get_context / get_impact / get_processes / health_check lookups all inherited the same 600s. So an AP that connects but then wedges made unified_search hang for up to 10 minutes before degrading — the "unreachable ⇒ status=partial" promise in unified_search's own docstring was not actually kept. (get_causal_chain has no AP dependency — it is pure knowledge-graph BFS over MemoryStore — and was never affected; an earlier revision of this PR incorrectly claimed otherwise.)

This is the unified_search hang the harness-comparison benchmark hit: a stalled probe, not a graceful Cortex-only fallback.

Fix

APBridge.call gains an optional timeout_s that wraps the client call in asyncio.wait_for and degrades a timeout to None (recorded reason + stderr note, via a shared _degrade helper) — exactly like every other AP failure path. A None from a timed-out search_codebase flows through as_list(None) → [] → unified_search returns status=partial with Cortex-only hits.

• Interactive read wrappers (search_codebase, get_symbol, get_context, get_impact, get_processes, health_check) pass interactive_call_timeout_s() — 30s default, env-overridable via CORTEX_AP_INTERACTIVE_TIMEOUT_S, 20× below the 600s indexing window.
• Indexing/write wrappers (index_codebase, analyze_codebase, resolve_graph, cluster_graph, detect_changes) and the build-loop query_graph stay unbounded — a single build query over a large graph may legitimately run long, and killing a live ingest is the exact regression callTimeoutMs=0 was set to avoid.
• unified_search's response now reflects the per-call outcome, not just the static is_enabled() flag. WorkflowGraphASTSource.search_codebase exposes last_search_degraded_reason (reads the same APBridge._unavailable_reason the timeout/exception path already records — no widened return type, no second round-trip). When AP was attempted but the call failed (timeout, transport error, not installed), the handler sets status="partial" and a degraded: {source: "ap", reason: ...} field, distinct from AP genuinely returning zero hits (status="ok", degraded: null). Previously a per-call timeout and a genuine empty AP result were byte-for-byte indistinguishable in the response.

Source

The 30s ceiling is documented in mcp_call_timeout.py: AP read tools are interactive (unified_search target <200ms per docs/mcp-tools.md); 30s is a wide margin over that yet 20× below the 600s wedge window reserved for indexing.

Tests

tests_py/infrastructure/test_ap_bridge_interactive_timeout.py (fake clients, no subprocess):

• A hanging client returns None within a 50ms ceiling; the unavailable-reason records the timeout and the tool name.
• A call under the ceiling returns its payload; a call with no timeout_s is unbounded (indexing path).
• The search_codebase wrapper forwards a positive interactive_call_timeout_s(); index_codebase forwards None.
• The default ceiling is positive and strictly below default_call_timeout_s() (600s), and honors/validates the env override (malformed/non-positive → default, never unbounded).

tests_py/infrastructure/test_workflow_graph_ast_search_degraded.py (new): a wedged bridge leaves search_codebase returning [] with last_search_degraded_reason set; a disabled AP leaves the reason None (never attempted, not a failure).

tests_py/handlers/test_unified_search_degraded_status.py (new): the handler surfaces status="partial" + degraded={"source":"ap",...} on a wedged AP call, stays status="ok", degraded=None when AP genuinely finds nothing, and stays status="partial", degraded=None when AP is disabled outright.

Gates: ruff check ✓ · ruff format --check ✓ · craftsmanship gate ✓ (fixed a APBridge.call 42-line method-size violation via an extracted _degrade helper, which also removed the duplicated except-branch logic) · pytest tests_py/infrastructure/test_ap_bridge_interactive_timeout.py tests_py/infrastructure/test_workflow_graph_ast_search_degraded.py tests_py/handlers/test_unified_search_degraded_status.py tests_py/infrastructure/test_workflow_graph_source_ast.py tests_py/handlers/test_response_budget_wiring.py → 42 passed.

Completes the trio of bugs the real benchmark surfaced, alongside #447 (FTS5 crash) and #448 (error misclassification).

🤖 Generated with Claude Code

---

Generated by Claude Code

[cdeust]

ZETETIC-REVIEW: REQUEST_CHANGES

Independent review (fresh-context reviewer, read-only via gh, head 3debfda). Three blocking findings:

1. CI is red on this exact commit — Craftsmanship Gate FAILURE: new method-size violation APBridge.call (ap_bridge.py:292-334, 42 lines > repo's 40-line cap), and the CI Green aggregator is FAILURE. The PR body's "craftsmanship gate ✓" claim is incorrect for the head commit. Fix: Extract Method — pull the shared "record reason + stderr note + return None" logic of the two except-branches into one _degrade helper (also removes the duplication).

2. The central claim is false — silent data loss present. unified_search.handler (unified_search.py:122-133) derives status from the static is_enabled() config check, never from the per-call outcome. On an AP timeout, search_codebase returns [], so the response is status="ok", sources=["cortex","ap"], counts.ap=0 — byte-for-byte indistinguishable from "AP found nothing". status=partial only fires when AP integration is disabled in config. Fix in THIS PR (no deferred coverage for a claim the PR itself makes): surface the AP-call outcome up through WorkflowGraphASTSource.search_codebase and reflect it in unified_search's status, with a test exercising the handler against a wedged bridge.

3. False side-claim in sources: get_causal_chain has no AP dependency (get_causal_chain.py imports only MemoryStore/knowledge_graph) and cannot hang on AP — remove that claim from the PR body and the source comments in mcp_call_timeout.py / ap_bridge.py.

Verified sound (kept): the 30s default is properly sourced (docs/mcp-tools.md <200ms interactive target, # source: comment present — §8 pass); ingestion paths stay unbounded (owner rule respected — index/analyze/resolve/cluster/detect_changes and query_graph's two ingestion-loop callers verified); the hang-simulation regression test is genuine (asyncio.Event().wait() cancelled by wait_for within ceiling).

Non-blocking: ap_bridge.py is pre-existing over the file cap (509 lines) and this PR grows it (+50) — follow-up split of the timeout/degrade concern into its own module.

[cdeust]

ZETETIC-REVIEW: APPROVE

Final delta re-review (same fresh-context reviewer across three rounds, head 4e036b7). All blocking findings independently re-verified against the code, not the claims:

1. APBridge.call method-size: fixed via a real _degrade extraction — re-measured at 39 lines (cap 40). Craftsmanship Gate green.
2. Silent-data-loss: unified_search now distinguishes three states — AP disabled (partial, degraded=None), AP attempted-and-failed (partial, degraded={"source":"ap","reason":...}), genuine empty result (ok). Real plumbing (last_search_degraded_reason → APBridge._unavailable_reason), handler-level test proven fail-before/pass-after.
3. False get_causal_chain claim removed from code comments AND the PR body, replaced by an accurate architectural statement.
4. Reason-reset landmine closed: call() resets _unavailable_reason before each attempt; regression test does fail-then-succeed on one reused instance, routed through connect()'s fast path so it cannot pass by accident.
5. .craftsmanship-baseline.json audited byte-for-byte against origin/main: the sync removes exactly the _classify_error entry made stale by PR fix(errors): stop masking query-level errors as 'database_not_connected' #448's merge — nothing grandfathered, the ratchet only shrank.

CI: all checks green on 4e036b7. Green AND verified — mergeable.