[Repo Assist] test(connection): expand SshTunnelService state-machine coverage

[github-actions]

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

Expands SshTunnelServiceTests.cs from 1 test to 12 tests, covering the full state machine and observable properties of SshTunnelService without requiring a real SSH process.

What was tested before

• ResetNotConfigured happy path only

New tests added (11)

Test	What it verifies
InitialState_IsNotConfiguredWithNoError	Fresh service starts in NotConfigured, IsActive=false, LocalTunnelUrl=null
MarkRestarting_SetsRestartingStatusAndErrorMessage	MarkRestarting(exitCode) transitions to Restarting, sets LastError
MarkRestarting_ErrorMessageContainsExitCode	Error message includes the exit code string
Stop_FromNotConfigured_StatusRemainsNotConfigured	Stop() with no process keeps status as NotConfigured (no spurious Stopped transition)
Stop_FromRestarting_TransitionsToStopped	Stop() after MarkRestarting transitions to Stopped
Stop_ClearsBrowserProxyPorts	Stop() zeroes CurrentBrowserProxyLocalPort / CurrentBrowserProxyRemotePort
LocalTunnelUrl_IsNullWhenNotRunning	LocalTunnelUrl is null when no process is running
CreateSnapshot_DefaultState_ReturnsNotConfiguredSnapshot	Snapshot reflects NotConfigured, IsRunning=false, LastError=null
CreateSnapshot_AfterMarkRestarting_CapturesErrorAndStatus	Snapshot captures Restarting status and error message
ResetNotConfigured_FromStopped_RestoresNotConfigured	Full Restarting → Stopped → NotConfigured cycle
Dispose_DoesNotThrow	Dispose() on a fresh service does not throw

Rationale

SshTunnelService is central to SSH gateway connectivity and restart logic. Only one state transition was previously covered. The new tests exercise the NotConfigured → Restarting → Stopped → NotConfigured lifecycle and snapshot consistency, which guards against regressions in reconnect/restart flows.

Test Status

• dotnet test ./tests/OpenClaw.Connection.Tests/ — Passed: 278/278 ✅
• dotnet test ./tests/OpenClaw.Tray.Tests/ — Passed: 956/958 (2 pre-existing skips) ✅
• dotnet test ./tests/OpenClaw.Shared.Tests/ — Passed: 2041/2078 (8 pre-existing McpHttpServerTests/ExecApprovalV2NormalizationTests infrastructure failures) ✅

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

Add this agentic workflows to your repo

To install this agentic workflow, run

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

[clawsweeper]

Codex review: needs changes before merge. Reviewed June 6, 2026, 9:10 AM ET / 13:10 UTC.

Summary
The PR adds 11 unit tests to SshTunnelServiceTests covering initial state, restarting/stopped transitions, snapshots, null tunnel URL behavior, proxy-port cleanup, and dispose behavior.

Reproducibility: not applicable. This PR adds unit coverage rather than reporting a user-facing bug. Source inspection of current master shows the coverage gap is real because only the original reset test exists there.

Review metrics: 1 noteworthy metric.

• Test-only surface: 1 file changed, 11 tests added, +130/-1. The change has low runtime blast radius, so review should focus on whether the added assertions provide real coverage.

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:

• Make Stop_ClearsBrowserProxyPorts exercise a non-zero proxy-port state, or rename/remove it so the test name matches what it actually proves.

Risk before merge

• [P1] The review did not independently run build/tests because this cleanup pass is read-only; the PR body reports test runs, but merge should still rely on CI or maintainer validation.
• [P1] The new proxy-port cleanup test can pass without proving cleanup from a non-zero proxy-port state, so it may give false confidence for that specific behavior.

Maintainer options:

1. Decide the mitigation before merge
   Land a focused test-only coverage PR whose assertions accurately exercise reachable SshTunnelService state transitions and whose validation is clean.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

• [P2] A narrow PR-branch repair can address the weak proxy-port assertion without changing the production SSH tunnel behavior.

Security
Cleared: The diff is test-only and does not touch dependencies, CI, scripts, credentials, permissions, or runtime security boundaries.

Review findings

• [P3] Exercise non-zero proxy ports before asserting cleanup — tests/OpenClaw.Connection.Tests/SshTunnelServiceTests.cs:89-93

Review details

Best possible solution:

Land a focused test-only coverage PR whose assertions accurately exercise reachable SshTunnelService state transitions and whose validation is clean.

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

Not applicable; this PR adds unit coverage rather than reporting a user-facing bug. Source inspection of current master shows the coverage gap is real because only the original reset test exists there.

Is this the best way to solve the issue?

Mostly yes, but not completely: a test-only PR is the right boundary, while the proxy-port cleanup test should first establish a non-zero port state or stop claiming that coverage.

Full review comments:

• [P3] Exercise non-zero proxy ports before asserting cleanup — tests/OpenClaw.Connection.Tests/SshTunnelServiceTests.cs:89-93
MarkRestarting leaves both browser-proxy port properties at their default 0 values, so this test still passes if Stop() stops clearing a previously active proxy forward. Set up a non-zero proxy-port state before calling Stop(), or rename/remove the test so it does not claim cleanup coverage.
  Confidence: 0.92

Overall correctness: patch is correct
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

• add P3: This is low-risk test coverage and polish for SSH tunnel state-machine behavior, with no runtime code change.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a github-actions[bot] Repo Assist PR that changes tests only, so the external contributor real-behavior proof gate is not applicable.

Label justifications:

• P3: This is low-risk test coverage and polish for SSH tunnel state-machine behavior, with no runtime code change.
• 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 github-actions[bot] Repo Assist PR that changes tests only, so the external contributor real-behavior proof gate is not applicable.

Evidence reviewed

Acceptance criteria:

• [P1] dotnet test ./tests/OpenClaw.Connection.Tests/OpenClaw.Connection.Tests.csproj.
• [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:

• PR diff: The public PR diff changes only tests/OpenClaw.Connection.Tests/SshTunnelServiceTests.cs, adding 130 lines and 11 new tests on top of the existing reset test. (tests/OpenClaw.Connection.Tests/SshTunnelServiceTests.cs:26, f3c47e53221a)
• Current master baseline: Current master still has only the existing ResetNotConfigured_ClearsStoppedTunnelErrorState test, so the PR is not obsolete on the default branch. (tests/OpenClaw.Connection.Tests/SshTunnelServiceTests.cs:5, 4be005707f44)
• Proxy cleanup test gap: MarkRestarting only sets Status and LastError, while Stop() clears browser-proxy ports; the proposed Stop_ClearsBrowserProxyPorts test starts from the default zero ports and does not prove cleanup from a non-zero state. (src/OpenClaw.Connection/SshTunnelService.cs:50, 4be005707f44)
• Connection ownership context: The architecture docs identify OpenClaw.Connection as the owner of SshTunnelService/Manager and place connection tests under tests/OpenClaw.Connection.Tests/. (docs/CONNECTION_ARCHITECTURE.md:19, 4be005707f44)
• Feature history: git blame and git log --follow show commit 85445c78066b2c4b5927e1e72a9a376f443ae8f4 introduced SshTunnelService, SshTunnelSnapshot, and the existing test file; git tag --contains shows that commit is in v0.6.3. (src/OpenClaw.Connection/SshTunnelService.cs:25, 85445c78066b)
• Repository policy: AGENTS.md was read fully; it requires validation after changes and directs connection work through the connection architecture docs. This review did not run artifact-generating validation because the task is explicitly read-only. (AGENTS.md:3, 4be005707f44)

Likely related people:

• christineyan4: Blame and file history show Christine Yan introduced SshTunnelService, SshTunnelSnapshot, and the existing SshTunnelServiceTests file in commit 85445c78066b2c4b5927e1e72a9a376f443ae8f4. (role: introduced behavior; confidence: high; commits: 85445c78066b; files: src/OpenClaw.Connection/SshTunnelService.cs, src/OpenClaw.Connection/SshTunnelSnapshot.cs, tests/OpenClaw.Connection.Tests/SshTunnelServiceTests.cs)
• Vincent Koc: Recent history in src/OpenClaw.Connection and connection tests includes multiple node/pairing connection fixes, making this a plausible adjacent routing candidate if maintainers want connection-layer review. (role: recent connection area contributor; confidence: medium; commits: ee8fd4ef4ff0, eb06fba21c44; files: src/OpenClaw.Connection, tests/OpenClaw.Connection.Tests)
• Ranjesh Jaganathan: History shows Ranjesh extracted the OpenClaw.Connection project, which is the ownership boundary containing the tested service. (role: adjacent owner; confidence: medium; commits: ffeff39c16c5; files: src/OpenClaw.Connection, tests/OpenClaw.Connection.Tests)

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.