feat(tests): J10 journey — agents call each other through the MCP server with their own keys (#2349)

[webmixgamer]

Summary

• J10 harness (tests/journeys/test_j10_agent_calls_agent_journey.py): two ephemeral agents, every call made through the MCP server with the caller's own agent-scoped key, read from its container — the harness crosses the real checkAgentAccess gate. Credential-free on every PR: attribution (IA-01, AC-01), refusal with a reason (P-02), stopped callee fails fast and row-less (IA-03), fan-out capped at both layers and landing as one batch (IA-02), loop stops at its budget, delete cascade (L-03). On a keyed stack: the callee's real answer read back from its execution record, and all 12 fan-out subtasks completed.
• Three strict xfails, each with its own issue: no chain-depth guard (bug: agent-to-agent chat chains have no depth guard — a runaway A→B→A bounce is bounded only by capacity parks and hop timeouts #2806), refused calls audited as success: true (bug(mcp): a denied chat_with_agent / fan_out call is audited as a successful tool call #2807), run_agent_loop skips the permission gate (abilityai/trinity-enterprise#628). The backend routes' owner-equivalence for agent keys is filed as a ruling (abilityai/trinity-enterprise#629) with no public reproducer, per the plan-gate decision.
• Rails wiring: journey conftest primitives (agent-key reader, MCP JSON-RPC session, edge/read-back helpers, concurrently provisioned agent pair); catalog J10 → tier: journey-smoke, built: false; IA-03's false A2A 409 claim corrected; registry entry; regenerated JOURNEYS.md; Testing pointers in four feature flows; two learnings entries; diff-scoped CSO report (0 findings introduced, 4 pre-existing tracked).

Fixes #2349

Journey Impact

Journey Impact: extends: J10

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Changes

• tests/journeys/test_j10_agent_calls_agent_journey.py (new, 12 tests)
• tests/journeys/conftest.py — create_agent_and_wait, agent_mcp_key, McpSession, edge / executions / activities helpers, journey_agent_pair, pair, mcp_as_caller
• tests/journeys/catalog.yaml, tests/registry.json, docs/testing/JOURNEYS.md (regenerated), docs/testing/orchestration-invariant-catalog.md (IA-03)
• docs/memory/feature-flows/{agent-to-agent-collaboration,agent-permissions,fan-out,run-agent-loop}.md, docs/memory/learnings.md
• docs/security-reports/cso-diff-2026-09-15-2349-j10-journey.{json,md}

Test Plan

• Journey tier on a live local stack with a real provider key: cd tests && TRINITY_API_URL=http://localhost:8000 pytest journeys/ -v -rsxX --timeout=300 → 11 passed, 4 xfailed (2 m 17 s); J10 alone → 9 passed, 3 xfailed (89 s); --runxfail on the two behavioural xfails shows the audit row success: True and the loop started on the sibling with no edge
• Guards: tests/unit/test_2338_journey_catalog.py, test_2337_invariant_namespace.py, test_2339_testing_docs_consolidated.py, test_ent614_source_agent_attribution.py → 189 passed; tests/lint_root_test_placement.py OK; scripts/ci/generate_journeys_md.py --check in sync
• Secret / host-path scan of the diff and every run log: 0 hits; enterprise-docs guard PCRE replayed over edited docs: 0 hits
• journey-smoke on this PR runs the credential-free subset (the keyed tests skip with the allowlisted reason) — expected: 12 collected in J10, 10 pass / 3 xfail / 2 skip across the tier

Needs the Docker socket of the host running the stack (the agent key lives only in the caller's container env); journey-smoke.yml boots the stack on the runner, so that holds there.

🤖 Generated with Claude Code

[dolho]

/review Report

Branch: feature/2349-j10-agent-calls-agent → dev
Merge-base: eb41896f5 (diffed against the merge-base, not the base tip)
Files changed: 13 (+1151/−24) · 1 commit
Scope: CLEAN
Plan completion: 7 done · 1 partial-by-design · 0 not done · 0 changed · 0 unverifiable

0 critical. 4 informational. This is careful work — most of what I went looking for was already reasoned about in a docstring. Findings below are small, and two of them are about a guarantee the harness states but does not check.

---

Scope check

Intent (#2349): a journey harness proving a two-agent round trip, the permission boundary, loop/budget guards and an honest failure on a dead callee — with findings shipped as strict=True xfails rather than silent product fixes.
Delivered: exactly that. No src/ file is touched at all — tests, catalog.yaml, registry.json and docs only. That is AC #7 honoured structurally, not just in spirit.

Plan completion vs the 8 acceptance criteria

AC		evidence
two-agent round trip returns a real result	DONE	test_i_can_read_what_they_said (@needs_model)
forbidden call denied, denial observable	DONE	test_without_permission_a_call_is_refused… + test_a_refused_call_still_leaves_an_audit_trail, with the audit lying carried as an xfail (#2807)
loop guards stop runaway recursion	PARTIAL by design	the budget half is real (test_a_loop_stops_by_itself_within_its_budget); the recursion half is a strict-xfail skeleton that raises rather than driving a chain (#2806). Sanctioned by AC #7 and the body says so — flagged only so "loop guards are covered" is not read off the checklist
budget guards bound a fan-out	DONE	test_a_fan_out_is_bounded_and_lands_as_one_batch, asserted at both the MCP tool and the backend
dead/stopped callee fails fast	DONE	test_a_stopped_callee_fails_fast_with_an_honest_reason, with ensure_running restoring the shared callee
named for the journey, not the ticket	DONE	test_j10_agent_calls_agent_journey.py
findings as strict=True xfail + own issue	DONE	3/3 — #2806, #2807, ent#628, all verified OPEN
catalog.yaml record citing invariant IDs	DONE	nine IDs cited, all resolve (below)

---

Informational findings

[I1] pair promises a clean slate it never verifies (Confidence: 8/10)
tests/journeys/conftest.py:469 / clear_edges

@pytest.fixture()
def pair(journey_client, journey_agent_pair):
    """The pair with a clean slate: no permission edges either way, both running."""

def clear_edges(client, source: str) -> None:
    for target in permitted_agents(client, source):
        client.delete(f"/api/agents/{source}/permissions/{target}")   # status discarded

Every other helper in this file asserts its call (add_edge, ensure_running, stop_agent_and_wait, permitted_agents); this one does not. That matters more than usual here, because the module-scope decision is explicitly justified by it:

"Order-independence comes from the pair fixture below, which resets edges and liveness before every test, not from test order."

So the expensive design choice rests on a reset that is not checked. A DELETE that 404s or 500s leaks an edge into the next test, and the failure direction — while safe (the permission test goes red) — goes red blaming the product for a harness leak, which is the one failure mode a journey harness must not have.

Suggestion: one line at the end of clear_edges, or in pair: assert not permitted_agents(client, source).

[I2] The model gate asks about the harness host, not about the agent (Confidence: 8/10)
tests/journeys/test_j10_agent_calls_agent_journey.py:60

_HAS_MODEL_KEY = bool(os.getenv("ANTHROPIC_API_KEY")) and os.getenv("ANTHROPIC_API_KEY") != "placeholder"
needs_model = pytest.mark.skipif(not _HAS_MODEL_KEY, reason="journey needs a real provider key")

os.getenv reads the pytest process's environment. A Trinity stack authenticated by subscription (SUB-003 — CLAUDE_CODE_OAUTH_TOKEN inside the container, subscription_credentials in the DB) has no ANTHROPIC_API_KEY on the host, yet its agents answer normally. On such a stack the two tests that assert B's real words skip permanently, and invisibly: the reason is already allowlisted in audit_skips.py:45, so the skip audit will not surface it either.

Suggestion: treat a registered subscription as satisfying the gate (GET /api/subscriptions), or probe the callee once and gate on that.

[I3] The SSE branch parses unguarded (Confidence: 6/10)
tests/journeys/conftest.py, McpSession._post

msgs = [json.loads(line[6:]) for line in resp.text.splitlines() if line.startswith("data: ")]
mine = [m for m in msgs if m.get("id") == body["id"]]

A data: line that is not JSON raises ValueError; a message that parses to a non-object raises AttributeError on .get. Both escape as raw harness errors — in a file that otherwise goes to real trouble to turn every failure into a named, actionable sentence (_open()'s two messages are models of this).

Suggestion: skip unparseable lines and non-dict messages rather than letting them throw.

[I4] FAN_OUT_CAP is a hand-copy across the language boundary (Confidence: 7/10)
test_j10…:72 — FAN_OUT_CAP = 50, against src/mcp-server/src/tools/chat.ts:637 .max(50).

Drift fails the test (safe direction) but reports it as a product regression. The mitigating half is already there and is the right instinct: the test asserts the refusal names the cap (assert str(FAN_OUT_CAP) in ...), so a changed cap produces a legible diff rather than a mystery.

Suggestion: none required — noting it so the next person to raise the cap knows two places move.

---

Clean categories — with the line that proves it

• Credential handling. agent_mcp_key extracts only TRINITY_MCP_API_KEY from the container env, and the key reaches no output surface: McpSession.__repr__ returns url + a yes/no, McpToolResult.__repr__ carries text only, and _open()'s failures interpolate the response body, never headers. Checked the remaining leak path too — both configured invocations (run-full.sh:150, journey-smoke.yml:148) use --tb=short with no --showlocals, so the key in _post's locals cannot surface in a traceback either.
• xfail discipline. Three strict=True xfails; each names a real, currently-OPEN issue — bug: agent-to-agent chat chains have no depth guard — a runaway A→B→A bounce is bounded only by capacity parks and hop timeouts #2806, bug(mcp): a denied chat_with_agent / fan_out call is audited as a successful tool call #2807, abilityai/trinity-enterprise#628. Verified individually, not by pattern.
• Invariant citations resolve. All nine IDs used across catalog.yaml and the test file (P-01, P-02, P-05, AC-01, L-03, L-06, IA-01, IA-02, IA-03) exist in docs/testing/orchestration-invariant-catalog.md, and none is restated — the issue's standing rule.
• Skip audit. The single skip reason introduced is already on audit_skips.py's allowlist; no new unallowlisted reason. (Orthogonal to bug: tests/run-full.sh cannot complete — directory guard globs the wrong cwd, postgres tier lacks REDIS_URL, skip audit allowlist rejects a clean run #2801, which is about the allowlist's broader accuracy.)
• Order independence. Module-scoped pair for cost, function-scoped pair for isolation, ensure_running restoring the callee the stop test kills — the one genuinely shared mutation, and it is restored. Modulo I1.
• Model gating is precise. I went looking for tests that dispatch real turns without @needs_model and expected to find a hole; there isn't one. The ungated tests assert only what is written at dispatch — source_agent_name, triggered_by, the collaboration activity — and the docstring says exactly that: "Holds without a model: all of it is written before B's container is called."
• Idempotency-key collisions avoided. _unique() appends a uuid, with the reason (feat: idempotency keys at all execution trigger boundaries (RELIABILITY-006) #525's deterministic key over caller/target/route/model/message) written down.
• Committed /cso report is clean — scanned for secret prefixes and non-localhost hosts; nothing.
• Not applicable: SQL/data safety, auth boundaries, enum completeness, frontend/XSS, migrations — no src/ change in the diff.

---

Summary

• Critical: 0 — nothing blocks merge
• Informational: 4 — I1 is the one I would fix before merging (it is one line, and it protects the property the fixture design is justified by)
• Scope: clean; no product code touched, which is what AC security: Fix token logging and add HTML reports to gitignore #7 asks for

No commits pushed to this branch — it is not mine, and there were no critical findings to auto-fix.

One note for sequencing rather than for this PR: #2806's xfail is the flip condition named in that issue's own AC #4, so whoever takes #2806 needs this PR merged first, or has to stack on it. Worth knowing before either is scheduled.

[webmixgamer]

Thanks for the pass — all four findings check out. Dispositions:

• I1 (reset never verified) — applied in 0e32cef8f. clear_edges now asserts each revoke's status and that the edge set is empty afterwards, with a message that names it as a harness reset, so a leaked edge can no longer fail a later permission test blaming the product.
• I3 (SSE branch parses unguarded) — applied in 0e32cef8f. Frames that are not JSON, or not objects, are skipped; the caller's existing "no message" path turns that into a named failure instead of a ValueError from inside the transport.
• I2 (model gate reads the pytest host, not the agent) — agreed, and it predates this PR: J03's first-turn journey carries the identical gate. Filed as bug(tests): the journey tier's model gate reads the pytest host's environment, so keyed journeys skip invisibly on subscription-authenticated stacks #2812 under Epic: CI health & gating + journey coverage — green CI, and every promise tested end to end #1850 rather than fixed here, because the replacement signal is not obvious — a fresh agent takes the platform key from the backend's resolver at creation and only gets a subscription through the auto-switch service, so gating on "the instance has a subscription" could turn today's invisible skip into a false failure. The issue asks for one shared tier helper that reflects what the callee will actually have.
• I4 (FAN_OUT_CAP hand-copied) — noted, no change, as you suggested; the refusal-names-the-cap assertion keeps drift legible.

No assertion changed. J10 rerun on a live stack after the fixes: 9 passed, 3 xfailed (96 s). And yes on sequencing — #2806's flip condition needs this merged first; the follow-ups are ordered that way.

[github-actions]

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

[dolho]

Re-review — head 7143c1a1 — APPROVE

Checked the dispositions against the code, not the comment:

• I1 — 0e32cef8f: clear_edges now asserts each revoke answers 200/204 and that permitted_agents() is empty afterwards, both with messages that say harness reset, so a leaked edge can no longer fail a later permission test blaming the product. Exactly the one line I asked for, plus the per-call check. ✅
• I3 — same commit: the SSE loop skips non-JSON and non-dict frames; the caller's "no message" path is the one that fires. ✅
• I2 — filed as #2812 (verified OPEN) rather than fixed here. Agreed with the reasoning: gating on "the instance has a subscription" would trade an invisible skip for a false failure on a fresh agent still on the platform key, so the shared tier helper is the right home. ✅
• I4 — no change, as suggested. ✅

The origin/dev merge (7143c1a1) resolved only learnings.md and tests/registry.json by hand; no conflict markers on the head; src/ still untouched against the merge-base, which is AC #7 held structurally. All 20 checks green including journey-smoke (5m49s) and the six-shard unit matrix with a clean regression diff. Fixes #2349 present, so the promoter picks it up on merge.

Sequencing reminder for whoever merges: #2806's own AC #4 is the strict-xfail in this file flipping, so this lands before #2806 is picked up.

[github-actions]

⚠️ Live-instance suite skipped — merge conflict against dev.

Resolve by merging dev locally and pushing the result; the next nightly re-tests.