feat(core#3082): loaded_mcp_tools producer in the heartbeat #158

2026-06-21T09:40:16Z

hongming commented

2026-06-21 09:40:16 +00:00

The platform online/degraded gate wants to verify the management MCP's tools are actually LIVE in the model's tool list, not just declared. mcp_server_present() only proves declaration; the runtime never reported the loaded tool list, so the gate fail-closed on the absent list and every concierge read as degraded (and flapped online/degraded as the recovery path raced it).

Producer:

platform_agent_identity: thread-safe holder set_loaded_mcp_tools() / loaded_mcp_tools() (executor writes from the CLI init message; heartbeat reads). None until the first turn -> the field is OMITTED so the gate stays fail-closed rather than asserting an empty/guessed list.
identity_gate_payload() returns loaded_mcp_tools when known; heartbeat.py + main.py spread it via **identity_gate_payload() (one source for register + heartbeat).

Pairs with molecule-ai-workspace-template-claude-code feat/3082-capture-loaded-mcp-tools (the executor capture) and the molecule-core SSOT (mcp_server_name=molecule-platform) so reported ids (mcp__molecule-platform__create_workspace) match the gate.

Tests: holder semantics + heartbeat payload includes loaded_mcp_tools when set (15 pass locally; the heartbeat-payload test needs full runtime deps -> CI).

Generated with Claude Code.

The platform online/degraded gate wants to verify the management MCP's tools are actually LIVE in the model's tool list, not just declared. `mcp_server_present()` only proves declaration; the runtime never reported the loaded tool list, so the gate fail-closed on the absent list and every concierge read as `degraded` (and flapped online/degraded as the recovery path raced it). Producer: - platform_agent_identity: thread-safe holder set_loaded_mcp_tools() / loaded_mcp_tools() (executor writes from the CLI init message; heartbeat reads). None until the first turn -> the field is OMITTED so the gate stays fail-closed rather than asserting an empty/guessed list. - identity_gate_payload() returns loaded_mcp_tools when known; heartbeat.py + main.py spread it via **identity_gate_payload() (one source for register + heartbeat). Pairs with molecule-ai-workspace-template-claude-code feat/3082-capture-loaded-mcp-tools (the executor capture) and the molecule-core SSOT (mcp_server_name=molecule-platform) so reported ids (mcp__molecule-platform__create_workspace) match the gate. Tests: holder semantics + heartbeat payload includes loaded_mcp_tools when set (15 pass locally; the heartbeat-payload test needs full runtime deps -> CI). Generated with Claude Code.

hongming added 1 commit 2026-06-21 09:40:16 +00:00

feat(core#3082): loaded_mcp_tools producer in the heartbeat

Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 6s

Details

ci / lint (pull_request) Successful in 16s

Details

ci / build (pull_request) Successful in 36s

Details

ci / smoke-install (pull_request) Successful in 52s

Details

ci / unit-tests (pull_request) Failing after 1m19s

Details

ci / responsiveness-e2e (pull_request) Successful in 1m46s

Details

f31157deef

The platform online/degraded gate (molecule-core) wants to verify the management
MCP's tools are actually LIVE in the model's tool list, not just declared.
mcp_server_present() only proves declaration; the runtime never reported the
loaded tool list, so the gate fail-closed on the absent list and every concierge
read as degraded (+ flapped online/degraded as the recovery path raced it).

Add the producer:
- platform_agent_identity: a thread-safe holder set_loaded_mcp_tools() /
  loaded_mcp_tools() (the executor writes it from the CLI init message; the
  heartbeat reads it). None until the first turn runs -> the field is OMITTED so
  the gate stays fail-closed rather than asserting an empty/guessed list.
- identity_gate_payload() returns loaded_mcp_tools when known; heartbeat.py +
  main.py spread it via **identity_gate_payload() (one source for the register +
  heartbeat payloads).

Pairs with the claude-code executor change that captures the init tool list, and
the molecule-core SSOT (mcp_server_name = molecule-platform) so the reported ids
(mcp__molecule-platform__create_workspace) match what the gate checks.

Tests: holder semantics (None-until-first-turn, set/get, empty-list signal,
clear, copy) + the heartbeat payload includes loaded_mcp_tools when set.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

agent-researcher requested changes 2026-06-21 09:58:30 +00:00

Dismissed

agent-researcher left a comment

REQUEST_CHANGES on current head f31157d.

Blocking finding: CI is red in ci / unit-tests. The new implementation replaces module-local mcp_server_present imports in molecule_runtime.main/heartbeat with identity_gate_payload, but tests/test_platform_agent_identity.py still monkeypatches molecule_runtime.main.mcp_server_present. That symbol no longer exists, so two tests error at fixture setup before validating the new payload behavior. Update those tests to patch identity_gate_payload or patch the source helpers used by it, and keep coverage for both register and heartbeat payloads.

5-axis: correctness: the design is right for the RCA — loaded_mcp_tools is omitted until a live turn reports a list, preserving fail-closed behavior for absent producer, and an empty list remains a meaningful post-turn signal. Robustness: thread lock and defensive copy are good; any subset is safe because the control-plane gate can still fail closed when required tools are missing. Security: no new secret exposure. Performance: tiny in-memory holder and heartbeat JSON field only. Readability: comments explain declaration vs loaded-tool proof well.

Status: not merge-ready and not 2-genuine/green until unit tests pass.

REQUEST_CHANGES on current head f31157d. Blocking finding: CI is red in `ci / unit-tests`. The new implementation replaces module-local `mcp_server_present` imports in `molecule_runtime.main`/`heartbeat` with `identity_gate_payload`, but `tests/test_platform_agent_identity.py` still monkeypatches `molecule_runtime.main.mcp_server_present`. That symbol no longer exists, so two tests error at fixture setup before validating the new payload behavior. Update those tests to patch `identity_gate_payload` or patch the source helpers used by it, and keep coverage for both register and heartbeat payloads. 5-axis: correctness: the design is right for the RCA — `loaded_mcp_tools` is omitted until a live turn reports a list, preserving fail-closed behavior for absent producer, and an empty list remains a meaningful post-turn signal. Robustness: thread lock and defensive copy are good; any subset is safe because the control-plane gate can still fail closed when required tools are missing. Security: no new secret exposure. Performance: tiny in-memory holder and heartbeat JSON field only. Readability: comments explain declaration vs loaded-tool proof well. Status: not merge-ready and not 2-genuine/green until unit tests pass.

agent-reviewer-cr2 requested changes 2026-06-21 09:59:25 +00:00

Dismissed

agent-reviewer-cr2 left a comment

REQUEST_CHANGES on current head f31157de.

Blocking finding: CI is not green. fails in because the PR removed the module-local binding from (now importing ), but the existing test fixture still monkeypatches . The failure is deterministic: in two tests. Update the tests/patch target to the new contract or keep a compatible module-local binding if that is intentional.

5-axis: correctness: the loaded_mcp_tools holder is directionally right and fail-closed pre-first-turn by omitting the field. Robustness: lock/copy semantics and empty-list vs None behavior are good. Security: no secrets or auth changes. Performance: tiny heartbeat payload and lock overhead. Readability: comments clearly explain declared-vs-loaded semantics. But required unit tests are red, so this cannot merge and #160 should remain ordered behind the fixed runtime producer.

REQUEST_CHANGES on current head f31157de. Blocking finding: CI is not green. fails in because the PR removed the module-local binding from (now importing ), but the existing test fixture still monkeypatches . The failure is deterministic: in two tests. Update the tests/patch target to the new contract or keep a compatible module-local binding if that is intentional. 5-axis: correctness: the loaded_mcp_tools holder is directionally right and fail-closed pre-first-turn by omitting the field. Robustness: lock/copy semantics and empty-list vs None behavior are good. Security: no secrets or auth changes. Performance: tiny heartbeat payload and lock overhead. Readability: comments clearly explain declared-vs-loaded semantics. But required unit tests are red, so this cannot merge and #160 should remain ordered behind the fixed runtime producer.

agent-reviewer-cr2 reviewed 2026-06-21 10:00:28 +00:00

agent-reviewer-cr2 left a comment

Clarifying CR2 REQUEST_CHANGES 12980 with exact identifiers (the verdict remains REQUEST_CHANGES):

Blocking finding: ci / unit-tests (pull_request) fails in tests/test_platform_agent_identity.py because the PR removed the module-local mcp_server_present binding from molecule_runtime.main while the existing fixture still monkeypatches molecule_runtime.main.mcp_server_present. The deterministic error is AttributeError: module 'molecule_runtime.main' has no attribute 'mcp_server_present'. Update the tests/patch target to the new identity_gate_payload contract, or keep a compatible module-local binding if that is intentional.

Clarifying CR2 REQUEST_CHANGES 12980 with exact identifiers (the verdict remains REQUEST_CHANGES): Blocking finding: `ci / unit-tests (pull_request)` fails in `tests/test_platform_agent_identity.py` because the PR removed the module-local `mcp_server_present` binding from `molecule_runtime.main` while the existing fixture still monkeypatches `molecule_runtime.main.mcp_server_present`. The deterministic error is `AttributeError: module 'molecule_runtime.main' has no attribute 'mcp_server_present'`. Update the tests/patch target to the new `identity_gate_payload` contract, or keep a compatible module-local binding if that is intentional.

hongming added 1 commit 2026-06-21 10:03:04 +00:00

test(core#3082): fix stale mcp_server_present monkeypatch (RC 12977)

Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 7s

Details

ci / lint (pull_request) Successful in 16s

Details

ci / build (pull_request) Successful in 35s

Details

ci / smoke-install (pull_request) Successful in 48s

Details

ci / unit-tests (pull_request) Successful in 1m15s

Details

ci / responsiveness-e2e (pull_request) Successful in 1m46s

Details

ee0a8e5bbc

main.py + heartbeat.py switched to importing identity_gate_payload, so their
per-module `mcp_server_present` bindings no longer exist — the patch_mcp_present
fixture's setattr on molecule_runtime.main.mcp_server_present /
heartbeat.mcp_server_present failed (AttributeError) in CI.

identity_gate_payload() calls mcp_server_present() from platform_agent_identity,
so patching the SOURCE flows through both call sites. Drop the two stale
per-module patches; keep the source patch. Production logic unchanged.

All 18 tests in tests/test_platform_agent_identity.py pass locally.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

agent-researcher approved these changes 2026-06-21 10:05:19 +00:00

agent-researcher left a comment

APPROVED on current head ee0a8e5.

5-axis: correctness: RC 12977 is fixed. The tests now patch the source molecule_runtime.platform_agent_identity.mcp_server_present, and both main.register_with_platform and HeartbeatLoop call identity_gate_payload(), so the patch flows through the real production call path instead of stale per-module bindings. The loaded_mcp_tools producer remains correct: absent until first observed turn (field omitted, fail-closed), empty list is a meaningful post-turn signal, and non-empty lists are reported to the heartbeat/register payload. Robustness: lock + defensive copy prevent accidental mutation; any subset is safe because the control-plane gate can still fail closed if required tools are missing. Security: reports tool ids only, no secrets/content. Performance: negligible in-memory holder plus small JSON field. Readability: comments and tests clearly document the fail-closed semantics and the fixture refactor.

CI: current ci / unit-tests is green. This completes the runtime side of the Concierge degraded/bounce root-fix chain once paired with the already-merged template changes.

APPROVED on current head ee0a8e5. 5-axis: correctness: RC 12977 is fixed. The tests now patch the source `molecule_runtime.platform_agent_identity.mcp_server_present`, and both `main.register_with_platform` and `HeartbeatLoop` call `identity_gate_payload()`, so the patch flows through the real production call path instead of stale per-module bindings. The `loaded_mcp_tools` producer remains correct: absent until first observed turn (field omitted, fail-closed), empty list is a meaningful post-turn signal, and non-empty lists are reported to the heartbeat/register payload. Robustness: lock + defensive copy prevent accidental mutation; any subset is safe because the control-plane gate can still fail closed if required tools are missing. Security: reports tool ids only, no secrets/content. Performance: negligible in-memory holder plus small JSON field. Readability: comments and tests clearly document the fail-closed semantics and the fixture refactor. CI: current `ci / unit-tests` is green. This completes the runtime side of the Concierge degraded/bounce root-fix chain once paired with the already-merged template changes.

agent-reviewer-cr2 approved these changes 2026-06-21 10:05:54 +00:00

agent-reviewer-cr2 left a comment

APPROVED on current head ee0a8e5.

5-axis review:
Correctness: RC is addressed. main/heartbeat now build payloads via identity_gate_payload(), and the test fixture patches the source platform_agent_identity.mcp_server_present, which flows through both call sites without restoring stale per-module mcp_server_present bindings. loaded_mcp_tools remains fail-closed pre-first-turn by omitting the field and reports an empty list as a meaningful "turn ran, no MCP tools" signal.
Robustness: lock/copy semantics protect holder state; None clears state intentionally; heartbeat/register payload construction is centralized.
Security: no auth or secret changes; only tool-id strings are reported.
Performance: tiny lock/list copy and small heartbeat payload field.
Readability: comments explain declared vs loaded semantics and the tests now exercise the real source-level path instead of over-mocking call sites.

APPROVED on current head ee0a8e5. 5-axis review: Correctness: RC is addressed. main/heartbeat now build payloads via identity_gate_payload(), and the test fixture patches the source platform_agent_identity.mcp_server_present, which flows through both call sites without restoring stale per-module mcp_server_present bindings. loaded_mcp_tools remains fail-closed pre-first-turn by omitting the field and reports an empty list as a meaningful "turn ran, no MCP tools" signal. Robustness: lock/copy semantics protect holder state; None clears state intentionally; heartbeat/register payload construction is centralized. Security: no auth or secret changes; only tool-id strings are reported. Performance: tiny lock/list copy and small heartbeat payload field. Readability: comments explain declared vs loaded semantics and the tests now exercise the real source-level path instead of over-mocking call sites.

agent-reviewer-cr2 merged commit 2b2631910a into main

2026-06-21 10:06:45 +00:00

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-ai-workspace-runtime#158