Skip to content

test(evals): tool-surface A/B — shipped sendMessage vs messaging-primitives post - #791

Open
Poytr1 wants to merge 17 commits into
mainfrom
claude/primitives-ab-test
Open

test(evals): tool-surface A/B — shipped sendMessage vs messaging-primitives post#791
Poytr1 wants to merge 17 commits into
mainfrom
claude/primitives-ab-test

Conversation

@Poytr1

@Poytr1 Poytr1 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

What this is

The behavioral A/B comparing two agent-facing messaging tool surfaces, measuring task success rate and total token consumption (plus invalid-call rate, tool calls, wall time):

  • Arm A — the production sendMessage surface, exactly as shipped.
  • Arm B — the post façade from the messaging-primitives proposal (docs(design): messaging primitives — counter-proposal to the sendMessage rework #551): one tool, three orthogonal params (conversation / addresses / visibility). Every call compiles into exactly one legal sendMessage input and executes through the product tool, so both arms share one implementation and differ ONLY in the presented surface.

Full method, fidelity notes, pre-registration, and measured static costs: docs/designs/messaging-primitives-ab.md.

The three evaluation-only seams (all contract-tested, zero production behavior change)

  1. executeProductTool — an evaluation-registry tool runs a product tool on the caller's own trusted SessionContext.
  2. hideProductTools — withhold named product descriptors so each arm presents exactly one surface; a hidden tool stays executable (that is how the façade compiles down to it).
  3. collaborationGuidance — the prompt-side complement: the standing collaboration guidance and parent-report append teach sendMessage shapes by name, so arm B swaps in texts that teach ITS surface (mirroring the production wording sentence-for-sentence outside the tool teaching). Production never sets it; unset ⇒ byte-identical prompts.

Fixture contract tests prove arm parity against a real daemon: descriptor lists, injected prompt text, real world-authorized delivery through the façade, and the post-flavored report-back append on a needsReply child.

Measured static cost (real descriptors + real injected prompts)

Component sendMessage post ratio
descriptor (chars) 10,024 (~2,506 tok) 2,071 (~518 tok) 4.8×
standing guidance (chars) 2,718 2,394 1.1×
combined per session 12,742 (~3,186 tok) 4,465 (~1,116 tok) 2.9×

Behavioral matrix status

4 scenarios × 2 arms × 3 trials, local Claude Code over ACP, judged from the daemon's own records (executed product form + world effects, never model claims), counterbalanced arm order, per-component token accounting. Implemented in evals/test/tool-surface-ab-real.test.ts (env-gated, never in CI). The smoke run verified the harness to the provider boundary; the full matrix is currently blocked on re-authenticating the local Claude Code CLI (its OAuth refresh token expired 2026-08-09). The doc's §6 carries the exact one-command protocol and the table skeleton it fills.

Pre-registered: clear arm-B win on static cost (confirmed) and on invalid-call rate (partly by construction), little/no expected difference on success/efficiency at n=3.

Tests

  • pnpm eval:collab:contracts — 115 passed
  • pnpm eval:contracts — 46 passed
  • new: 20 façade/classifier contracts + 3 fixture arm-parity contracts
  • daemon suite: 2880 passed; the 2 failures (daemon-agent-mention-routing 5s-timeout, live acp-matrix) reproduce identically on clean main on this machine — pre-existing environmental, not from this branch
  • typecheck / lint / format clean

🤖 Generated with Claude Code

Poytr1 and others added 6 commits August 9, 2026 10:58
…h tool surfaces

Groundwork for the A/B comparing the landed `sendMessage` surface against the
messaging-primitives `post` primitive. This commit lands the façade, its
contract tests, and the credential-free static measurement; the behavioral
scenarios follow.

The façade is a façade and nothing else. Every `post` compiles into exactly ONE
legal `sendMessage` input and is then executed BY the product tool, so the two
arms share one implementation and differ only in the schema and description the
model carries. No routing, activation, addressing or policy code is touched.

Two small evaluation-only seams make that possible:

- `McpControlServer.executeProductTool` lets an evaluation-registry tool run a
  product tool on the same trusted token-bound SessionContext. It grants no
  capability the caller did not already have, and re-entry is safe because the
  registry is consulted by exact name and a product tool never matches it.
- `hideProductTools` withholds named product descriptors for a run, so an A/B
  arm presents exactly one surface for a capability. A withheld tool stays fully
  executable — which is precisely how the façade compiles down to it.

All six legal forms compile: agent+channel, bare channel, postless agent call,
DM, channel-root user mentions, and the parent-session reply. The façade refuses
rather than guesses when a post is under-specified, and refuses the combinations
the product has no form for instead of inventing one.

Deliberately NOT expressible by either arm, and excluded from the experiment: a
fully-addressed cross-room handoff into an existing THREAD. The routing rework
removed `thread` from every target (baseline §6.4), so including it would
measure a known product gap rather than the two surfaces.

STATIC COST, measured from the real descriptors rather than from prose:

  sendMessage   2602 description + 7353 schema = 9955 chars (~2489 tokens)
  post          1046 description + 1025 schema = 2071 chars (~518 tokens)

a 4.8x smaller surface the model must carry on every turn. Token figures are a
tokenizer-free approximation and are labelled as such.

8 contract tests, credential-free.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ic extraction

Four scenarios of the reduced matrix, a send-form classifier shared by both
arms, and trial metric extraction from the recorded ACP event stream.

The classifier is what makes the arms comparable: arm A's raw sendMessage args
and arm B's compiled args score through one vocabulary, so 'addressed it
correctly, first try' means the same thing on both sides. A wrong-but-accepted
form counts as a failure to complete, not a success — the product accepting a
call says nothing about whether it did the task.

10 contract tests. One of them already earned its keep: it caught the first
scenario's task text using the word 'conversation', which is an arm-B field
name and would have biased the comparison toward the façade.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… its own surface

The standing collaboration guidance and the parent-report append teach
`sendMessage` call shapes by name, so an arm that withholds `sendMessage`
would carry a system prompt describing a tool it does not have — priming it
with the other arm's vocabulary. This is the prompt-side complement of
`hideProductTools`: the evaluation environment may supply replacement
texts, production never sets them, and everything else in the prompt stays
byte-identical.

Arm B's texts mirror the production structure sentence-for-sentence outside
the tool teaching (pinned by tests), and the parent append names the exact
`post` parent form with the real session id.

Also: give the metric extractor per-component token sums (cache traffic
dominates local runs, so input+output is reported beside the total), an
arm-specific classifier hook (arm B classifies the COMPILED product args),
a turn count, and give scenario 4 a concrete relayed question so the
parent-session wake carries a real task.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oven arm parity

AbFixture boots the real daemon against the three-room A/B topology
(briefing / peer-briefing / plaza) with production mention-gated routing and
full platform-echo fidelity. The `arm` option changes ONLY the messaging
surface: arm A is the shipped `sendMessage`; arm B carries the `post`
façade with `sendMessage` withheld and the arm-B guidance texts — while
every façade call still executes through the product tool on the same
trusted session context.

Credential-free contract tests read the daemon's own control socket and the
world's delivered effects to pin the fairness preconditions:

- arm A sessions list `sendMessage` (no `post`) and carry the production
  guidance;
- arm B sessions list `post` (no `sendMessage`), their prompt teaches the
  post surface with no `sendMessage` text anywhere, and a compiled call
  produces a real world-authorized delivery;
- a needsReply wake under arm B installs the `post`-flavored parent-report
  append on the child, and the postless exchange projects nothing into any
  channel.

Plus listDaemonTools on the scripted-host IPC client so a test can prove
what a session's descriptor list actually contained.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…als, daemon-judged

evals/test/tool-surface-ab-real.test.ts runs the pre-registered matrix
against a real ACP subject template (env-gated, never in a CI gate). Every
score comes from the daemon's own records: an attempt satisfies a scenario
only when its EXECUTED product form (arm B scored on what its call compiled
to) names the right target ids, AND the world's effects show the intended
delivery — a delivered post in the target channel, the addressed agent's
real activation, the postless ask leaking nowhere, the parent actually
woken by the child's answer.

Scenario 4's parent session is real: the peer is told to delegate the
quoted request with an answer-back obligation, and a trial where the peer
never delegates is recorded invalid, not scored against either surface.
Provider failures and timeouts likewise invalidate the trial rather than
an arm. Token consumption is read from turn.completed usage, scoped to the
subject and reported for the whole run, with per-component sums. Arm order
is counterbalanced per (scenario, trial); artifacts (events.jsonl,
world-events.jsonl, trial.json, summary.json) are written per run,
redacted with the subject template's secret set.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…red static costs

docs/designs/messaging-primitives-ab.md records the full pre-registered
protocol (matrix, scenarios, judging, validity), the fairness seams and
their contract coverage, the honesty constraints, and the measured static
costs: sendMessage 10,024 descriptor chars (~2,506 tokens) vs post 2,071
(~518) — 4.8x — and 12,742 vs 4,465 chars (2.9x) including the standing
guidance each arm's sessions actually carry (measured from real injected
prompts, logged by the fixture contract test).

The behavioral table's run protocol is in the doc; the 24-run matrix is
implemented and blocked only on re-authenticating the local Claude Code
CLI (its OAuth refresh token expired 2026-08-09 — the 2026-08-08 baseline
re-run caught its last hours). Linkage to the primitives proposal doc is
noted but deliberately not added while #551 is an open PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Poytr1 Poytr1 changed the title eval(a/b): tool-surface experiment — shipped sendMessage vs messaging-primitives post test(evals): tool-surface A/B — shipped sendMessage vs messaging-primitives post Aug 9, 2026
Poytr1 and others added 3 commits August 9, 2026 12:24
…sion auto-allow

Measured in the tool-surface A/B's first live trial: arm B's correct
`post` call hung on an ACP permission request until the trial budget
burned — the daemon auto-allows its OWN MCP tools (ALL_TOOL_NAMES) but an
evaluation-registry tool served by the same trusted server fell through to
the interactive policy, and the arena has no human to tap the card. That
is a guaranteed stall AND a fairness bug: arm A's product tool was
auto-allowed while arm B's surface required an unanswerable approval.

The FQNs (mcp__agentconnect__<name> / mcp.agentconnect.<name>) are minted
where the registry is installed, and resolveAcpPermission grants them with
reason 'evaluation_game_tool' through the same fail-safe matcher rungs as
isBuiltinSystemTool (exported as matchesToolPermissionFqns, unit-tested).
Empty outside evaluation runs — production behavior unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Measured: a subscription session limit surfaces as a generic turn_failed
RequestError plus an apologetic delivered reply — the old provider-code
allowlist scored those runs as ordinary behavioral trials with zero
attempts. This experiment is a single explicit send, so unlike a long
arena game it can never legitimately absorb a failed turn: invalidate on
any turn.failed / turn.timed_out and carry the code in the reason.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s, 28-46% fewer tokens under post

The full pre-registered matrix ran 2026-08-09 (local Claude Code over ACP,
sonnet, 24/24 valid trials). Success 11/12 vs 11/12 and first-attempt 9/12
vs 9/12 — identical, as pre-registered. Token consumption: 28-46% fewer
total subject tokens and 47-75% fewer uncached in+out under the primitives
surface wherever the session carries the full surface, wall time 21-64%
lower; child sessions equal. Pre-registration #2 REFUTED in direction: arm
B produced the only 2 invalid calls (both actionable facade refusals the
model repaired); arm A emitted zero illegal combinations in 24 runs.

Shared scenario-4 finding, both arms identically: the child session answers
its parent through Claude Code's own built-in SendMessage tool (a name
collision with the product tool), so the answer never reaches the daemon —
0/6 first-attempt on parent-session across arms, 2/3 eventual success each.
A product problem upstream of either surface.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The production guidance now leads with the tool-precedence rule (the
runtime's built-in SendMessage silently swallows parent reports — issue
800). Arm B carries the equivalent bullet worded for its surface, so the
A/B keeps comparing surfaces rather than one arm's extra warning.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found three blocking evaluation-correctness issues on the exact trusted head:

  1. The new deterministic façade/classifier/fixture contracts are not run by either CI contract command. The workflow invokes pnpm eval:contracts and pnpm eval:collab:contracts, but both scripts use explicit path lists that omit post-facade.test.ts, tool-surface-ab.test.ts, and tool-surface-ab-fixture.test.ts. The new production-side evaluation seams therefore have no effective CI regression gate.
  2. The head-only tool-precedence bullet is present only in arm B at this base revision. Arm A's production guidance has no corresponding warning, so the current apparatus no longer has prompt parity. It also makes the checked-in static figures stale (arm-B standing guidance is now 2,770 chars, not 2,394; combined is 4,841, not 4,465), while the documented 24-run results were recorded before this head change. Either remove the B-only addition for this base, or bring in the matching A guidance and rerun/update the measurements.
  3. The channel-bare judge accepts any delivered plaza post even when the peer was activated, although the scenario explicitly requires waking nobody. That is a success false positive and should be part of effectsOk.

Non-blocking warning: matchesToolPermissionFqns uses raw substring matching for registered FQNs, so a longer identity containing a registered name also receives the evaluation auto-allow. This is evaluation-only, but exact or delimiter-aware suffix matching would better match the stated fail-safe contract.

Verification: exact HEAD 7168426344289847a514696419fccadef5632aa0 was confirmed; git diff --check passed; a dependency-free compilePost smoke check passed; and a script inspection confirmed all three contract files are absent from the CI command lists. I could not execute Vitest because dependencies are not installed and downloading the pinned pnpm package failed with DNS resolution errors, including after the permitted network retry.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

attributable to the _surface_, not to a second implementation.

Three evaluation-only seams make the comparison fair, and each is
contract-tested in the CI gates:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These contracts are not currently CI-gated. .github/workflows/test.yaml runs pnpm eval:contracts and pnpm eval:collab:contracts, but the explicit file lists in both package scripts omit evals/test/post-facade.test.ts, evals/test/tool-surface-ab.test.ts, and evals/test/tool-surface-ab-fixture.test.ts. Please wire the new deterministic tests into a CI command; otherwise regressions in all three evaluation seams can merge without exercising these proofs.

Comment thread evals/games/post-facade.ts Outdated
// Arm parity with the production precedence bullet (issue #800): the runtime's
// built-in messaging tools are a hazard for BOTH surfaces, so both arms carry
// the equivalent warning — worded for the surface each arm actually has.
`- AgentConnect's tools (the \`agentconnect\` MCP server, e.g. \`mcp__agentconnect__post\`) are the ` +

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the trusted base/head, arm A's production guidance in session-manager.ts does not contain the corresponding tool-precedence warning; this text exists only in arm B. That makes the current prompts behaviorally asymmetric and changes arm-B guidance from the documented 2,394 chars to 2,770 (combined 4,841 rather than 4,465). The published 24-run results were also committed before this addition. Please remove this B-only warning for the current base, or bring in the A-side equivalent and rerun/update the static and behavioral measurements.

Comment thread evals/test/tool-surface-ab-real.test.ts Outdated
if (!peerActivated) notes.push('the addressed agent was never activated')
break
case 'channel-bare':
effectsOk = deliveredInPlaza

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can score success even when the peer was woken, although the channel-bare task explicitly requires that nobody be woken or notified. A model can make a bare product call whose message body still mentions the peer, yielding both deliveredInPlaza and peerActivated; this branch accepts it. Require deliveredInPlaza && !peerActivated (and record the activation note) so the daemon-effects judge enforces the scenario's negative condition.

Poytr1 added a commit that referenced this pull request Aug 10, 2026
…cedence rule (#800) (#801)

Measured in the tool-surface A/B (2026-08-09, PR #791): a child session on
the Claude Code runtime also carries the runtime's own built-in
`SendMessage` (agent-teams messaging) — a literal name match for the
report-back instruction — and in 3 of 6 parent-report trials the model's
first attempt went there, silently losing the parent's answer in 2 of 6.

The standing context now says, before anything else about collaboration:
AgentConnect's MCP tools are the ONLY channel that reaches other agents
and humans, similarly-named runtime built-ins do not reach AgentConnect
and anything sent through them is lost. ~80 standing tokens per session.
This is the prompt-side half of #800; suppressing the built-in tool and
the reply-hint / inferred-reply hardening remain follow-ups.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Poytr1 and others added 2 commits August 10, 2026 11:18
…ompt parity, tighten the channel-bare judge

Review findings on the exact head, all fixed:

1. The façade/classifier/fixture contract files were not in either CI
   contract command's explicit path list — added all three to
   eval:collab:contracts (now 18 files / 138 tests).
2. Prompt parity: merged main, which now carries the #800 tool-precedence
   bullet (#801), so BOTH arms' guidance includes it again. The doc's
   static table now records both revisions — the 24-run revision (parity
   held: neither arm had the bullet) and the current head (parity held:
   both do, A 3,099 / B 2,771 chars) — plus the separately measured
   bullet effect (built-in SendMessage attempts 3/6 -> 0/10, losses 0/10,
   success 10/10; issue #800).
3. channel-bare's judge now also requires that nobody was woken; the six
   recorded trials were re-verified under the stricter rule (peer ran
   zero turns in all six) and their 6/6 stands.

The substring FQN matching in matchesToolPermissionFqns is deliberate
consistency with the production isBuiltinSystemTool matcher (same
id-suffixed adapter variants), noted in the review reply.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Poytr1

Poytr1 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Review findings addressed in 6c28d3a:

  1. CI gatepost-facade.test.ts, tool-surface-ab.test.ts, and tool-surface-ab-fixture.test.ts are now in eval:collab:contracts (18 files / 138 tests, verified locally).
  2. Prompt parity + stale figures — merged main, which now carries the Claude Code's built-in SendMessage tool collides with the AgentConnect messaging tool and silently swallows parent reports #800 tool-precedence bullet (fix(daemon): lead the collaboration guidance with a tool-precedence rule (#800) #801), so both arms include it again. §4's table now records both revisions explicitly: the 24-run revision (parity held — neither arm had the bullet) and the current head (parity held — both do: A 3,099 / B 2,771 chars, combined 13,123 vs 4,842 = 2.7×), plus §6 revision notes with the separately measured bullet effect (built-in SendMessage attempts 3/6 → 0/10, losses 0/10, success 10/10).
  3. channel-bare judge — now also requires that nobody was woken (deliveredInPlaza && !peerActivated). The six recorded channel-bare trials were re-verified under the stricter rule against their events.jsonl: the peer ran zero turns in all six, so the recorded 6/6 stands.

On the non-blocking warning: the substring FQN matching in matchesToolPermissionFqns is deliberate consistency with the production isBuiltinSystemTool matcher, which uses the same includes rung for adapter id-suffixed identities (mcp__agentconnect__<name>-42); diverging the two matchers' semantics seemed worse than sharing the known trade-off. Happy to tighten both together in a follow-up if preferred.

🤖 Generated with Claude Code

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new revision addresses all three prior blockers and passes review.

  • The façade, classifier, and real-daemon fixture contracts are now included in eval:collab:contracts, which the GitHub Actions workflow runs.
  • Merging the current base brings the production tool-precedence warning into arm A, restoring prompt parity with arm B. The design now distinguishes the original 24-run revision from the current guidance lengths, and the checked static figures match the current constants.
  • The channel-bare effects judge now requires !peerActivated and records an explanatory note when the negative condition fails.

I also re-inspected the complete current base-to-head diff; no new blocking correctness, lifecycle, or security issue emerged. The evaluation-only substring FQN behavior remains consistent with the existing production built-in matcher and is not a blocker.

Verification: the trusted checkout is the synthetic merge 206bbe015b9ff23344b39bb8ea12dc9e871fa11a whose parents are exactly base 29feb9b6e2021fbd88e4431ba46c764d0a81e5a6 and head 6c28d3a00f99e0b26c664317f088808a830b3c7c; git diff --check passed; dependency-free façade compilation/static-cost assertions passed; and static checks confirmed the CI paths and stricter judge. I could not execute Vitest locally because the pinned pnpm download still failed DNS resolution after the permitted network retry.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

@agentconnect-md-test

Copy link
Copy Markdown
Contributor

Confirmed—these changes addressed the review findings, and I approved revision 6c28d3a0. Keeping the FQN matching consistent is reasonable; delimiter-aware tightening can be a separate follow-up.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

Poytr1 and others added 3 commits August 11, 2026 23:46
…test

# Conflicts:
#	packages/daemon/src/session/session-manager.ts
…ion gate

The A/B matrix's four scenarios each demand one explicit send, so the #801
tool-precedence bullet could score 10/10 on parent-session while breaking
ordinary thread play (the live counting-game regression that #861 reverted;
issue #800 records the lesson). Scenario 5 is the missing coverage: one
plaza thread, BOTH agents on the arm's surface, a human kickoff
@-mentioning both, and the count carried by ordinary replies through the
platform echo and the #549 continuation ladder.

- `THREAD_COUNT_SCENARIO` + `judgeThreadCount` (daemon-judged): hard pass =
  count reaches 6 via delivered thread replies, ZERO messaging-tool calls
  by either participant (product surface, other arm's tool, or the runtime
  built-in — delivered or refused), no rejected participant reply. Soft
  (reported, never failed on): duplicates, skips, overshoot,
  meta-narration beyond the bare number, reply length, turns per number.
- Wired into the real-run matrix at index 4: same counterbalancing, seed
  derivation, validity rules (failed/timed-out turn => invalid), artifact
  layout, and summary; rides the existing scenario/arm env filters.
- Contract tests (eval:collab:contracts): the #801 handoff trace scores
  FAIL, a clean reply-only trace scores PASS, built-in SendMessage and
  arm-B `post` variants fail identically, soft-metric traces pass while
  measured; plus a scripted end-to-end fixture test proving a
  both-mentioned kickoff plus ordinary replies really carries the count
  peer-to-peer on a real daemon.
- Arm-B guidance parity: drop the #800 precedence bullet from
  POST_COLLAB_GUIDANCE, mirroring the production revert (#861) — both arms
  are back at the 24-run revision's text (2,718 / 2,394 chars, measured).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nge gate

messaging-primitives-ab.md: §8 documents the in-thread turn-taking
scenario (the #801 incident, shape, daemon judge, and the 2026-08-11
post-revert baseline — 6/6 clean passes across both arms, zero
messaging-tool calls, zero meta-narration) plus the standing rule; §2/§4
updated for the scenario list and the parity revert (guidance back to the
24-run revision, re-measured at 2,718/2,394 chars).

collaboration-arena-baseline.md: §4.2 states the prompt-change gate — any
change to the standing collaboration guidance or parent-report append must
be validated against BOTH the parent-session scenario AND the in-thread
scenario before landing, citing #801/#861 as the incident that created the
rule.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Poytr1

Poytr1 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Pushed the #800/#861 follow-up onto this PR (it owns the A/B apparatus):

  • Merged main (brings the revert(daemon): drop the #801 tool-precedence bullet after a live in-thread regression #861 revert; the standing guidance in the seam's default is byte-identical to production again) and dropped the arm-B parity bullet from POST_COLLAB_GUIDANCE to match — both arms are back at the 24-run revision's guidance (re-measured: 2,718 / 2,394 chars).
  • Scenario 5 in-thread-count (36332e0): the in-thread turn-taking gate the matrix was missing — two subjects in one plaza thread, human kickoff mentioning both, count to 6 via ordinary replies; judgeThreadCount hard-fails on ANY participant messaging-tool call (the fix(daemon): lead the collaboration guidance with a tool-precedence rule (#800) #801 failure mode), on an incomplete count, or on a lost reply; duplicates/skips/meta-narration/turns/tokens are soft metrics. Contract tests are in eval:collab:contracts (judge traces + a scripted end-to-end fixture run); the real-run matrix gains the scenario at index 4 with the same counterbalancing, validity, and artifact rules.
  • Baseline (db42907, doc §8.4): 2 arms × 3 trials on real local Claude Code (sonnet), post-revert prompt — 6/6 clean passes, all bare numbers 1–6 in order, zero messaging-tool calls. §8.5 + collaboration-arena-baseline.md §4.2 record the standing prompt-change gate: guidance changes must be validated against BOTH parent-session AND in-thread-count before landing.

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previously approved four-scenario A/B remains intact, but the new scenario-5 hard judge has two blocking false-pass paths:

  1. It recognizes post and mcp__agentconnect__post, but not the daemon-supported dotted flattened identity mcp.agentconnect.post. A participant can therefore call the arm-B messaging surface under that valid adapter spelling and still receive a zero-tool-call PASS.
  2. It treats every participant reply with no thread field as if it belonged to the kickoff thread. In the virtual world, thread: undefined is a channel-root post that opens a different conversation, so six root replies currently satisfy the “delivered ordinary thread replies” hard criterion.

Both were reproduced directly against judgeThreadCount: a clean 1–6 target-thread trace plus a dotted mcp.agentconnect.post event passes with no recorded messaging calls, and a clean 1–6 trace consisting entirely of threadless channel-root effects also passes. Please recognize both flattened FQN spellings and require the delivered count effects to match the exact kickoff thread, with regression tests for both cases.

Verification: the trusted checkout is synthetic merge 47fac0d18e6056b98276f907aab9dd56b24039b7, whose parents are exactly base c1a5253d6eba95b91b9a60d1153fca383d667014 and head db42907a5b72dde3502a6879da0d81e1fc018dec; git diff --check passed; the prior CI wiring and prompt-parity fixes remain present; and dependency-free TypeScript imports reproduced the two verdict errors above. The full collaboration suite could not start because downloading the pinned pnpm package failed DNS resolution in both sandboxed and permitted network attempts.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

Comment thread evals/games/tool-surface-ab.ts Outdated
export function isMessagingToolName(name: string): boolean {
const normalized = name.toLowerCase()
if (normalized.includes('sendmessage')) return true
return normalized === 'post' || normalized.endsWith('__post')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This misses the dotted flattened MCP identity mcp.agentconnect.post, even though the daemon explicitly supports dotted FQNs alongside mcp__agentconnect__post (for example in BUILTIN_TOOL_FQNS and evaluation permission identities). Replaying a completed tool event with title mcp.agentconnect.post leaves messagingToolCalls empty and the judge passes. Please recognize the dotted suffix too and add it to the matcher contract.

Comment thread evals/games/tool-surface-ab.ts Outdated
effect.agentId !== undefined &&
participants.has(effect.agentId) &&
effect.channel === options.channel &&
(effect.thread === undefined || effect.thread === options.thread)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thread === undefined is a channel-root post in ArenaWorld—it creates a new thread—rather than a reply in options.thread. Because this predicate accepts it, six participant root effects containing 1–6 currently produce a hard PASS despite no number landing in the kickoff conversation. Delivered count effects should require effect.thread === options.thread; please pin a wrong-root/wrong-thread trace as a failure (and keep rejected-reply accounting consistent with the stated “no participant reply was lost” rule).

@agentconnect-md-test

Copy link
Copy Markdown
Contributor

Thanks—the scenario and baseline are useful. The latest review remains blocked on two db42907a judge false positives:

  • mcp.agentconnect.post is not recognized as messaging.
  • Threadless channel-root effects count as kickoff-thread replies.

Please fix and contract-test both paths.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

- Match messaging tools by name segment, so the daemon's dotted ACP
  identity (`mcp.agentconnect.post`) fails the hard rule exactly like
  `post` / `mcp__agentconnect__post`, while `compost`/`postpone` still
  never match.
- Require a STRICT thread match on counted replies: a participant reply
  effect with no `thread` (or another thread) is a channel-root post in a
  different conversation — numbers landed there (where a messaging-tool
  detour would put them) no longer satisfy the count. The 6/6 baseline is
  unaffected: every recorded reply carries the kickoff thread id.

Both pinned by new contract tests in the eval:collab:contracts gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two requested fixes are correct and their new contracts cover the exact reported cases. One blocking adapter spelling still escapes the revised hard-rule matcher, however.

The daemon explicitly supports ACP adapters that suffix an opaque invocation id to an underscore-flattened FQN (see containsBuiltinToolFqn and the existing mcp__agentconnect__sendMessage-42 permission test). With the new last-segment implementation, mcp__agentconnect__post-42 splits to a final segment of 42, so it is not recognized. Replaying a completed participant call under that identity alongside a clean target-thread count leaves messagingToolCalls empty and returns PASS. Please match a post name segment before an optional invocation suffix and add the suffixed underscore/dotted forms to the matcher and judge contracts.

Verification: exact HEAD cb89aaeeeaa2f1f91c555e8eb2b9f4a0d3fc4324 was confirmed; git diff --check passed; dependency-free judge checks confirmed that dotted mcp.agentconnect.post and off-thread replies now fail, while mcp__agentconnect__post-42 still passes incorrectly. The full Vitest suite remains unavailable in this workspace because dependencies are absent and the pinned pnpm package cannot be downloaded through the environment's DNS.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

Comment thread evals/games/tool-surface-ab.ts Outdated
.split(/[^a-z0-9]+/)
.filter(Boolean)
.at(-1)
return lastSegment === 'post'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still misses the adapter-suffixed identity that the daemon explicitly supports: mcp__agentconnect__post-42 splits to ['mcp','agentconnect','post','42'], so lastSegment is 42. The existing daemon contract already pins the analogous mcp__agentconnect__sendMessage-42 form because some ACP adapters append opaque invocation ids. A completed post-42 event currently leaves messagingToolCalls empty and lets the hard judge pass. Please recognize post immediately before an optional suffix and contract-test both underscore and dotted suffixed spellings.

…ead judge (review)

Some ACP adapters suffix an opaque invocation id to the flattened MCP name
(daemon.ts containsBuiltinToolFqn, e.g. `mcp__agentconnect__post-42`);
the last-segment matcher missed those, leaving a false-pass spelling.
Match `post` as a BOUNDED separator-delimited segment instead — every
underscore/dotted/suffixed spelling fails the hard rule, `compost` and
`postpone` still never match — and pin the suffixed spellings in both the
matcher and judge contracts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@agentconnect-md-test agentconnect-md-test Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The adapter-suffixed FQN false-pass is fixed, and this revision passes review.

The bounded-segment matcher now recognizes the product and façade across bare, underscore-flattened, dotted, and opaque invocation-ID-suffixed identities, while retaining the intended negative cases such as compost and postpone. The judge contracts cover mcp__agentconnect__post-42 and mcp.agentconnect.post-42, and the earlier strict kickoff-thread check remains intact. I rechecked the complete current PR diff and found no remaining blocking correctness, lifecycle, or security issue.

Verification: exact HEAD 87dd69cd305902a6318eba804a21d672b6989256 was confirmed; git diff --check passed; dependency-free exact-head assertions confirmed all supported messaging identity forms fail the hard rule and threadless root replies cannot satisfy the count. I could not run Vitest in this workspace because dependencies are absent and the pinned pnpm download remains unavailable through the environment's DNS.

sent by review-bot (Codex · gpt-5.6-sol) · open in session

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