Skip to content

[Repo Assist] test(connection): add OperatorScopeHelper and ChatNavigationReadiness tests#530

Draft
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/improve-operatorscopehelper-tests-2026-05-24-7428b3954b1b9553
Draft

[Repo Assist] test(connection): add OperatorScopeHelper and ChatNavigationReadiness tests#530
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/improve-operatorscopehelper-tests-2026-05-24-7428b3954b1b9553

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This is an automated PR from Repo Assist.

What

Adds 17 unit tests in OpenClaw.Connection.Tests for two untested helpers:

OperatorScopeHelper.CanApproveDevices

  • Empty scope list → false
  • operator.admin and operator.pairing accepted (both spellings, all cases)
  • Non-approval scopes rejected (operator.read, admin, pairing, etc.)
  • Approval scope within a mixed list → true
  • Mixed list with no approval scope → false

ChatNavigationReadiness.IsOperatorHandshakeReady + WaitForOperatorHandshakeAsync

  • Null manager → true (opt-out semantics)
  • Connected state → ready; all other states → not ready
  • Already connected → WaitFor resolves immediately
  • Timeout path → returns false
  • StateChanged event fires Connected mid-wait → resolves true
  • CancellationToken cancelled → OperationCanceledException

Why

OperatorScopeHelper.CanApproveDevices gates device-pairing approval in GatewayConnectionManager and ConnectionPage but had no dedicated tests. The case-insensitive scope matching is security-relevant (a scope string comparison error could allow or block pairing unexpectedly). ChatNavigationReadiness is similarly used before routing to chat and had no test coverage for edge cases like cancellation.

Test Status

dotnet test tests/OpenClaw.Connection.Tests/ → Passed: 256 (was 239), Failed: 0
dotnet test tests/OpenClaw.Shared.Tests/ → Passed: 1928, Failed: 8 (pre-existing McpHttpServer failures, not caused by 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

… tests

17 new tests covering:
- OperatorScopeHelper.CanApproveDevices: empty list, operator.admin/operator.pairing
  (case-insensitive), non-approval scopes, mixed scope lists
- ChatNavigationReadiness.IsOperatorHandshakeReady: null manager, connected, not-connected
- ChatNavigationReadiness.WaitForOperatorHandshakeAsync: already connected, null manager,
  timeout, state-event-driven success, cancellation token

These helpers are used in GatewayConnectionManager and ConnectionPage for pairing-approval
gating but had no dedicated unit tests.

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

clawsweeper Bot commented May 24, 2026

Codex review: needs changes before merge.

Latest ClawSweeper review: 2026-05-24 01:41 UTC / May 23, 2026, 9:41 PM ET.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The PR adds a new connection test file covering OperatorScopeHelper scope matching and ChatNavigationReadiness handshake readiness/wait behavior.

Reproducibility: yes. for the patch defect by source inspection: the added private _operatorState field is assigned but never read, and test projects treat warnings as errors. I did not run the build or tests because this review is required to keep the checkout read-only.

PR rating
Overall: 🧂 unranked krab
Proof: 🌊 off-meta tidepool
Patch quality: 🧂 unranked krab
Summary: Useful test intent, but the patch is not quality-ready until the warnings-as-errors compile blocker is removed.

Rank-up moves:

  • Remove the unused _operatorState field or make the stub actually read it.
  • Run the focused connection tests and the repository-required validation commands after the repair.
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.

Real behavior proof
Not applicable: Not applicable because this is a bot-authored, test-only PR; build and test validation are the relevant proof path.

Risk before merge

  • Repository-required validation has not been demonstrated on a repaired head; the PR body reports focused connection tests passing but shared tests failing from claimed pre-existing failures.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the added coverage, remove the unused stub state, then validate the focused connection tests and the repository-required build/shared/tray checks.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
A repair worker can handle the narrow compile cleanup in one new test file without a product decision.

Security
Cleared: The diff is limited to one C# unit-test source file and adds no dependencies, workflows, scripts, secrets, or runtime code.

Review findings

  • [P2] Remove the unused stub field — tests/OpenClaw.Connection.Tests/OperatorScopeHelperTests.cs:121
Review details

Best possible solution:

Keep the added coverage, remove the unused stub state, then validate the focused connection tests and the repository-required build/shared/tray checks.

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

Yes for the patch defect by source inspection: the added private _operatorState field is assigned but never read, and test projects treat warnings as errors. I did not run the build or tests because this review is required to keep the checkout read-only.

Is this the best way to solve the issue?

No as submitted; the coverage target is appropriate, but the stub should avoid unused state before the PR can merge cleanly.

Label changes:

  • add P2: This is a bounded test-coverage PR with a likely build-blocking warning, not a runtime emergency or broad product change.

Label justifications:

  • P2: This is a bounded test-coverage PR with a likely build-blocking warning, not a runtime emergency or broad product change.
  • rating: 🧂 unranked krab: Current PR rating is 🧂 unranked krab because proof is 🌊 off-meta tidepool, patch quality is 🧂 unranked krab, and Useful test intent, but the patch is not quality-ready until the warnings-as-errors compile blocker is removed.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Not applicable because this is a bot-authored, test-only PR; build and test validation are the relevant proof path.

Full review comments:

  • [P2] Remove the unused stub field — tests/OpenClaw.Connection.Tests/OperatorScopeHelperTests.cs:121
    tests/Directory.Build.props sets TreatWarningsAsErrors=true, and this private field is assigned but never read. That produces CS0414 as an error for the new test file, so remove the field or derive any needed state from CurrentSnapshot.
    Confidence: 0.93

Overall correctness: patch is incorrect
Overall confidence: 0.91

Acceptance criteria:

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

What I checked:

Likely related people:

  • shanselman: Current helper-file blame points to the merge commit authored by Scott Hanselman, and a later commit by the same person adjusted adjacent node auto-approve behavior. (role: recent area contributor and merger; confidence: high; commits: c499c294b764, 45dccec60c51; files: src/OpenClaw.Connection/OperatorScopeHelper.cs, src/OpenClaw.Connection/ChatNavigationReadiness.cs, src/OpenClaw.Connection/GatewayConnectionManager.cs)
  • bkudiess: GitHub PR metadata for the merged branch that introduced the helper files identifies bkudiess as the branch author. (role: feature branch contributor; confidence: medium; commits: c499c294b764; files: src/OpenClaw.Connection/OperatorScopeHelper.cs, src/OpenClaw.Connection/ChatNavigationReadiness.cs, tests/Directory.Build.props)
  • Ranjesh: Commit history shows Ranjesh added adjacent connection and pairing behavior that used the approval-scope helper and added related connection tests. (role: adjacent area contributor; confidence: medium; commits: 7747e6075052; files: src/OpenClaw.Connection/GatewayConnectionManager.cs, src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml.cs, tests/OpenClaw.Connection.Tests/NodePairAutoApproveTests.cs)

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

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels May 24, 2026
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 24, 2026

ClawSweeper PR egg

🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress.

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.
What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@clawsweeper clawsweeper Bot added the P2 Normal priority bug or improvement with limited blast radius. label May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. repo-assist status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants