[test][node] Cover the one-node-graph join stage-1 edge case

[thep2p]

Summary

• Add test_join_stage1_link_level0_one_node_graph_edge_case: introducer's own reply names introducer itself as s, the existing search_by_id fallback exercised unmodified. join_stage1_link_level0 needs no special-casing and completes exactly as it would for a distinct s.
• Required acceptance-criteria coverage for [Node] Implement: level-0 join linking (Phase 1) #90 (issue [Node] Implement: level-0 join linking (Phase 1) #90 explicitly lists this as one of the three required test scenarios). No production code change: the orchestration this exercises already shipped in the previous PR in this stack.

Test plan

• cargo fmt --check
• cargo clippy --all-targets -- -D warnings
• cargo clippy --all-targets --all-features -- -D warnings
• cargo test

[Copilot]

Copilot review overview

🟡 Changes recommended

The edge-case test must be corrected to exercise the required one-node and self/fallback paths.

Review effort: Lite
Findings: 1

Open (1)

• Test does not cover one-node graph with no right neighbor · New

What changed in this PR

Adds test utilities and a stage-1 join edge-case test where the introducer is selected as s.

Changes:

• Adds poll_until_some for async test coordination.
• Refactors existing tests to use the helper.
• Adds introducer-as-s join test coverage.

File	Summary
src/​node/​testutil.rs	Adds the shared polling helper.
src/​node/​base_node.rs	Refactors polling and adds the edge-case test; the fixture does not exercise the one-node z = None path or the real search_by_id fallback.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[thep2p]

@claude review

[github-actions]

Claude finished @thep2p's task —— View job

---

Review: PR #140

• Read repo files & production code (join_stage1_link_level0, send_link_request, handle_set_link_response)
• Read changed files (base_node.rs, testutil.rs)
• Analyze test coverage & correctness
• Assess the Copilot finding vs. author's rebuttal
• Post review verdict

---

Summary

This is a clean, well-targeted test-only change. It adds test_join_stage1_link_level0_introducer_is_the_closest and extracts a poll_until_some helper that de-duplicates five hand-rolled loop { … yield_now().await } polling blocks. No production code changes.

Correctness verification

I traced the new test against the production path and it holds up:

• Search direction: introducer < node_id ⇒ search_direction = Right (base_node.rs:431). The mock asserts Direction::Right on both the SearchByIdRequest and the GetNeighborOp. ✅
• s == introducer path: The search reply returns result: introducer, i.e. s is the introducer itself, and search_result != own_id, so the collision guard at base_node.rs:454 is correctly not tripped. The neighbor query then goes to s (= introducer) and returns z. ✅
• Slot mapping: send_link_request stores the waiter with dir.opposite() (base_node.rs:346) and handle_set_link_response gates the table write on res.dir + belongs_in_own_slot (base_node.rs:820-838). The test's SetLinkOp replies use dir: Left for s and dir: Right for z, so the final assertions (left = introducer, right = z) are consistent with the real write path — not just with the waiter resolution. ✅

On the Copilot finding + rename

The renamed title ..._introducer_is_the_closest now matches what the test actually exercises (introducer is s, and a right neighbor z exists). The author's rebuttal is sound: join_stage1_link_level0 branches on query_result (Some vs None) at base_node.rs:470, and that branch is independent of whether s == introducer. The Some(z) arm here and the z = None arm are orthogonal, so covering z = None in a separate test (#141) rather than combining them adds no lost coverage. The original Copilot concern about a misleading name is resolved by the rename.

Minor / non-blocking

• poll_until_some's doc comment references wait_until ("Unlike wait_until…"), but I couldn't find a wait_until in testutil.rs. If it lives elsewhere that's fine; if it was removed/renamed, the doc reference is stale.
• Nice touch that poll_until_some takes FnMut() -> Option<T> so the probe can borrow local state — the refactored call sites read much cleaner (e.g. s.zip(m)).

Note

I was unable to run cargo test / clippy (command not permitted in this environment), so my verdict is based on static analysis of the diff against the production code. The PR description lists the full fmt/clippy/test plan.

---

No blocking issues found.
• Branch: thep2p/90-join-stage1-one-node-graph-test

[github-actions]

Clean, test-only change. Traced the new test_join_stage1_link_level0_introducer_is_the_closest against the production path (search direction, s==introducer not tripping the collision guard, and slot mapping via send_link_request's dir.opposite plus handle_set_link_response's belongs_in_own_slot gate) — assertions are consistent with the real write path. The rename resolves Copilot's misleading-name concern, and the author's rebuttal is correct: the Some(z)/z=None arms are orthogonal to s==introducer, so splitting z=None into a separate PR loses no coverage. poll_until_some is a nice de-dup. Non-blocking: its doc references a wait_until I could not locate in testutil.rs (possibly stale). Could not run cargo test/clippy in this environment; verdict is from static analysis.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The test does not exercise the search_by_id fallback it is intended to cover.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Test does not cover one-node graph with no right neighbor

[thep2p]

wait_until lives in src/core/testutil/fixtures.rs; the doc now links to it by full path: d932243