fix(gateway): dedupe probe warnings by gateway identity#85791
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 4, 2026, 3:23 PM ET / 19:23 UTC. Summary PR surface: Source +55, Tests +216, Docs +2. Total +273 across 9 files. Reproducibility: yes. Current main builds both SSH tunnel and configured remote targets and warns on Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this focused identity-based warning fix after maintainer review, then close the linked false-positive report once the PR is merged. Do we have a high-confidence way to reproduce the issue? Yes. Current main builds both SSH tunnel and configured remote targets and warns on Is this the best way to solve the issue? Yes. Dedupe at the warning/output layer keeps the useful direct-path probe intact, while skipping redundant configured targets would reduce diagnostics coverage. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against ab0a633ab98b. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +55, Tests +216, Docs +2. Total +273 across 9 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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 PR egg ✨ Hatched: 💎 rare Moonlit Branchling Hatch commandComment Hatchability rules:
Rarity: 💎 rare. What is this egg doing here?
|
galiniliev
left a comment
There was a problem hiding this comment.
This is a focused fix for #69135: gateway probe can probe an SSH tunnel URL and a configured remote URL that both reach the same gateway, and the patch changes multiple_gateways from a raw reachable-target count to a conservative self-identity + logical-port check. The source-level behavior and the focused tests line up with the reported bug.
Findings:
-
[P2] Please align the gateway docs with the new warning semantics. The code now suppresses
multiple_gatewayswhen reachable transports share a gateway identity, but the public gateway docs still describe the warning as "more than one target was reachable" / "more than one target answered" (docs/cli/gateway.md:337,docs/cli/gateway.md:382,docs/gateway/troubleshooting.md:576). After this patch that is no longer true: two targets can answer and correctly produce no warning if they identify as the same gateway on the same logical port. Please update those docs to describe the warning as distinct or identity-ambiguous reachable gateways, and mention that SSH tunnel + configured remote can be one gateway with multiple transports. -
[P2] Please add real behavior evidence before merge, or explicitly document why maintainer acceptance is unit-only here. The implementation has good fixture coverage, and I verified the focused tests locally, but the PR body still says a live SSH tunnel plus direct WebSocket probe was not tested. For this bug, the user-visible claim is specifically the
openclaw gateway probeSSH/direct remote flow, so a redacted terminal transcript, screenshot, or logs showing the post-patch command output would close the proof gap.
Proof I ran:
node scripts/run-vitest.mjs src/commands/gateway-presence.test.ts src/commands/gateway-status/output.test.ts --reporter=verbose
pnpm exec oxfmt --check --threads=1 src/commands/gateway-presence.ts src/commands/gateway-presence.test.ts src/commands/gateway-status/output.ts src/commands/gateway-status/output.test.ts CHANGELOG.md
git diff --check upstream/main...HEADResult: focused Vitest passed (2 files, 10 tests), formatting passed, and diff whitespace passed.
Provenance: clear. The old warning behavior was carried forward from the gateway status warning path introduced around src/commands/gateway-status/output.ts by fe5819887b5; later reachability work kept the warning tied to reachable entries. This PR changes that exact warning decision point in src/commands/gateway-status/output.ts:129 and adds focused regression coverage.
Best-fix verdict: the source change is the right shape and keeps distinct, unknown, and different-port cases conservative. I would not merge until the docs/proof gaps above are resolved and the currently failing PR checks are green.
25bc974 to
ecca770
Compare
ecca770 to
250ba26
Compare
|
@clawsweeper re-review Updated this PR on current Validation run locally:
Live behavior proof:
No merge performed. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
250ba26 to
cb977a2
Compare
|
@clawsweeper re-review Updated the remaining linked gateway docs for the identity-based
The stale target-count wording is now removed from the gateway docs, and the docs describe distinct or identity-ambiguous reachable gateways while noting that an SSH tunnel plus configured remote URL can be one gateway with multiple transports. Validation run locally:
No merge performed. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
cb977a2 to
cd65418
Compare
|
Rebased onto current main and force-updated with an explicit lease. The failed checks-node-agentic-agents job timed out in src/agents/model-catalog-visibility.test.ts; that file passes locally after the rebase.\n\nValidation run locally in WSL from /root/src/openclaw-85791:\n- |
fda0302 to
f8259b6
Compare
3079755 to
97b81a2
Compare
|
Updated #85791 onto current New signed head: Validation after the refresh:
All 7 rebased commits are signed. No config surface or plugin API surface changed. No merge performed. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Updated #85791 onto current New signed head: Validation after the refresh:
All 7 rebased commits are signed. No config surface or plugin API surface changed. No merge performed. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Refreshed #85791 onto current Updates on signed head
Validation in WSL from
No public config, plugin API, CLI flag, env var, migration, or plugin contract surface was added. No merge performed. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Refreshed #85791 onto current New signed head: Validation in WSL from
No config surface or plugin API surface changed. No merge performed. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Refreshed this on current New signed head: Validation rerun locally:
No config, CLI, plugin API, or public surface changes. |
|
@clawsweeper re-review Refreshed head
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Refreshed #85791 onto current Updates on signed head
Validation in WSL from
No merge performed. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Merged via squash.
Thanks @giodl73-repo! |
Summary
main.operator.readdocs text per review feedback.Validation
OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs src/infra/system-presence.test.ts src/infra/system-presence.version.test.ts src/commands/gateway-presence.test.ts src/commands/gateway-status/output.test.ts --reporter=dot(3 shards, 22 tests)node_modules/oxfmt/bin/oxfmt --check --threads=1 src/infra/system-presence.ts docs/cli/gateway.md docs/gateway/index.md docs/gateway/multiple-gateways.md docs/gateway/troubleshooting.md src/commands/gateway-presence.test.ts src/commands/gateway-presence.ts src/commands/gateway-status/output.test.ts src/commands/gateway-status/output.tsnode_modules/oxlint/bin/oxlint --tsconfig config/tsconfig/oxlint.core.json src/infra/system-presence.ts src/commands/gateway-presence.test.ts src/commands/gateway-presence.ts src/commands/gateway-status/output.test.ts src/commands/gateway-status/output.tsgit diff --check origin/main...HEAD && git diff --checkCurrent Head
mainat69df4c9136173b7387d4cf79ccd9121c93d49932.13e3c00f56.