Skip to content

docs(F153): Phase I spec — Step Summary (Agent Loop count) - #2

Merged
bouillipx merged 4 commits into
mainfrom
feat/F153-step-summary
May 19, 2026
Merged

docs(F153): Phase I spec — Step Summary (Agent Loop count)#2
bouillipx merged 4 commits into
mainfrom
feat/F153-step-summary

Conversation

@bouillipx

Copy link
Copy Markdown

Summary

  • Define F153 Phase I: Step Summary — first-class agent-behavior view in Hub observability, with agent_loop_count (= cat_cafe.llm_call span count per route) as the primary step metric.
  • Reuse existing telemetry; only new instrument is a2a.dispatch.count counter on mention_dispatch span creation.
  • Stay descriptive (KD-16 / new KD-32): no efficiency/quality scoring; UI shows for restored spans where sub-step hierarchy was flattened by hydrate-traces.ts.

Multi-cat alignment thread 2026-05-19: 铲屎官 + 砚砚(缅因猫) + 宪宪(布偶猫). Refs zts212653#721.

Why this naming

Industry convention picks the agent-behavior unit, not the implementation unit:

  • LangChain AgentExecutor, Anthropic SDK agent loop, AutoGPT step, OpenAI Assistants run step, DeepEval StepEfficiencyMetric — all use agent loop / step.
  • LLM call is the implementation anchor, agent loop is the behavior concept. Same count, better name.

Boundary: one agent loop = one cat_cafe.llm_call + 0..N tool_use + tool result observations. A pure reply (width=0) still counts as one step — the decision point exists.

Test plan

  • Spec proofread by 砚砚 — AC-I1..AC-I7 correctness, KD-31/KD-32 reasoning
  • Confirm mention_dispatch span path / Phase G integration assumption (AC-I3 a2a.dispatch.count placement)
  • Confirm Phase H dependency direction for mcp_tool_call_count vs basic_tool_call_count split (currently out-of-scope, deferred to Phase H真实执行边界)
  • Implementation PR will follow this spec; this PR is docs-only

[宪宪/Opus-47🐾]

Define a first-class "step length" view for the Hub observability plane.

- Step = one agent loop, boundary anchor = cat_cafe.llm_call span
- Length × Width model (depth + avg tools per loop)
- Reuse existing telemetry; add 1 new counter (a2a.dispatch.count)
- Live vs Restored visual distinction (— marker for unrecoverable subcounts)
- AC-I1..AC-I7
- KD-31 (step = agent loop), KD-32 (descriptive only, no quality score)

Naming aligns with industry: LangChain AgentExecutor, Anthropic SDK
agent loop, AutoGPT step, OpenAI run step, DeepEval StepEfficiencyMetric
all use agent loop / step as the basic unit — not LLM call, not tool call.

Refs zts212653#721

[宪宪/Opus-47🐾]

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
bouillipx and others added 3 commits May 19, 2026 16:13
P1-1 (Finding 1): Change agent loop anchor from cat_cafe.llm_call span
to assistant message done event. llm_call span only created when
durationApiMs available (invoke-single-cat.ts:1408-1410); Codex/Gemini/
Kimi don't produce it — would yield 0/missing loops for those providers.
done event path is provider-agnostic.

P1-2 (Finding 2): Rename aggregate counter to cat_cafe.a2a.dispatch.count
and remove invocationId from its attributes. metric-allowlist.ts:8-9
forbids high-cardinality attributes on metrics. Per-route a2a_dispatch_count
now derives from cat_cafe.mention_dispatch span count instead.

P2-1 (Finding 3): tool_call_count goes dual-track — child cat_cafe.tool_use
spans (MCP/business) + invocationSpan tool.basic_call_count attribute (basic
tools, per span-helpers.ts:81-96 design to avoid trace tree flooding).

P2-2 (Finding 4): token_total sources from cat_cafe.route span
ROUTE_TOTAL_TOKENS attribute (route-serial.ts:1900), not the token.usage
metric instrument (no route dimension on metrics).

Add KD-33/34/35 capturing the data-source corrections.

[宪宪/Opus-47🐾]

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
P1 (Finding 1 v2): Loop boundary is NOT observable from done event.
done event yields once per CLI invocation (ClaudeAgentService.ts:476,
CodexAgentService.ts:709), and cat_cafe.llm_call span also goes through
the done path (msg.metadata.usage is consumed only in msg.type === 'done'
branch, invoke-single-cat.ts:1308/1387) — both are per-invocation, not
per-loop. Spec now requires per-provider stream parser to identify LLM
call boundaries and emit a unified cat_cafe.agent_loop marker; providers
without an identifiable boundary signal show '—' (NOT degraded to
invocation count).

P2 (Finding 2): cat_cafe.a2a.dispatch.count bounded labels reduced to
AGENT_ID only. CALLBACK_TOOL/CALLBACK_REASON are callback auth failure
semantics, unrelated to dispatch; if dispatch.source/status labels are
needed during implementation, the metric-allowlist must be extended
first.

Update KD-31 (anchor = stream marker, not done event), KD-33 (marker
strategy), KD-34 (label set). Reflect new marker/counter in section
header.

[宪宪/Opus-47🐾]

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
P3 (Maine Coon review 3rd round): AC-I7 explicitly requires test
coverage for the "live provider without cat_cafe.agent_loop marker
shows '—', not invocation count" case — Phase I's most critical
non-degradation boundary.

[宪宪/Opus-47🐾]

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@bouillipx
bouillipx merged commit b36540c into main May 19, 2026
@bouillipx
bouillipx deleted the feat/F153-step-summary branch May 19, 2026 08:25
bouillipx added a commit to zts212653/clowder-ai that referenced this pull request May 20, 2026
F153 Phase I implementation. Closes #721.

Surfaces agent_loop_count as the primary "step length" per cat_cafe.route
in the Hub Traces tab, with Length × Width visualization. Descriptive
only (per KD-16/KD-32, no efficiency or quality scoring).

- Register cat_cafe.a2a.dispatch.count counter (AGENT_ID-only attribute
  per KD-34, metric-allowlist compliant) wired at both dispatch sites
  (route-serial in-process + dispatch-span callback) with lazy creation
  so phantom dispatches under depth-limit / dedup / ping-pong / fallback
  don't pollute counts.
- recordAgentLoop(invocationSpan) helper using WeakMap + setAttribute
  pattern (analogous to tool.basic_call_count). Provider-agnostic, does
  NOT depend on durationApiMs or done event (both per-invocation, not
  per-loop — KD-33).
- cat_cafe.agent_loop stream marker emitted by Claude provider at
  message_stop event (the true per-LLM-call boundary in Claude NDJSON
  stream). Other providers (Codex/Gemini/Kimi/Antigravity/OpenCode/DARE/
  A2A) don't emit yet — Step Summary shows '—' per AC-I2/I7
  non-degradation rule.
- agent_loop is telemetry-only at invoke-single-cat: returned via
  return outputs (empty array), never pushed to outputs/transcript/
  downstream forwarding.
- GET /api/telemetry/step-summary route — session-authed, reads full
  buffer via traceStore.stats().maxSpans, returns descriptive metrics
  + Length × Width derivation + is_restored flag.
- computeStepSummary aggregation: dual-track tool count (MCP child spans
  + tool.basic_call_count attribute per KD-35), span-derived
  a2a_dispatch_count (KD-34), ROUTE_TOTAL_TOKENS token total (KD-35),
  null-propagation for restored / no-marker cases.

- HubTraceTree.tsx StepSummaryPanel inside expanded TraceCard:
  6 metrics grid + Length × Width line + "Restored (history)" badge +
  '—' rendering for null/missing/restored sub-counts (never '0').

- 21 source-level + behavioral tests:
  * Counter wiring (instruments + dispatch-span + route-serial)
  * recordAgentLoop helper + Claude parser message_stop emit
  * AgentMessageType extension + invoke-single-cat telemetry-only branch
  * step-summary route registration + lazy worklist regression
  * Behavioral: live aggregation, restored returns null, missing marker
    returns null (NOT 1 — non-degradation), no normative fields,
    error_count by status code, 600-span long-trace non-truncation.

This PR was developed and reviewed on clowder-labs/clowder-ai
(internal fork) before being submitted upstream:
- clowder-labs#2: spec PR (3-round Maine Coon review)
- clowder-labs#3: implementation PR (3 review rounds —
  P1×2 + P2-1 + P2-2 all addressed; 砚砚 final approval
  "放行延续到 0de3046. No findings.")
- clowder-labs#4: governance split (dir-exceptions
  extension) — owner gpt52 + ADR-010 reviewer 砚砚 both approved.

- KD-31: step = one agent loop, anchored on cat_cafe.agent_loop stream
  marker
- KD-32: Phase I descriptive only, no efficiency/quality score
- KD-33: done event / llm_call span are per-invocation; marker required
- KD-34: metric counter only AGENT_ID; route dim via span count
- KD-35: tool dual-track (MCP span + basic counter attr); token via
  ROUTE_TOTAL_TOKENS

- Other providers' agent_loop marker emit (Codex/Gemini/Kimi/etc) —
  will follow in subsequent phases; non-emitting providers display '—'.
- Task-level step length — needs cross-invocation task_id primitive.
- Step efficiency / quality eval — descriptive-plane boundary (KD-16).

[宪宪/Opus-47🐾]

Co-Authored-By: Maine Coon Codex (砚砚, GPT-5.5) <noreply@anthropic.com>
Co-Authored-By: Maine Coon GPT-5.4 (gpt52) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zts212653 pushed a commit to zts212653/clowder-ai that referenced this pull request May 22, 2026
* feat(F153): Phase I — Step Summary (Agent Loop count) — community PR

F153 Phase I implementation. Closes #721.

Surfaces agent_loop_count as the primary "step length" per cat_cafe.route
in the Hub Traces tab, with Length × Width visualization. Descriptive
only (per KD-16/KD-32, no efficiency or quality scoring).

- Register cat_cafe.a2a.dispatch.count counter (AGENT_ID-only attribute
  per KD-34, metric-allowlist compliant) wired at both dispatch sites
  (route-serial in-process + dispatch-span callback) with lazy creation
  so phantom dispatches under depth-limit / dedup / ping-pong / fallback
  don't pollute counts.
- recordAgentLoop(invocationSpan) helper using WeakMap + setAttribute
  pattern (analogous to tool.basic_call_count). Provider-agnostic, does
  NOT depend on durationApiMs or done event (both per-invocation, not
  per-loop — KD-33).
- cat_cafe.agent_loop stream marker emitted by Claude provider at
  message_stop event (the true per-LLM-call boundary in Claude NDJSON
  stream). Other providers (Codex/Gemini/Kimi/Antigravity/OpenCode/DARE/
  A2A) don't emit yet — Step Summary shows '—' per AC-I2/I7
  non-degradation rule.
- agent_loop is telemetry-only at invoke-single-cat: returned via
  return outputs (empty array), never pushed to outputs/transcript/
  downstream forwarding.
- GET /api/telemetry/step-summary route — session-authed, reads full
  buffer via traceStore.stats().maxSpans, returns descriptive metrics
  + Length × Width derivation + is_restored flag.
- computeStepSummary aggregation: dual-track tool count (MCP child spans
  + tool.basic_call_count attribute per KD-35), span-derived
  a2a_dispatch_count (KD-34), ROUTE_TOTAL_TOKENS token total (KD-35),
  null-propagation for restored / no-marker cases.

- HubTraceTree.tsx StepSummaryPanel inside expanded TraceCard:
  6 metrics grid + Length × Width line + "Restored (history)" badge +
  '—' rendering for null/missing/restored sub-counts (never '0').

- 21 source-level + behavioral tests:
  * Counter wiring (instruments + dispatch-span + route-serial)
  * recordAgentLoop helper + Claude parser message_stop emit
  * AgentMessageType extension + invoke-single-cat telemetry-only branch
  * step-summary route registration + lazy worklist regression
  * Behavioral: live aggregation, restored returns null, missing marker
    returns null (NOT 1 — non-degradation), no normative fields,
    error_count by status code, 600-span long-trace non-truncation.

This PR was developed and reviewed on clowder-labs/clowder-ai
(internal fork) before being submitted upstream:
- clowder-labs#2: spec PR (3-round Maine Coon review)
- clowder-labs#3: implementation PR (3 review rounds —
  P1×2 + P2-1 + P2-2 all addressed; 砚砚 final approval
  "放行延续到 0de3046. No findings.")
- clowder-labs#4: governance split (dir-exceptions
  extension) — owner gpt52 + ADR-010 reviewer 砚砚 both approved.

- KD-31: step = one agent loop, anchored on cat_cafe.agent_loop stream
  marker
- KD-32: Phase I descriptive only, no efficiency/quality score
- KD-33: done event / llm_call span are per-invocation; marker required
- KD-34: metric counter only AGENT_ID; route dim via span count
- KD-35: tool dual-track (MCP span + basic counter attr); token via
  ROUTE_TOTAL_TOKENS

- Other providers' agent_loop marker emit (Codex/Gemini/Kimi/etc) —
  will follow in subsequent phases; non-emitting providers display '—'.
- Task-level step length — needs cross-invocation task_id primitive.
- Step efficiency / quality eval — descriptive-plane boundary (KD-16).

[宪宪/Opus-47🐾]

Co-Authored-By: Maine Coon Codex (砚砚, GPT-5.5) <noreply@anthropic.com>
Co-Authored-By: Maine Coon GPT-5.4 (gpt52) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(F153 Phase I): cover F185 deferred A2A path with mention_dispatch span + counter + trace ctx

砚砚 R1 P2: route-serial 的 F185 fairness-gate deferred branch 只调 deferA2AEnqueue 入队,
跳过了 inline path 的 mention_dispatch span 创建 / a2aDispatchCount.add / caller trace
context 注入,造成两个观测断点:
- Step Summary 的 a2a_dispatch_count 在 fairness gate 触发时低报
- 通过 fairness gate 进入 InvocationQueue 的 child route 失去跨 route causality

修复:
- route-helpers.ts: deferA2AEnqueue entry type 增加 callerTraceContext 字段
- route-serial.ts: deferred path 镜像 inline path :1661-1675 — lazy 创建 mention_dispatch
  span(dispatch.source='text-scan-deferred' 区分 inline/callback),立刻 end span
  (child route 在另一个 process loop 跑),把 span ctx 通过 entry 传到 QueueProcessor,
  由后者作为 caller trace context 重建 remote parent

新增 5 个 regression tests 覆盖:
- source-string × 4:span 创建、counter+1、entry callerTraceContext 字段、entry type 契约
- behavioral × 1:完整 dispatch → queue handoff → 重建 → 重新 parenting 链路

[宪宪/Opus-47🐾]
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* style(F153): biome format mention-dispatch-trace test for CI Lint

Per 砚砚 R2: biome collapses assert.ok(...) and otelTracer.startSpan(...) to single-line.
Tests still 37/37 pass.

[宪宪/Opus-47🐾]
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(F153): scope Step Summary to per-route subtree + mixed-provider coverage

Maintainer inbound review (#732 comment):

1. Route vs trace scope — counts were trace-wide while duration/tokens
   came from one route span. Now computeStepSummary scopes to a single
   cat_cafe.route subtree (collectSubtree BFS on parentSpanId). API
   accepts optional routeSpanId; UI auto-detects root route.

2. Mixed-provider coverage — new agent_loop_partial flag (true when some
   live invocations have agent_loop.count and others don't). UI renders
   count as "N+" to indicate lower bound.

3. Issue gate — added triaged + feature:F153 labels to #721.

4. F027 anchor normalization — fixed F27 → F027 in test comment.

Tests: +4 behavioral (multi-route subtree, auto-detect root, mixed
partial, full coverage), updated frontend source test.

[宪宪/Opus-46🐾]

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(F153): root-route auto-detect checks trace ancestry, not route-only

Maintainer re-review P1: child route parented under mention_dispatch was
misidentified as root (parent not a route ≠ parent absent from trace).

Fix: auto-detect now picks the route span whose parentSpanId is absent
from the entire trace span set, with earliest startTimeMs tiebreaker.
UI no longer passes routeSpanId — relies on corrected backend.

Regression test updated to real A2A shape (route2.parentSpanId = dispatch,
not route1) + newest-first ordering case proving auto-detect stability.

[宪宪/Opus-46🐾]

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Maine Coon Codex (砚砚, GPT-5.5) <noreply@anthropic.com>
bouillipx pushed a commit that referenced this pull request May 25, 2026
…ed wiring (zts212653#695)

* fix(embedding): batch splitting + reprobe + collection embed deps

Bug #1: Extract embedIndexedItems from IndexBuilder into shared
embed-utils.ts with 64-item batch splitting. IndexBuilder's private
version sent all items at once — embed-api MAX_BATCH_SIZE=64 caused
>64 items to fail with HTTP 400, silently swallowed by fail-open.

Bug #2: Add reprobeIfNeeded() to EmbeddingService. load() probes
/health once at startup; if embed-api starts later, isReady() stays
false forever. reprobeIfNeeded() re-probes when not ready.

Bug #6: Wire embedDeps into CollectionIndexBuilder and library.ts
rebuild route. Collection rebuild now produces vector embeddings
(was FTS-only). Also pass force param from request body (was ignored).

Upstream: zts212653#693

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
[宪宪/Opus-46🐾]

* fix(embedding): wire production call sites for reprobeIfNeeded and per-collection VectorStore

P1 fixes from gpt55 review:
- embed-utils.ts: call reprobeIfNeeded() before isReady() check so all
  callers (IndexBuilder + CollectionIndexBuilder) get late-start recovery
- library.ts: accept embeddingService instead of static embedDeps;
  dynamically load sqlite-vec and create per-collection VectorStore at
  rebuild time (prevents cross-DB vector pollution)
- index.ts: pass memoryServices.embeddingService to libraryRoutes

Tests: add reprobe recovery test, update all embedding mocks with
reprobeIfNeeded method. 68/68 embedding tests pass.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(embedding): wire setEmbedDeps on collection store for search-time hybrid

P1 from gpt55 R2: rebuild writes vectors via CollectionIndexBuilder but
the collection's SqliteEvidenceStore never gets setEmbedDeps, so search()
stays lexical-only. Now the rebuild route calls store.setEmbedDeps() after
constructing the per-collection VectorStore, using the system embedMode.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: tianyiliang <tianyiliang@tianyiliangdeMacBook-Pro.local>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Lysander Su <773678591@qq.com>
buproof pushed a commit to buproof/clowder-ai that referenced this pull request Jun 29, 2026
* feat(F153): Phase I — Step Summary (Agent Loop count) — community PR

F153 Phase I implementation. Closes zts212653#721.

Surfaces agent_loop_count as the primary "step length" per cat_cafe.route
in the Hub Traces tab, with Length × Width visualization. Descriptive
only (per KD-16/KD-32, no efficiency or quality scoring).

- Register cat_cafe.a2a.dispatch.count counter (AGENT_ID-only attribute
  per KD-34, metric-allowlist compliant) wired at both dispatch sites
  (route-serial in-process + dispatch-span callback) with lazy creation
  so phantom dispatches under depth-limit / dedup / ping-pong / fallback
  don't pollute counts.
- recordAgentLoop(invocationSpan) helper using WeakMap + setAttribute
  pattern (analogous to tool.basic_call_count). Provider-agnostic, does
  NOT depend on durationApiMs or done event (both per-invocation, not
  per-loop — KD-33).
- cat_cafe.agent_loop stream marker emitted by Claude provider at
  message_stop event (the true per-LLM-call boundary in Claude NDJSON
  stream). Other providers (Codex/Gemini/Kimi/Antigravity/OpenCode/DARE/
  A2A) don't emit yet — Step Summary shows '—' per AC-I2/I7
  non-degradation rule.
- agent_loop is telemetry-only at invoke-single-cat: returned via
  return outputs (empty array), never pushed to outputs/transcript/
  downstream forwarding.
- GET /api/telemetry/step-summary route — session-authed, reads full
  buffer via traceStore.stats().maxSpans, returns descriptive metrics
  + Length × Width derivation + is_restored flag.
- computeStepSummary aggregation: dual-track tool count (MCP child spans
  + tool.basic_call_count attribute per KD-35), span-derived
  a2a_dispatch_count (KD-34), ROUTE_TOTAL_TOKENS token total (KD-35),
  null-propagation for restored / no-marker cases.

- HubTraceTree.tsx StepSummaryPanel inside expanded TraceCard:
  6 metrics grid + Length × Width line + "Restored (history)" badge +
  '—' rendering for null/missing/restored sub-counts (never '0').

- 21 source-level + behavioral tests:
  * Counter wiring (instruments + dispatch-span + route-serial)
  * recordAgentLoop helper + Claude parser message_stop emit
  * AgentMessageType extension + invoke-single-cat telemetry-only branch
  * step-summary route registration + lazy worklist regression
  * Behavioral: live aggregation, restored returns null, missing marker
    returns null (NOT 1 — non-degradation), no normative fields,
    error_count by status code, 600-span long-trace non-truncation.

This PR was developed and reviewed on clowder-labs/clowder-ai
(internal fork) before being submitted upstream:
- clowder-labs#2: spec PR (3-round Maine Coon review)
- clowder-labs#3: implementation PR (3 review rounds —
  P1×2 + P2-1 + P2-2 all addressed; 砚砚 final approval
  "放行延续到 0de3046. No findings.")
- clowder-labs#4: governance split (dir-exceptions
  extension) — owner gpt52 + ADR-010 reviewer 砚砚 both approved.

- KD-31: step = one agent loop, anchored on cat_cafe.agent_loop stream
  marker
- KD-32: Phase I descriptive only, no efficiency/quality score
- KD-33: done event / llm_call span are per-invocation; marker required
- KD-34: metric counter only AGENT_ID; route dim via span count
- KD-35: tool dual-track (MCP span + basic counter attr); token via
  ROUTE_TOTAL_TOKENS

- Other providers' agent_loop marker emit (Codex/Gemini/Kimi/etc) —
  will follow in subsequent phases; non-emitting providers display '—'.
- Task-level step length — needs cross-invocation task_id primitive.
- Step efficiency / quality eval — descriptive-plane boundary (KD-16).

[宪宪/Opus-47🐾]

Co-Authored-By: Maine Coon Codex (砚砚, GPT-5.5) <noreply@anthropic.com>
Co-Authored-By: Maine Coon GPT-5.4 (gpt52) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(F153 Phase I): cover F185 deferred A2A path with mention_dispatch span + counter + trace ctx

砚砚 R1 P2: route-serial 的 F185 fairness-gate deferred branch 只调 deferA2AEnqueue 入队,
跳过了 inline path 的 mention_dispatch span 创建 / a2aDispatchCount.add / caller trace
context 注入,造成两个观测断点:
- Step Summary 的 a2a_dispatch_count 在 fairness gate 触发时低报
- 通过 fairness gate 进入 InvocationQueue 的 child route 失去跨 route causality

修复:
- route-helpers.ts: deferA2AEnqueue entry type 增加 callerTraceContext 字段
- route-serial.ts: deferred path 镜像 inline path :1661-1675 — lazy 创建 mention_dispatch
  span(dispatch.source='text-scan-deferred' 区分 inline/callback),立刻 end span
  (child route 在另一个 process loop 跑),把 span ctx 通过 entry 传到 QueueProcessor,
  由后者作为 caller trace context 重建 remote parent

新增 5 个 regression tests 覆盖:
- source-string × 4:span 创建、counter+1、entry callerTraceContext 字段、entry type 契约
- behavioral × 1:完整 dispatch → queue handoff → 重建 → 重新 parenting 链路

[宪宪/Opus-47🐾]
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* style(F153): biome format mention-dispatch-trace test for CI Lint

Per 砚砚 R2: biome collapses assert.ok(...) and otelTracer.startSpan(...) to single-line.
Tests still 37/37 pass.

[宪宪/Opus-47🐾]
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(F153): scope Step Summary to per-route subtree + mixed-provider coverage

Maintainer inbound review (zts212653#732 comment):

1. Route vs trace scope — counts were trace-wide while duration/tokens
   came from one route span. Now computeStepSummary scopes to a single
   cat_cafe.route subtree (collectSubtree BFS on parentSpanId). API
   accepts optional routeSpanId; UI auto-detects root route.

2. Mixed-provider coverage — new agent_loop_partial flag (true when some
   live invocations have agent_loop.count and others don't). UI renders
   count as "N+" to indicate lower bound.

3. Issue gate — added triaged + feature:F153 labels to zts212653#721.

4. F027 anchor normalization — fixed F27 → F027 in test comment.

Tests: +4 behavioral (multi-route subtree, auto-detect root, mixed
partial, full coverage), updated frontend source test.

[宪宪/Opus-46🐾]

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(F153): root-route auto-detect checks trace ancestry, not route-only

Maintainer re-review P1: child route parented under mention_dispatch was
misidentified as root (parent not a route ≠ parent absent from trace).

Fix: auto-detect now picks the route span whose parentSpanId is absent
from the entire trace span set, with earliest startTimeMs tiebreaker.
UI no longer passes routeSpanId — relies on corrected backend.

Regression test updated to real A2A shape (route2.parentSpanId = dispatch,
not route1) + newest-first ordering case proving auto-detect stability.

[宪宪/Opus-46🐾]

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Maine Coon Codex (砚砚, GPT-5.5) <noreply@anthropic.com>
bouillipx pushed a commit that referenced this pull request Jul 7, 2026
…ce (zts212653#713)

* fix(zts212653#712): unify MCP config to capabilities.json single source of truth

Problem: MCP server configuration was scattered across multiple provider
config files (.mcp.json, config.toml, settings.json, mcp.json). Each
provider had its own writer, leading to inconsistencies when toggling
MCP servers — changes in capabilities.json were overwritten by stale
provider configs on restart.

Solution: capabilities.json is now the single source of truth for all
MCP configuration. Provider config files are generated from it (not the
reverse). Key changes:

- Remove bidirectional provider writers; capabilities.json → provider
  configs is one-way generation
- MCP discovery runs once (gated by discoveryVersion), not on every GET
- External config files (.mcp.json etc.) are legacy artifacts, no longer
  authoritative
- Extract shared MCP service for plugin activation path
- Unify per-cat state to blockedCats (blacklist), remove legacy
  overrides field
- Frontend MCP toggle derives state from raw data (cats/globalEnabled)
  matching Skills pattern, not from backend computed enabled field
- Clean up all legacy `?? cap.enabled` fallbacks — globalEnabled is
  the sole runtime source of truth
- Multi-project MCP sync management (F249): cascade global MCP state
  to registered projects, drift detection, per-project blockedCats

Closes zts212653#712

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(test): update f041 tests to use globalEnabled + blockedCats

Tests were using legacy `enabled` + `overrides` fields that
`isMcpEnabledForCat` no longer reads after the zts212653#712 cleanup.
At runtime, `readCapabilitiesConfig` fills `globalEnabled` from
`enabled` via in-memory migration, so raw pre-migration data never
reaches `resolveServersForCat` — but tests construct configs directly.

- Add `globalEnabled` alongside `enabled` in config round-trip and
  hot-reload tests
- Rewrite per-cat override tests to use `blockedCats` instead of
  deprecated `overrides` field
- Update test names to reflect canonical model semantics

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(test): add globalEnabled to capability-orchestrator tests

Two more tests in capability-orchestrator.test.js used `enabled: false`
without `globalEnabled`, causing isMcpEnabledForCat and
isSameRepoExternalSplit to treat them as enabled (globalEnabled ?? true).

- ensureCatCafeMainServer: disabled cat-cafe-limb now has globalEnabled: false
- resolveServersForCat: disabled MCP entry now has globalEnabled: false

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(codex): include dummy command in disabled MCP --config entries

Codex CLI ≥0.142 validates transport for ALL mcp_servers entries,
including disabled ones. A bare `enabled=false` override (no command,
no type) fails with "invalid transport in mcp_servers.<name>".

Add `command="echo"` + `args=["disabled"]` to disabled entries,
matching the existing legacy `cat-cafe` shim pattern that already
works. This gives Codex enough info to infer stdio transport without
actually starting the server.

Root cause: buildCatCafeMcpArgs injected `--config
mcp_servers.<name>.enabled=false` for globally-disabled capabilities
(e.g. cat-cafe-memory with globalEnabled:false), but omitted
command/args. Codex couldn't determine transport → validation error.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(codex): skip disabled MCP entries instead of injecting enabled=false

Disabled capabilities don't need CLI --config overrides — L5
writeCodexMcpConfig already deletes disabled managed entries from
.codex/config.toml. Injecting a bare `enabled=false` (no command,
no type) caused Codex CLI ≥0.142 "invalid transport" validation
errors. The previous commit added dummy command/args as a workaround;
this commit removes the override entirely, which is simpler and
correct: no stale config.toml entry to override → no override needed.

The legacy `cat-cafe` shim remains as the only disabled override —
user-level ~/.codex/config.toml may have old entries that L5 cannot
reach.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(zts212653#712): restore legacy enabled fallback in isMcpEnabledForCat

Invoke-time code paths (Codex, Claude, Kimi, OpenCode, ACP) read
capabilities.json with raw readFileSync, bypassing the in-memory
migration in readCapabilitiesConfig that copies enabled→globalEnabled.
When a capability has enabled:false but no globalEnabled field,
isMcpEnabledForCat(cap.globalEnabled ?? true) returned true — wrong.

Add ?? cap.enabled fallback so legacy configs without globalEnabled
are still respected. globalEnabled takes precedence when present.

Includes regression test covering:
- legacy enabled:false (no globalEnabled) → disabled
- legacy enabled:true (no globalEnabled) → enabled
- globalEnabled:false overrides legacy enabled:true

Review finding: P1 #4 from maintainer review on PR zts212653#713.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* chore: add tips_exempt to F249 feature doc

Design-phase spec — capability tips will be added when the feature
reaches implementation. Fixes pnpm check gate failure.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(security): harden probe endpoint with local+owner gates (zts212653#712 review)

P1: POST /api/mcp/:id/tools accepted ad-hoc command/args/url from
request body with only resolveUserId() — a trusted browser origin
without session got default-user, enabling arbitrary command execution
(stdio probe) and SSRF with env-var exfiltration (HTTP probe headers).

Fix: always require requireLocalCapabilityWriteRequest (blocks remote
access since probe spawns child processes); ad-hoc probes (body.command
or body.url) additionally require resolveOwnerGate.

P2: invalid projectPath silently fell back to STARTUP_REPO_ROOT instead
of returning 400 (inconsistent with drift-check route at line 123-126).

Fix: return 400 on validateProjectPath failure.

Failure-mode audit: both findings are "sibling route access control
inconsistency" — drift-resolve/sync-all use requireMcpWriteAccess
(session+local+owner) but probe only used resolveUserId. Now aligned.

Review: 缅因猫 GPT-5.4 @gpt52, PR zts212653#713.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(zts212653#712): address maintainer deep review P1 findings

P1 #9: drift-resolve resolutions array now validated — each entry must
have string mcpId and decision fields. Prevents malicious injection.

P1 #10: probeHttpMcp now validates URL scheme — only http: and https:
allowed. Prevents SSRF via file:// / gopher:// etc.

P2 #18: Fixed 7 remaining F240→F249 references in MCP test comments
(capabilities-route.test.js + capabilities-mcp-write-route.test.js).
Connector test F240 references left unchanged (correct feature ID).

Review: maintainer deep review on PR zts212653#713.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(zts212653#712): review R2 quick fixes — atomic write, safe access, globalEnabled, enum validation

P1 #2: writeCapabilitiesConfig now uses temp file + rename for atomic writes
P1 #7: installMcpCapability replaces non-null assertion with safe index access
P1 #10: drift-resolve resolutions validation adds enum check + array length cap
P2 #13: removeMcpCapability sets globalEnabled=false alongside legacy enabled

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test(zts212653#712): add behavior tests for drift/sync subsystem (review R2 P1#12 blocker)

23 tests covering mcp-drift-detector, mcp-drift-resolver, mcp-sync-engine:
- Drift detector: global-new/project-orphan/config-mismatch detection
- Drift resolver: add/remove/update/skip with use-global and keep-project
- Sync engine: canonicalJson, hash determinism, extractMcpEntries
- Sync project: add new, update changed, skip overrides, remove orphans

Drift detector tests use inline config params (no file I/O).
Resolver and sync engine tests use real temp dirs with capabilities.json.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(zts212653#712): update drift-resolver test for atomic write compatibility

The existing test made capabilities.json read-only to block writes, but
atomic write (temp file + rename) bypasses file-level chmod — rename()
needs only directory write permission on POSIX. Now makes the .cat-cafe/
directory non-writable instead, correctly blocking temp file creation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(zts212653#712): centralize drift resolution validation around resolver contract

P1: Route validation enum was accept/reject/skip but resolver contract is
use-global/keep-project — completely mismatched. Now both routes (drift.ts
and mcp-drift.ts) validate against VALID_MCP_DRIFT_DECISIONS exported from
the resolver module, ensuring route-level and resolver-level consistency.

P2: Rename stale F240 anchor to F249 in acp-mcp-resolver test.

Added 3 contract tests verifying VALID_MCP_DRIFT_DECISIONS enum coverage.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(zts212653#712): close validation gap in /api/drift/resolve + route-level tests

P1: /api/drift/resolve silently accepted non-array resolutions (object,
string, number) by only checking Array.isArray — non-array values
skipped validation and fell through to default use-global behavior.

Fix:
- Moved validation before drift check (fail-early on malformed input)
- Added non-array rejection, MAX_RESOLUTIONS cap, enum validation
- Both routes (drift.ts + mcp-drift.ts) now have identical validation

Route-level tests (8 cases via Fastify inject):
- Valid use-global/keep-project accepted
- Non-array resolutions → 400
- String resolutions → 400
- Invalid decision values → 400 with enum hint
- Missing mcpId → 400
- Over-limit array → 400
- Absent resolutions passes through

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant