Skip to content

[Repo Assist] test(connection): add missing DeriveOverall edge-case coverage#595

Draft
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/improve-deriveoverall-tests-2026-05-30-2f79d46c47d7ce56
Draft

[Repo Assist] test(connection): add missing DeriveOverall edge-case coverage#595
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/improve-deriveoverall-tests-2026-05-30-2f79d46c47d7ce56

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This is an automated PR from Repo Assist.

Summary

Adds 6 missing [InlineData] cases to the DeriveOverall_ReturnsCorrectState theory in ConnectionStateMachineTests.cs. These cases exercise branches in GatewayConnectionSnapshot.DeriveOverall that are reachable in production but were not previously covered.

Missing cases filled

Operator Node nodeEnabled Expected Why important
Connected Error false Ready Node errors suppressed when node mode disabled
Connected PairingRejected false Ready Same — node degraded states ignored when disabled
Connected RateLimited false Ready Same
Connected Connecting false Ready Node-connecting ignored when node disabled
Connected Idle true Connected Startup fallthrough — operator connected, node not yet started
Connected PairingRequired false PairingRequired Pairing requirement surfaces regardless of nodeEnabled

The most significant gap was Connected + Idle + nodeEnabled=true → Connected — this is the real state during startup (operator handshake done, node not yet initiated) and hit the last fallthrough branch without test coverage.

Test Status

All 268 Connection tests pass:

Passed! - Failed: 0, Passed: 268, Skipped: 0, Total: 268

(8 pre-existing Windows-specific failures in OpenClaw.Shared.Tests — unrelated to this change, reproducible on main before this PR.)

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

Add 6 new [InlineData] entries to the existing DeriveOverall_ReturnsCorrectState
theory in ConnectionStateMachineTests.cs that were not previously covered:

- Connected + Error/PairingRejected/RateLimited + nodeEnabled=false → Ready
  (node errors are suppressed when node mode is disabled; was only tested for
  nodeEnabled=true)
- Connected + Connecting + nodeEnabled=false → Ready
  (node-connecting state is ignored when node is disabled)
- Connected + Idle + nodeEnabled=true → Connected
  (fallthrough case: operator is connected, node not yet started — real startup
  state, reached the last "if (op == Connected) return Connected" branch)
- Connected + PairingRequired + nodeEnabled=false → PairingRequired
  (node pairing requirement surfaces regardless of nodeEnabled)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 30, 2026

Codex review: needs maintainer review before merge. Reviewed May 29, 2026, 9:37 PM ET / 01:37 UTC.

Summary
The branch adds six InlineData rows to ConnectionStateMachineTests covering GatewayConnectionSnapshot.DeriveOverall edge cases.

Reproducibility: not applicable. as a bug reproduction. Source inspection shows the current DeriveOverall branches are reachable, and the PR adds targeted theory rows for cases missing from current main.

Review metrics: 2 noteworthy metrics.

  • Test-only surface: 1 test file changed, 0 production files. This keeps merge risk low and focuses review on whether the added cases match current connection-state behavior.
  • Coverage rows added: 6 InlineData cases added. The count maps directly to the stated DeriveOverall branch coverage gap.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Run the repository-required build, shared tests, and tray tests on the PR head before merge.

Risk before merge

  • [P1] Repository-required build, shared tests, and tray tests were not run during this read-only review; the PR body only reports focused connection-test output and notes shared-test failures that should be verified before merge.

Maintainer options:

  1. Decide the mitigation before merge
    Land the focused test coverage after required repo validation confirms the PR head has no new failures.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No automated repair is needed; maintainer action is draft readiness plus required validation before deciding whether to merge.

Security
Cleared: The diff is test-only and does not change dependencies, workflows, secrets handling, package resolution, or runtime code.

Review details

Best possible solution:

Land the focused test coverage after required repo validation confirms the PR head has no new failures.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug reproduction. Source inspection shows the current DeriveOverall branches are reachable, and the PR adds targeted theory rows for cases missing from current main.

Is this the best way to solve the issue?

Yes. Extending the existing theory with the missing rows is the narrowest maintainable way to cover these edge cases without changing runtime behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against f52b829a6f3c.

Label changes

Label justifications:

  • P3: This is low-risk test coverage cleanup for an existing connection-state helper.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a bot-authored test-only PR, so the external contributor real-behavior proof gate does not apply; the PR body reports focused test output.
Evidence reviewed

Acceptance criteria:

  • [P1] ./build.ps1.
  • [P1] dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore.
  • [P1] dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore.

What I checked:

Likely related people:

  • ranjeshj: The central DeriveOverall implementation and existing ConnectionStateMachineTests theory both trace to the merged SetupEngine connection refactor commit. (role: introduced behavior and current tests; confidence: high; commits: cefce3952ab1; files: src/OpenClaw.Connection/GatewayConnectionSnapshot.cs, tests/OpenClaw.Connection.Tests/ConnectionStateMachineTests.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. repo-assist status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants