Conversation
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head f751838e621a3e77b22df004eb086f93db0ea75b against live main@7a1aafb4e1dac5d2840cd3ab524f6d6bd0658694. The current object is 13 commits ahead / 12 behind its actual live-main merge base e1e72f109c9540d6ed2aceeaf1b5bfcacbab5efc. There were no existing PR comments/reviews when I started. I read the three RoomLink-specific commits above f9821fb1caf67fa54895fe312cd0896067ebdef3, the exact grant/catalog/dispatch ABI, the HTTP and peer-session transports, Desktop capability planning, relevant tests, source-PR lineage, exact-head CI, and the surrounding hosted-room stack.
There is a lot here worth preserving. The v2 grant expands authority into explicit room/home/authority/member/target/profile coordinates rather than a bearer-shaped ambient permission; HostedMemberDispatch binds task + execution generation + source sequence + prompt/capability digests; non-loopback RoomLink is HTTPS-only; credentialed HTTP goes through the cross-origin header guard; response-loss recovery replays one deterministic admission identity; and Stop stays tied to the exact attempt instead of guessing from current presentation state. The preserved source lineage from #88408 (pesho-vsn), #94336 (giaiant), #88819 (pierrenode), and #93952 (liuhao1024) is also explicit rather than being silently absorbed. Good work on those boundaries. 🚀
I do have two merge blockers on this exact parent object, plus one exact-head acceptance failure.
P1 — the exact backend speaks RoomLink v2 while the exact Desktop contract still accepts only v1
gateway/hosted_room_peer.py says the important part plainly:
# Version 2 adds authority/member lineage to scoped grants. It is intentionally
# not wire-compatible with the unpublished v1 draft; mixed gateways must fall
# back to Desktop-driven rooms instead of accepting a weaker token shape.
PROTOCOL_VERSION = 2catalog_mapping() / local_catalog_mapping() default protocol_versions to that PROTOCOL_VERSION, so this head's gateway advertises v2. PeerHostedRoomTransport.submit() likewise serializes protocol_version=PROTOCOL_VERSION into the recipient-validated dispatch.
But this same head's apps/desktop/src/plugins/hermes-bots/hosted-room-client.js::resolveAutonomousRoomPlan() requires v1 in both compatibility checks:
roomLink?.catalog?.protocolVersions?.includes(1)
...
!roomLink?.catalog?.protocolVersions?.includes(1)and hosted-room-client.test.mjs pins the supposed capable home/peer fixtures to protocol_versions: [1]. So the exact v2 catalog produced by this backend is classified as needing setup by the exact client ABI in the same object, while the v1 catalog that the backend explicitly says must be quarantined is the one the planner accepts.
This is already wired code, not dead prose: the Bot plugin classifies groups.capabilities responses and passes them to resolveAutonomousRoomPlan(). The immediate stacked child #95967 actually demonstrates the missing repair: it introduces ROOM_LINK_PROTOCOL_VERSION = 2 and changes both checks to .includes(ROOM_LINK_PROTOCOL_VERSION). That repair belongs in this parent contract; the child should not be required to make its dependency internally consistent.
Required repair: make the RoomLink wire version one explicit shared/parity-tested contract and require v2 at the Desktop compatibility boundary. Update the parent tests so [2] is accepted and [1] is rejected/quarantined. Add one vertical regression that takes the exact current groups.capabilities.room_link.catalog shape from the backend through classifyHostedRoomCapability() + resolveAutonomousRoomPlan() and proves a qualifying two-gateway plan succeeds, while the unpublished v1 catalog does not silently downgrade.
P1 — this stack regrows gateway/platforms/api_server.py after its godfile decomposition contract
The RoomLink-specific delta above f9821fb1… adds 602 lines and removes 28 in gateway/platforms/api_server.py; the complete stacked head is +1,137/-37 there versus live main. The exact submitted file still extends past line 7,000.
That violates the repository's standing decomposition invariant, not a style preference. #78643 is the owning gateway/platforms/api_server.py decomposition issue and states that godfiles are sharded and never regrown; #78647 records api_server.py as one of the completed godfile kills. The already-open shard #83546 owns the idempotency cluster specifically, extracting _IdempotencyCache, _idem_cache, and _make_request_fingerprint into gateway/platforms/api_server_idempotency.py with the old import surface preserved.
That is directly adjacent to the source work this RoomLink stack composes: #88408/#94336 add durable/idempotent /v1/runs behavior, and this branch currently reconciles those semantics by growing the monolith again. The correct composition is to keep the behavior and contributor credit but land it through the accepted sub-2k API-server seams rather than restore authority-bearing behavior to the killed godfile.
Required repair: extract/reconcile the peer-run admission/status/RoomLink API additions into coherent sub-2k modules (and the existing idempotency seam where applicable), preserving stable re-exports and monkeypatch surfaces. The final changed production files need to satisfy the hard 2K gate; do not resolve #83546/#78643 by textual conflict or by abandoning the shard.
Exact-head acceptance is not green
At review time the exact f751838e… workflows are:
- CI
33029962978— failure; the failing job isPython tests / Run tests(98379997648). I do not have a reliable failed-step log receipt here, so I am not inventing a cause. - Docker
33029962512— success. - Nix
33029962541— success.
The isolated two-host canary and 281 focused tests are useful development evidence, but they do not replace the red exact-head CI object. Every commit in the eventual landing stack needs its own truthful acceptance state.
Graph / landing order
The current ownership boundaries are coherent if kept explicit:
- #95314 remains the authority/replay foundation; #95622 is the same-gateway driver above it.
- #95965 is a composition carrier, not a silent supersession of #88408/#94336/#88819/#93952. If those source PRs land independently first, drop the patch-identical source commits and retain their authorship/credit.
- #95966 should own the scoped gateway-to-gateway RoomLink ABI and transport. It needs to be internally v2-consistent before anything consumes it.
- #95967 is the Desktop/user-facing consumer above this parent. Its v2 constant fix should be pulled down rather than used to mask the parent's ABI mismatch.
- #92931 remains complementary renderer relay/settlement fallback, not a second gateway authority or queue owner. #91911 remains the broader identity/delivery/approval/cancellation control-plane direction.
- #78643/#83546 own the API-server decomposition seam that this stack must compose with.
Once protocol parity is repaired at the parent boundary, the API-server work is restored to shard discipline, and the exact head is green, the rest of the scoped-grant/recovery design is shaped well for the cross-gateway layer. The strongest part of this PR is that it is already trying to make every consequential operation consume qualified identity rather than reconstruct it downstream; the remaining work is to apply that same exactness to the wire version and to the repository's module ownership boundary.
f9492d3 to
e2686ff
Compare
|
@andrexibiza thanks for the exact-head review. Both blockers are addressed on the final rebased backend head |
e528196 to
3fd47b2
Compare
3fd47b2 to
f13a13f
Compare
This diff is too large for an automated line-level review (>30 files or >200KB). Recommend a manual review. |
|
Closing this cumulative prototype now that #97681 captures the user contract, safety boundaries, and phased landing plan against the rebuilt Bot Mode tree. Its useful layer can be re-cut as a narrow current-main contribution after the foundation direction is agreed. |
Summary
Same-gateway rooms can keep working after Desktop closes, but a room containing
Bots from two gateways still needs Desktop to relay every turn.
This stacked draft adds a narrow gateway-to-gateway RoomLink so the home gateway
can dispatch an exact member turn directly, recover it after response loss, and
observe terminal state without a proprietary cloud coordinator.
The transport is text-first, explicit, scoped per room/member/profile, and
fail-closed. No user-facing cross-gateway creation path is enabled in this PR.
When a peer is provably unreachable before admission, the exact turn stays in
the durable room queue and retries with capped backoff. A timeout, connection
reset, or HTTP 5xx remains uncertain instead: it uses the same bounded
idempotent recovery path and never silently fails over to a second route.
Security and identity boundary
install, target profile, permissions, and expiry.
calls use room-scoped grants.
different origin.
Allow onceorDenyand binds the exactrequest_id, task, generation, member, and grant.Reliability boundary
and recovers the same run and execution generation.
from 100 ms to 2 s, and compacts terminal observations.
stoppinguntil the target run is terminal. Completion may win.API-server ownership
This PR closes the decomposition blocker raised on the first review rather than
adding another authority cluster to
gateway/platforms/api_server.py:api_server_runs.py(1,388 lines);api_server_room_grants.py(358 lines);api_server_run_idempotency.py(265 lines);api_server_room_dispatch.py(146 lines).Against the peer-run parent,
api_server.pyshrinks from 8,859 to 7,901 lines(
+141/-1,099). The remaining changes there are imports, stable re-exports,route wiring, and thin adapter delegates. Reducing the pre-existing 8K file
below 2K would require a separate extraction of unrelated APIs.
Boundaries
feat(bot-mode): keep same-gateway rooms running without Desktop #95622; byte transport across gateways is a separate security surface.
Validation
Rebased on verified boundary
origin/main@f3cbb262c1, exact backend head7cf0b44392:py_compilegit diff --check7cf0b44392The final UAT also covered named Bot profiles. A named profile now inherits the
gateway root's advertised RoomLink endpoint unless it explicitly overrides it;
models, credentials, tools, and every other profile setting remain isolated.
This keeps adding a Bot on the normal green path instead of requiring repeated
network configuration per profile.
Terminal peer Stop was revalidated after a real two-host canary exposed a
history/status ordering edge. Home now accepts cancellation only for the exact
peer task and execution generation before consulting the lossy history view.
The isolated two-host canary must print:
UAT_OK remote_reply=1 restart_recovered=1 stop_acknowledged=1 target_terminal=1 scoped_route_revoked=1That canary passed on the complete integration head using isolated gateway
processes, temporary state, a temporary scoped key, and a temporary HTTPS
route. Production Hermes state and processes were not used.
An additional refused-port UAT proved the offline contract: queued work stayed
durable with
blocked=0, the route becameunavailable, reconnect admitted onerun, published one reply, and returned the route to
ready.Related work
Type of change