fix: prepare eval a2a publish source refs - #10
Merged
Conversation
…s212653#910) Hotfix-lane cherry-pick of cat-cafe#2253 to close clowder-ai#910 without waiting for the next full outbound sync. Why: connectorGatewayOptions.defaultCatId hardcoded 'opus' as CatId, which crashed no-mention IM messages with Unknown cat ID: opus under runtime cat-* catalogs. Changes: - index.ts: pass getDefaultCatId function reference - ConnectorRouter.ts: widen defaultCatId to CatId | (() => CatId) and resolve per parseMentions call - connector-gateway-bootstrap.ts: widen deps type - connector-gateway-default-cat-invariant.test.js: guard against reintroducing the hardcoded fallback Review: A2A approved by 砚砚/GPT-5.5 in PR comment 4691063684. Closes zts212653#910
Why: eval:a2a scheduled and manual invocations need trusted raw snapshot/attribution refs before cat_cafe_publish_verdict can create a verdict PR. The publish tool must not fabricate evidence, so the API prepares sourceRefs from in-process telemetry and injects them into the eval-cat context. Validation: pnpm --dir packages/api run build; node --test packages/api/test/harness-eval/a2a-publish-source-refs.test.js packages/api/test/harness-eval/eval-cat-invocation.test.js packages/api/test/harness-eval/eval-domain-daily.test.js packages/api/test/harness-eval/eval-manual-trigger-handlers.test.js; pnpm biome check --diagnostic-level=error <changed files>; pnpm check; git diff --check. [砚砚/GPT-5.5🐾]
mindfn
force-pushed
the
fix/f192-a2a-source-refs
branch
from
June 13, 2026 03:09
48b6eee to
895d018
Compare
mindfn
added a commit
that referenced
this pull request
Jun 27, 2026
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>
mindfn
added a commit
that referenced
this pull request
Jun 27, 2026
…, 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>
mindfn
added a commit
that referenced
this pull request
Jun 29, 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>
mindfn
added a commit
that referenced
this pull request
Jul 13, 2026
…e, MemoryQuery, sunset fix Comprehensive revision based on Sol's CHANGES REQUESTED (5 P1s) and Fable's follow-up (instance boundary retraction + acceptance criteria): P1-1 recall three-mode coverage: - Added MemoryQuery discriminated union (relevance | recent) - related() stays as separate method (key-based graph traversal) - scope → content filter, namespace → library selector: two independent small tables replace the old 30-cell matrix P1-2 rebuild/maintain boundary: - rebuild = host replays full ingest (scanner → remember() × N) - maintain() = component-internal index health only - Phase 1 updated: IndexBuilder scanner stays OUTSIDE component - This split resolves 16 getDb escapes (not new work, fixing known leak) P1-3 sunset lifecycle contradiction fixed: - §5.1 signal table refined: "observed source absence → dormant" vs "explicit user delete → hard delete + cascade" (Sol precision) - Phase 2 rewritten: no more cascade delete as default; 188 orphans classified by deletion reason, not bulk-cleared - Cascade delete violates Iron Law #5 (Fable: user data default persistent) - Doc-type lifecycle downgraded from "基本完整" to "⚠️ no executor" - Denial #11 added: cascade delete as sunset fix P1-4 old remnants cleaned: - §2 problem statement: "can't replace conversation backend" → "no unified capability boundary, 16 getDb escapes" - Phase 2: ConversationProvider/两个Provider removed P1-5 Wire = transport adapter: - §4.6: Wire changed from "删除" to "降级为 transport adapter" - §8: MemoryComponent = logical contract; HTTP/socket = optional transport - Parent ADR sync obligation explicitly noted Instance boundary (Sol ruling, Fable agreed): - One Clowder runtime = one MemoryComponent instance - Federation (cross-namespace RRF) is component-internal capability - Architecture diagram: Federation Layer removed from external - MemoryKey.namespace = library identifier (project/global/collection-id) - Denial #10 added: per-namespace replacement = coordinator resurrection 13-capability mapping refined (Fable acceptance criteria): - #6 entity resolution: internal, not on contract - #7 materialization: host-side governance, component only ingests output - #11 provenance: two coverage points (MemoryInput + recall filter) - #13 federation: now component-internal (was external) File renamed: echomem-collaboration-design.md → memory-component-abstraction.md 465 → 505 lines. All three reviewers' inputs consolidated. [布偶猫/claude-opus-4-6🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
eval:a2apublish sourceRefs producer that writes raw F167 snapshot and attribution YAML underdocs/harness-feedback/{snapshots,attributions}/.publishSourceRefsinto scheduled and manualeval:a2ainvocation context.cat_cafe_publish_verdict, and to stop/report when refs are missing.Why
eval:a2adaily verdict publishing was blocked becausecat_cafe_publish_verdictrequires explicitsourceRefs.snapshotNameandsourceRefs.attributionName, while scheduled/manual invocation did not prepare or surface those basenames. The publish tool must not fabricate evidence, so the API now prepares refs from trusted in-process telemetry before invoking the eval cat.Review
Opus reviewed the working diff in
thread_mouste3im3xlkkah, found P2-1, then acknowledged the fix and gave final approval: “P2-1 已修，放行.”Validation
pnpm --dir packages/api run buildnode --test packages/api/test/harness-eval/a2a-publish-source-refs.test.js packages/api/test/harness-eval/eval-cat-invocation.test.js packages/api/test/harness-eval/eval-domain-daily.test.js packages/api/test/harness-eval/eval-manual-trigger-handlers.test.jspnpm biome check --diagnostic-level=error <changed files>pnpm checkgit diff --check[砚砚/GPT-5.5🐾]