Skip to content

test(ffi): quarantine datagram timeout pending root-cause evidence - #7954

Merged
kixelated merged 6 commits into
mainfrom
quest/m0/test-flakes-2/ffi-datagram-roundtrip
Nov 5, 2026
Merged

kixelated merged 6 commits into
mainfrom
quest/m0/test-flakes-2/ffi-datagram-roundtrip

Conversation

@kixelated

@kixelated kixelated commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The CI timeout in datagrams_cross_a_default_connection still 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

  • Public API and wire: unchanged.
  • One flaky test is quarantined until diagnosis; this does not complete the quest.
  • No deployment, publication, release, workflow dispatch, host configuration, or repository setting changes.

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

  • Unchanged baseline: all three datagram connection cases passed.
  • Earlier diagnostic investigation: 300/300 stress cases passed, with 300 successful sends, 300 routed receives and zero route misses. This does not establish the cause of the CI timeout.
  • Final 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).
  • Prior implementation validation: quest gate just test interop --all passed Rust interop 10/10, but the full matrix was incomplete because JS, js-native-node and js-native-bun setup failed with tsc: 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 head 2685afa56a79c0e3a36da9f97a499e1d94a1bb02 against origin/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 (head dc3d9642e066f347e7809570ce0eca9327034665), Codex gpt-6.1-sol, medium effort, reviewed against origin/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 check passed.

Decisions

✅ Decided 2026-11-05 (maintainer, /quest-plan): keep only datagrams_cross_a_default_connection quarantined 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)

@kixelated kixelated changed the title moq-ffi's datagram tests don't hinge on one datagram test(ffi): quarantine datagram timeout pending root-cause evidence Nov 5, 2026
kixelated and others added 2 commits November 5, 2026 08:50
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
@kixelated
kixelated marked this pull request as ready for review November 5, 2026 17:23
@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You'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.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 800af7fc-1ad1-4dc0-997b-5a24e724c923
Commits

Reviewing files that changed from the base of the PR and between 99200d9 and dc3d964.

Files selected for processing: 4

  • quest/m0/test-flakes-2/README.md
  • quest/m0/test-flakes-2/ffi-datagram-roundtrip.md
  • rs/moq-ffi/src/test.rs
  • sh/gh/test-inventory.allow
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

  • Quarantined the flaky FFI datagram test: datagrams_cross_a_default_connection now carries #[ignore = "...ffi-datagram-roundtrip.md"], matching the existing load-flaky convention (rs/moq-gst and src/test/rung.rs), with the two requested_ siblings left enabled as the maintainer decided.
  • Registered the quarantine with the test inventory: the new ignore moq-ffi::test::datagrams_cross_a_default_connection entry is the exact key sh/gh/test-inventory.py derives 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 ## Waiting on #7951 are consistent, and the README ## Required line was updated to the new title. I reran the pinned quest validator locally (quest check, 587 documents) and it passes.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 5, 2026
Merged via the queue into main with commit 1938eb1 Nov 5, 2026
9 of 10 checks passed
@kixelated
kixelated deleted the quest/m0/test-flakes-2/ffi-datagram-roundtrip branch November 5, 2026 19:22
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