Repository navigation
chore: sync fork main and harden hosted-room lifecycle - #19
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5b8f61c98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
૮ >ﻌ< ა ci reviewran on 20d0a6a — fix(groups): preserve concurrent disband and legacy approval
|
… hops stop dropping it (NousResearch#99176) The PKCE payload is a flat 'provider=...;state=...;verifier=...;next=...' string. A raw ';' is a cookie-attribute terminator, so Python's http.cookies emits the value in RFC 6265 quoted form with each ';' escaped as the backslash-octal '\073'. Mainstream browsers echo that form back verbatim and Python parsers decode it — the browser round trip is fine. But '"' and '\' are outside the plain cookie-octet set, and non-Python hops that re-serialize the Cookie header reject the value and drop the cookie entirely: Go's net/http (Traefik middleware, Authentik outposts, other gateways) refuses any cookie value containing a backslash. The OIDC callback then 400s with "Missing PKCE state cookie" even though the browser sent the cookie. Field reproduction: support thread "Still unable to use Authentik for signin with traefik" — devtools showed the browser sending the intact quoted \073 cookie on /auth/callback while Hermes logged missing_pkce_cookie behind a Traefik+Authentik chain. Fix: URL-encode the whole payload in set_pkce_cookie (quote(payload, safe='') — ';' becomes '%3B') so the wire value contains only cookie-octets and no parser in the chain has anything to reject, and decode through a single shared inverse, cookies.parse_pkce_payload(), in BOTH readers: the OAuth /auth/callback and the native password-login path (routes.login_submit), whose broker/provider binding check would otherwise parse zero segments from the newly-encoded value and silently disable itself. Regression coverage: the wire-shape test pins the full cookie-octet set (the '"'/'\' assertions are the ones a Go-parser hop fails pre-fix), the round-trip tests drive the real /auth/login → /auth/callback path, and the next= test pins the exact post-login redirect byte shape. Native-flow broker assertions updated to decode through parse_pkce_payload instead of substring-matching the raw wire value. Salvaged from NousResearch#84065 (rebased onto current main, which gained the SameSite=None PKCE attrs and the RFC 8252 native password flow since the PR branched): kept main's _pkce_attrs cookie shape, extended the fix to the login_submit reader the original PR predated, and reframed the rationale — browsers do NOT truncate at the first ';' (there is no literal ';' on the wire in the quoted form); the failing hop is a strict middlebox cookie parser. Closes NousResearch#83832 Co-authored-by: Kailigithub <12250313+Kailigithub@users.noreply.github.com>
_drain_polling_connections still bounded its shutdown()/initialize() with asyncio.wait_for (NousResearch#66377), while its sibling the general-pool drain moved to _await_with_thread_deadline (NousResearch#98094). httpcore's pool close runs under AsyncShieldCancellation, so a cancellation-resistant close keeps wait_for pending forever even after its timeout fires — the tracked _polling_error_task wedges and every escalation gate behind it stalls. Use the same wall-clock deadline helper (cancel + abandon, no cancel-await) on both polling-drain awaits, and add a regression test whose close swallows cancellation — the shape the existing cancellable-hang test cannot catch.
_looks_like_connect_timeout and _looks_like_pool_timeout carried two copies of the same 15-line DFS skeleton (seen-set, stack, __cause__/ __context__ descent) differing only in the one-line match predicate — follow-up to the NousResearch#98094 review. Extract _iter_exception_graph() and collapse both classifiers onto it. Behavior is byte-identical (subprocess parity vs origin/main on real PTB error fixtures: 6/6 identical), and the two classifiers gain direct unit tests for the first time, including the cycle/diamond chain shapes the inline copies had no coverage for.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca18e49eae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
…mary identity (NousResearch#99206) The stream consumer called prefers_fresh_final_streaming(text, metadata=...) only, and no metadata producer stamps a platform key — so RelayAdapter's hook always fell back to the PRIMARY descriptor's platform (the scalar-vs-per-chat capability seam, third occurrence). Two failure directions on multiplexed relays with platforms.relay.extra.slack.unfurl_links/media: true (NousResearch#97957): - Slack primary fronting Telegram/Discord: every link-bearing streamed final on the non-Slack chats finalized as a fresh send with no delete op advertised -> the answer delivered TWICE (orphaned preview). - Non-Slack primary fronting Slack: the hook returned False, leaving the force-on unfurl feature dark on exactly the chats it shipped for. Pass chat_id=self.chat_id from the consumer; the relay hook already accepted it and resolves via _platform_by_chat + the per-platform negotiated descriptor. Graduated TypeError fallback keeps the single-platform hook signatures (Telegram, base class) and legacy test doubles working unchanged. Both regression tests verified RED against the unfixed consumer, GREEN with the fix; single-platform relays are unaffected (NousResearch#97957's own 30 tests unchanged-green).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d42059049
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87dec320de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
hermes-agent-almorshednet/gateway/hosted_room_replicas.py
Lines 657 to 660 in 37be945
Whenever a locally authoritative room is demoted, the preceding insert adds an authority.lost event, but this update advances only next_seq and never adds the event's encoded size to event_bytes. After one or more promotion/demotion cycles, both per-room and gateway-wide byte accounting understate the durable log, allowing later appends to exceed the configured storage bounds; compute the event size, apply the control-event capacity check, and increment event_bytes in this transaction.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1ef0ea7d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27cbc9bbbb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please review exact head 10618eb. All 18 prior review threads now have exact-head evidence and are resolved. Local official-runner evidence: 310/310 across the eight modified hosted-room test files and 35/35 Telegram focused tests; all required GitHub checks are green. Please focus on Stop ownership/reclaim, admission fences, terminal publication correlation/reserve, and process identity, and report only findings that still apply to this exact head. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10618eb097
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please review exact head 7c6a0c8 only. All required GitHub checks pass and all prior review threads are resolved with exact-head evidence. Focus on atomic Stop/admission fencing, durable discussion and terminal capacity liabilities, correlated terminal replay, direct and cross-process demotion barriers, concurrent interrupt claims, and regressions across process boundaries. Re-open or add only findings that still apply to this SHA; do not rely on reviews of older commits. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c6a0c8e64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6db80964f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please review exact head b7f1acd. Focus on the three remaining hosted-room concerns: replica promotion reserve accounting, durable member identity for approval actions including legacy payloads, and the durable disband admission barrier. Local evidence: 379 focused/broad tests passed via scripts/run_tests.sh, ruff passed, and git diff --check passed. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7f1acdef0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please review exact head 20d0a6a. This follow-up addresses the two prior exact-head findings: concurrent disband idempotence after another process commits the tombstone, and atomic migration/retirement of already-published legacy approval rows while preserving compatible decisions. Local evidence: 141 focused tests passed; 389 hosted-room/group tests passed; |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Current head
87dec320debe42f6288d89327ee5d84669d8a2cfVerification
3338712384333/33, virtual-history cache17/17, Knowledge Sync36/36, Windows backend readiness21/21uv lock --check,npm audit --omit=dev, andgit diff --checkpassed0/15unresolved after exact-head evidence was postedReview gate
A Codex review was requested for the exact current head. Do not merge until that review completes, the head is re-read, required checks remain green, and explicit merge approval is bound to the final SHA.
Safety