Repository navigation
bug(tests): mcp-server access-wiring "#2807 refused loop is audited as a refusal" reads the previous test's late fire-and-forget audit row — flaky on dev alone #2952
Copy link
Copy link
Closed
Labels
complexity-lowComplexity: low (board points 1-3)Complexity: low (board points 1-3)priority-p2ImportantImportantstatus-in-devMerged to dev, awaiting release cut to mainMerged to dev, awaiting release cut to maintheme-reliabilityTheme: ReliabilityTheme: Reliabilitytype-bugBug fixBug fix
Description
Summary
src/mcp-server/src/access-wiring.test.ts → #2807: a refused loop start is audited as a refusal, not as a successful call fails intermittently with
the refusal was audited as a success: {"tool":"run_agent_loop","duration_ms":1,"success":true}
The test is reading the previous test's audit row. Nothing in the gate or the audit stamping is wrong — the assertion is anchored on the wrong row.
Context
- Reproduces on
devalone at742b20c13, with CI's exact invocation (node --import tsx --test src/*.test.ts src/tools/*.test.ts): 2 of 2 full-suite runs on a macOS host failed this subtest; the file run alone passes 3 of 3 on the same tree. - Hit CI once on PR fix(files): a shared file is for the person the turn was for (abilityai/trinity-enterprise#549) #2936 (
MCP Server Testsrun 35730033664, head680a1e1fb), a PR that does not touch the loop tools, the access gate, or the audit wrapper; the same job is green on the last fivedevpushes. - Same class as bug(tests): test_2638 TestApiKeyFallback::test_it_never_raises depends on xdist worker history — passes only while no API key is visible, reds unrelated PRs' regression diff #2856 (a test whose verdict depends on what ran before it), on the MCP side.
Mechanism
Test order in the describe block:
an agent key with no edge is refused …— refusedthe same key with an edge starts the loop on the sibling— starts a loop (success: true)a loop on itself needs no edge …— starts a loop (success: true)#2807: a refused loop start is audited as a refusal …— refused
The audit POST /api/internal/audit is fire-and-forget (the file's own header says so). Test 4 does
const seen = auditRows.length; // captured BEFORE test 3's audit row has necessarily arrived
…
const row = await auditRowFor("run_agent_loop", seen); // first run_agent_loop row after `seen`so when test 3's success: true row lands after seen was captured, auditRowFor returns it and the assertion row.details.success === false fails. Under the full-suite run the process is busier and the late arrival is common; alone, the row is usually in before test 4 starts.
Acceptance Criteria
- The
#2807subtest anchors on its own call's audit row, not "the firstrun_agent_looprow after an index" — e.g. each test that triggers an audit awaits its row before finishing (drain), or the finder matches a per-call discriminator (the key label / a request marker) rather than the tool name alone. - The full suite (
node --import tsx --test src/*.test.ts src/tools/*.test.ts) passes 10 of 10 consecutive runs ondev. - The same anchoring is applied to the sibling
#2807: deny then allow on ONE session …test if it readsauditRowsthe same way.
Technical Notes
src/mcp-server/src/access-wiring.test.ts—auditRowFor(tool, after)and the fouritblocks above;reset()clearsloopPostsbut notauditRows, which is why an index is used.- Cheapest durable fix: make
reset()await the previous test's audit row (or clearauditRowsafter awaiting it), soseenis always a settled index. A discriminator match is more robust if tests ever run concurrently. - Trinity-side audit stamping (bug(mcp): a denied chat_with_agent / fan_out call is audited as a successful tool call #2807) is not implicated: the refused call's own row does carry
success: false, denied: true— the test just finds an earlier row first.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
complexity-lowComplexity: low (board points 1-3)Complexity: low (board points 1-3)priority-p2ImportantImportantstatus-in-devMerged to dev, awaiting release cut to mainMerged to dev, awaiting release cut to maintheme-reliabilityTheme: ReliabilityTheme: Reliabilitytype-bugBug fixBug fix
Activity
merge-train: merge dev, rebuild tests/registry.json (#2952) — mechani…
test(mcp): each loop-tool call drains and identity-checks its own aud…