Repository navigation
test(ffi): quarantine datagram timeout pending root-cause evidence - #7954
Conversation
Quest-Claim: aca6caa1fe19041d
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 9 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit 2685afa. No actionable findings. Quarantining the known flaky case while retaining an explicit diagnosis and re-enablement task is reasonable; retries or longer timeouts would mask the unresolved failure.
Verification: GitHub-only static review of the changed files, test inventory and relevant subscriber ordering. I did not run tests or diagnostic commands.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Quarantined the flaky FFI datagram test:
datagrams_cross_a_default_connectionnow carries#[ignore = "...ffi-datagram-roundtrip.md"], matching the existing load-flaky convention (rs/moq-gstandsrc/test/rung.rs), with the tworequested_siblings left enabled as the maintainer decided. - Registered the quarantine with the test inventory: the new
ignore moq-ffi::test::datagrams_cross_a_default_connectionentry is the exact keysh/gh/test-inventory.pyderives from the module tree, and no reachable run selects that test with--run-ignored, so it is correctly an allowlist entry rather than an error. - Reworked the quest and its epic entry: the title, goal, maintainer decisions, the 300-case stress investigation record, and a
## Waitingon #7951 are consistent, and the README## Requiredline was updated to the new title. I reran the pinned quest validator locally (quest check, 587 documents) and it passes.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Problem
The CI timeout in
datagrams_cross_a_default_connectionstill has no confirmed cause. Repeated sending would assume ordinary loss and could hide a protocol bug.Approach
Disable only the known flaky case and register its existing investigation quest in the test inventory. Record the diagnostic results and remaining work in that quest. Requested siblings remain enabled. Temporary send/route diagnostic prints were removed.
Impact
Alternatives
Do not add a longer timeout or repeated sending without evidence of ordinary loss. All three tests await
consumer.info()before appending, and the driver's answer decode and synchronous registration precede its next receive pass. The answer race addressed by moq-dev/moq#7951 was not reproduced here and cannot be claimed as the cause.Validation
quest gate just ci origin/main: passed all selected checks, including Rust tests, binding checks, GitHub tooling and quest validation. The previously observed direnv hook failure did not recur.quest check --base origin/main --branch quest/m0/test-flakes-2/ffi-datagram-roundtrip: passed (587 documents).quest gate just test interop --allpassed Rust interop 10/10, but the full matrix was incomplete because JS, js-native-node and js-native-bun setup failed withtsc: command not found; it was stopped with exit 130. Client configurations matched unchanged main. Not rerun for this documentation-only iteration; this is not a passing full interop result.Review
Claude Code,
claude-opus-5-5, medium effort, round 2 reviewed head2685afa56a79c0e3a36da9f97a499e1d94a1bb02againstorigin/main: zero findings. Both round-1 findings were fixed: the quest now gives an explicit local loaded-run command including the ignored test, and its parent epic matches the updated title and goal.After merging
origin/main(headdc3d9642e066f347e7809570ce0eca9327034665), Codexgpt-6.1-sol, medium effort, reviewed againstorigin/main(c76f9eb9f4): zero findings. The merge resolved without conflicts and the changed lines are identical to the reviewed head's; only diff context moved.just checkpassed.Decisions
✅ Decided 2026-11-05 (maintainer, /quest-plan): keep only
datagrams_cross_a_default_connectionquarantined and land this quarantine. Re-diagnose under CI load after moq-dev/moq#7951 merges, capturing a failing run with send/route diagnostics. No repeated sending without evidence of ordinary loss. Re-enabling now was rejected. The existing quest owns this wait and the remaining diagnosis; it does not block landing the quarantine.Follow-ups
The existing quest retains the unfinished diagnosis and re-enablement. Investigate the unchanged interop JS compiler setup separately; restore the intended compiler command or dependency in its own work. The previously supplied host blockers remain outside this PR.
(Written by GPT-6.1 Sol with high effort)