fix(matrix): recover from rejected sync cursors

[nepenth]

Summary

Recover from a rejected Matrix /sync pagination cursor without undoing main's structured auth classifier.

Some homeservers and reverse proxies reject a persisted since token as 403 / M_FORBIDDEN (or M_UNKNOWN_POS). Main currently treats every structured 403 as permanent auth and stops the loop, even when a cursor is present. This PR keeps the classifier (errcode + http_status only; no HTML substring 403 scans) and adds a bounded cursor reset.

Behavior

• 401 / M_UNKNOWN_TOKEN / M_MISSING_TOKEN still stop the loop.
• Structured 403 / M_FORBIDDEN without a cursor still stop the loop.
• Structured 403 / M_FORBIDDEN with a live since token, or M_UNKNOWN_POS, clear the cursor, persist next_batch=None, and retry a snapshot. Budget: 3 consecutive resets (a healthy snapshot clears the counter). A later 403 with no cursor is permanent auth.
• Unstructured 5xx/HTML that happens to contain the digits 403 still retries with the same cursor (main's false-positive fix is preserved).
• Snapshot recovery drops rooms that disappeared from the join set.

Tests

python -m pytest tests/gateway/test_matrix.py -q

124 passed on this head.

Non-goals

• Changing Matrix display defaults or Tool activity.
• Treating message/HTML text as an auth signal.
• Recovering from a revoked access token.

Security

No new network, credentials, or shell execution. Sync errors are classified from structured mautrix attributes only.

[tonydwb]

Code Review Summary

Verdict: Approved

Looks Good

• Fixes Matrix sync cursor recovery when cursors are rejected by the server
• Handles the edge case gracefully without losing sync state
• Test coverage added
• Scoped, well-documented fix
• No security concerns

[teknium1]

Thanks for the focused Matrix recovery patch. The premise is confirmed on current main: plugins/platforms/matrix/adapter.py:2268-2279 sends the stored cursor as since, while plugins/platforms/matrix/adapter.py:2327-2337 treats any exception containing 403 as terminal and stops the loop.

Problems

• The new tests exercise forbidden errors only as raised exceptions. The PR also adds a separate non-exception sync_data branch, while existing tests/gateway/test_ws_auth_retry.py:115-153 documents a SyncError-style result-object path through this loop. That new branch is not regression-covered.

Suggested changes

• Add one result-object 403/M_FORBIDDEN recovery test with a stored cursor, and one no-cursor terminal test. This would validate both newly introduced forbidden paths.

This is an automated hermes-sweeper review.

[nepenth]

@teknium1 Addressed on the current head (3108c1ef5).

MatrixAdapter._sync_loop() now distinguishes rejected incremental cursors from permanent token failures for both raised errors and SyncError-like result objects. When a stored cursor is present, _reset_rejected_sync_cursor() clears memory and the sync store and resumes from a fresh sync under a bounded reset budget. With no stored cursor, the same 403 / M_FORBIDDEN result is terminal: it does not write the store, sleep, or call handle_sync.

The two requested result-object regressions are in tests/gateway/test_matrix.py:

• stored cursor → clear, retry with since=None, persist the recovered cursor;
• no cursor → stop after one sync call without mutation or retry.

Fresh focused verification: 11 passed, 247 deselected; py_compile and git diff --check origin/main...HEAD also pass. I have left review state/conversation ownership with maintainers.

[nepenth]

Current head 2842fd367 remains limited to rejected Matrix sync-cursor recovery. Full owning-suite verification: tests/gateway/test_matrix.py — 258 passed; the focused cursor-recovery selection remains 11 passed. Ruff, Python compilation, and git diff --check pass. All 36 current-head GitHub checks are complete with no failures. Ready for maintainer re-review.

[GottZ]

This was generated by AI during triage.

Summary

Two PRs address the same Matrix failure mode: a persisted incremental sync cursor rejected with 403/M_FORBIDDEN currently terminates the listener. Both distinguish rejected cursors from terminal authentication failures and retry from a fresh sync, while #61206 additionally bounds repeated resets and covers both exception and SyncError-like result-object paths.

Related pull requests

• #57725 [closed] duplicate — (+226/-17) — superseded: Implements rejected incremental-cursor clearing and retry while preserving terminal handling for 401, unknown-token, and fresh-sync forbidden failures, with exception-path regressions. Although closed, it remains relevant as the original implementation now superseded by the rebased and more complete #61206.
• #61206 related — (+379/-16) — preferred: Implements the same root-cause fix, adds a three-reset lifetime budget to prevent endless rejection cycles, and tests exception, textual-status, repeated-rejection, and SyncError-like result-object behavior. It also addresses the keep_open review on #61206 by adding the specifically requested stored-cursor recovery and no-cursor terminal result-object tests.

Duplicates

#57725 and #61206 are substantially duplicate implementations of rejected Matrix incremental-sync cursor recovery; #61206 is the expanded successor.

Suggested consolidation

Merge #61206: it contains the shared root-cause fix, preserves terminal auth behavior, bounds recovery attempts, and incorporates the contributor review's missing result-object coverage. Keep #57725 closed as superseded by #61206.

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 Dup57725 ["PRs duplicating each other"]
        P57725["PR #57725 (closed)"]
        P61206["PR #61206 (open)"]
    end
    class P57725 closed
    class P61206 open
    class P61206 target
    click P57725 "https://github.com/NousResearch/hermes-agent/pull/57725"
    click P61206 "https://github.com/NousResearch/hermes-agent/pull/61206"

Loading

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: 30 kB of PR diffs, 4 kB of issue/PR text, 3 kB of discussion (5 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[nepenth]

Rebased onto current main as a clean single-commit recut (46423b455f60).

Behavior unchanged in intent: rejected incremental Matrix sync cursors (403 / M_FORBIDDEN) now clear under a bounded reset budget and retry from a fresh sync for both raised exceptions and SyncError-like result objects. Permanent auth failures (401 / unknown token) and no-cursor forbidden results remain terminal.

Verification on this head: focused sync-cursor suite 7 passed; py_compile clean.

[nepenth]

Follow-up on the current head: successful Matrix /sync responses are dictionaries, so auth-like text inside a timeline event must not be interpreted as an auth-error object before normal sync dispatch. This adds regression coverage for embedded HTTP 401 Unauthorized and HTTP 403 Forbidden text and preserves the existing non-dict rejected-cursor recovery path.

Fresh verification: full tests.gateway.test_matrix module (121 passed), focused sync-loop selection (10 passed, 2 skipped), and git diff --check clean.

[nepenth]

Status — rebuilt on current main (66666f6e2eca)

Current head 7abeb4e15cdfcdd7a023cbe05de6c6a382ccbbf7.

Follow-up: successful sync dicts are never regex-scanned for HTTP 401/403 text; string-only M_UNKNOWN_TOKEN exceptions stay fatal; a malformed first status attribute no longer hides a later numeric status_code.

Focused local result: 11 passed (tests/gateway/test_matrix.py -k 'sync_loop or cursor').

[nepenth]

Updated on upstream main 966637323e6f90864e069dbc12755934c2c86387; current head 0e1749dee4656d72c5ed515f67e470076d936a42.

The sync recovery gap is still present on current main. This rebase keeps rejected-cursor reset, message-only M_FORBIDDEN handling as both exceptions and result objects, and departed-room dispatch exclusion. Optional SDK cases skip without the Matrix extra. Verified: Matrix 132 passed / 2 skipped, auth retry 4 passed.

The rewritten PR description is the current scope and verification reference. Local suite results are not a claim of full-repository or live-homeserver validation; current-head CI is tracked separately by the checks below.

[nepenth]

Rebased onto current main (36221c6d32).

This recut keeps main's structured sync-auth classifier (errcode + http_status; no HTML substring 403 scans). A persisted since token rejected as 403 / M_FORBIDDEN, or M_UNKNOWN_POS, is cleared and retried as a snapshot (bounded to three resets). 401 / M_UNKNOWN_TOKEN, and 403 with no cursor, still stop the loop. Snapshot recovery drops rooms that disappeared from the join set.

Verification: python -m pytest tests/gateway/test_matrix.py -q — 124 passed.

[nepenth]

Follow-up on review: reset budget is consecutive, not process lifetime. A successful snapshot with a new next_batch clears the counter so rare cursor rejections over a long process do not stop the loop.

python -m pytest tests/gateway/test_matrix.py -q — 124 passed.