feat(connection): enforce gateway protocol compatibility - #1127
Conversation
|
Codex review: needs maintainer review before merge. Reviewed August 9, 2026, 2:55 AM ET / 06:55 UTC. ClawSweeper reviewWhat this changesThis PR validates Gateway wire-protocol handshakes and projects compatibility results into connection state, tray recovery guidance, local MCP diagnostics, telemetry, documentation, and tests. Merge readinessKeep open. This collaborator-authored draft changes established Gateway connection behavior and needs a maintainer decision on its intentional terminal compatibility policy before merge. Priority: P1 Review scores
Verification
How this fits togetherGateway handshake responses establish operator and Windows-node sessions. The connection manager owns that state and passes it to the tray recovery UI and the local MCP status command. flowchart LR
A[Gateway connect response] --> B[Handshake contract validation]
B --> C{Owned and compatible?}
C -->|Yes| D[Connection manager state]
C -->|No| E[Terminal compatibility recovery]
D --> F[Tray and MCP diagnostics]
E --> F
Decision needed
Why: This is an intentional upgrade and availability policy change for existing Gateway installations, not a purely mechanical bug repair. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Adopt one documented terminal compatibility policy for owned handshake failures, preserve explicit Connect and Disconnect recovery, and land it only with maintainer agreement on the upgrade behavior. Do we have a high-confidence way to reproduce the issue? Not applicable as a standalone issue. The PR body supplies a current-head local-MCP scenario matrix, and source inspection confirms the tested handshake boundary. Is this the best way to solve the issue? Unclear until maintainers choose the intended compatibility policy. The implementation is coherent, but deciding to make previously tolerated handshake shapes terminal is product and upgrade policy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4712e227d110. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (15 earlier review cycles; latest 8 shown)
|
c493b00 to
f63e7a2
Compare
f63e7a2 to
6e09cea
Compare
f9ca1c2 to
775f044
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
775f044 to
9f55849
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
Require exact connect response ownership, validate the full hello contract, separate Gateway current from effective protocol, and preserve operator recovery precedence. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve observed Gateway protocol semantics, honor structured error codes, accept schema-valid policy integers, and remove inert recovery UI seams. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Project node-only protocol failures as compatibility guidance without offering a misleading network retry, and make the operator mismatch latch visible across threads. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Related: #1105
Note
This remains a draft. Do not mark ready until exact-head hosted checks and current-head visible UI proof are complete.
What Problem This Solves
Prevents missing, unknown, stale, or racing Gateway responses from completing or failing the Windows operator/node handshake. It also fails closed on malformed successful handshakes before readiness or token persistence, and reports role-aware protocol compatibility without confusing the Gateway current constant with the effective wire protocol.
Why This Change Was Made
connectrequest ID owns handshake success and errors.hello-okshape are validated before side effects.Upstream claims are pinned in
docs/CONNECTION_PROTOCOL_RESEARCH.mdto Corev2026.6.11commite085fa1a3ffd32d0ea6917e1e6fb4ecbffbb77d2and N-1 commit912af0a56f673787cc5ea813f17582b14e187e7b.User Impact
Supported connections continue normally. Directional mismatches identify whether the Gateway or Windows app needs updating. Malformed or unowned responses cannot publish readiness, store tokens, alter pairing, or suppress reconnect. Gateway package version remains distinct from wire protocol diagnostics.
Change Type
Scope
winnodeValidation
Exact local head:
6b6b875e17f3d12918ae89f47386799d6371f6ee./build.ps1: all 5 projects passedError; no laterDisconnectedoverwrites itFinal immutable-head blind review:
Hosted CI, CodeQL, ClawSweeper,
proof: sufficient, andstatus: ready for maintainer lookare pending for this pushed head.Real Behavior Proof
Environment: isolated Windows tray data, isolated roaming/local roots, disposable loopback fake Gateway on port 19873, and isolated Local MCP on port 19874. No real credentials or user data were used.
Current-head
winnode --command app.connection.statusobservations:gateway_too_oldgateway_too_newcompatiblecompatiblegateway_too_newunauthorized)winnode --list-toolsincludedapp.connection.status. The fake Gateway used the pinned full Core response shape and exact structured mismatch fields.Proof manifest and SHA-256 hashes are retained in the private session artifact directory for run
gwcompat-20260808-213736. All run-owned processes and ports were stopped. The production roaming manifest was unchanged. A pre-existing unowned WSL VHD timestamp changed concurrently, so strict local-root isolation is reported as compromised rather than success-shaped.Current-head UI screenshots are not verified. Computer Use returned no discoverable app windows despite a responsive Companion window, and no shell/window-manager screenshot workaround was used. The older committed screenshots are not claimed as current-head evidence.
Security Impact
NoNoNoNoNoThe change fails closed before token persistence or connected state for invalid owned handshakes, and ignores unowned responses without handshake, token, state, pairing, or reconnect effects.
Compatibility and Migration
Yes, for the documented v3-v4 client range and the upstream node-only N-1 windowNoNoReview Conversations