feat(gateway): add revisioned conversation sync - #63149
Conversation
b535542 to
7d46560
Compare
|
Thanks for the detailed contract and concurrency work. I found no concrete correctness defect in the reviewed diff. Current The PR base ( Automated hermes-sweeper review. |
|
Superseded by #72651, which reconstructs the revisioned mobile conversation synchronization protocol on current |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two open PRs implement the same revisioned mobile conversation-synchronization protocol: #63149 adds authoritative snapshots, sequenced events, bounded replay, cursor recovery, and transport-handoff barriers on an older stack, while #72651 reconstructs those mechanisms against current main and the scoped authorization foundation in #72635.
Related pull requests
- #63149
related— (+3616/-243) — superseded by #72651: The diff implements the reported synchronization gap comprehensively, including revisioned snapshots,complete/gap/resetrecovery, UTF-8 delta offsets, replay bounds, and concurrency barriers. Despite thekeep_openreview on #63149, which found no concrete correctness defect and rated it salvageable, #72651 applies the same design to the substantially changed current gateway paths; #63149 could still be refreshed by its author or maintainers, but maintaining both implementations is unnecessary. - #72651
duplicate— (+3602/-221) — preferred current-main implementation: The diff preserves the synchronization contract from #63149 while adapting it to current crash continuation, failed-turn retention, display-history, process-isolation, transport-ownership, tool-lifecycle, and legacy-wire behavior; it is layered on the required scoped-authorization dependency #72635.
Duplicates
#63149 and #72651 are substantially duplicate implementations of the same revisioned mobile conversation-synchronization contract; #72651 is the current-main reconstruction of #63149.
Suggested consolidation
Merge #72651 after #72635 lands and required review/CI remains green, then close #63149 as superseded by #72651. This does not imply that #63149 is globally read-only or impossible to salvage; it reflects that #72651 already carries the same feature forward across the current gateway architecture while preserving behaviors added since #63149's base.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup63149 ["PRs duplicating each other"]
P63149["PR #63149 (open)"]
P72651["PR #72651 (open)"]
end
class P63149 open
class P72651 open
class P63149 target
click P63149 "https://github.com/NousResearch/hermes-agent/pull/63149"
click P72651 "https://github.com/NousResearch/hermes-agent/pull/72651"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 353 kB of PR diffs, 6 kB of issue/PR text, 2 kB of discussion (3 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
|
Agreed with the consolidation recommendation. #72651 is the maintained current-main reconstruction of this synchronization contract, preserves Eric Lewis’s original synchronization commit authorship, and is intentionally layered on #72635. No separate implementation of #63149 is planned. The active maintenance order is #72635 first, then #72651 against the refreshed authorization foundation, followed by the required CI/review gates. Once the replacement stack lands, #63149 can be closed as superseded. This triage review identifies no additional code defect requiring a change to the legacy branch. |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Delta since our previous triage comment
@freemanconsulting confirmed that #72651 is the maintained current-main reconstruction, preserves Eric Lewis’s synchronization commit authorship, and is intentionally sequenced after #72635. They also clarified that no separate implementation of #63149 is planned and identified no additional defect requiring changes to its legacy branch; this reinforces rather than changes our previous assessment.
Suggested consolidation
The consolidation recommendation is unchanged: proceed with #72635, then #72651 through the required CI/review gates, and close #63149 as superseded once the replacement lands.
Complex graph unchanged since our previous triage comment.
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Diffs were read for 1 of 2 PRs (rest unavailable); Assessment working set: 176 kB of PR diffs, 6 kB of issue/PR text, 3 kB of discussion (4 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Stack
This draft is stacked on #62858 (
feat/mobile-contract-hello-scopes). It intentionally includes that PR's commits until #62858 merges; rebase this branch ontomainafter the parent lands.d8bc3bc9b1277b54d72b097c7a5d0328ecb62348f67aae323010e32c592a185984d36b20e9fa474aericlewis/cuttle#3.Summary
complete,gap, orresetwithout claiming evicted events are recoverablegateway.readyExisting clients keep their existing fields and legacy wire behavior; sync retention is activated only after a mobile-capable transport requests it.
Validation
Post-rebase validation against parent head
d8bc3bc9b:tests/tui_gateway/test_*.pyfiles in fresh process isolation: 396 passedtests/test_tui_gateway_server.py: 316 passedtests/hermes_cli/test_dashboard_auth_ws_auth.py: 61 passedtests/hermes_cli/test_dashboard_auth_ws_tickets.py: 22 passedtests/test_tui_gateway_ws.py: 12 passedtests/hermes_cli/test_web_server_console_ws.py: 5 passedgit diff --check: passedThe
tests/tui_gatewayfiles are intentionally invoked in fresh processes because the current upstreamtest_inline_rpc_gil_starvation.pyreplaces the globalprompt.submithandler without restoring it; file-scoped isolation avoids that unrelated cross-file test leak.Deliberate limits / risks