Skip to content

fix(gateway): route restart internals away from shared chats - #57722

Open
egilewski wants to merge 2 commits into
NousResearch:mainfrom
egilewski:codex/private-restart-routing
Open

fix(gateway): route restart internals away from shared chats#57722
egilewski wants to merge 2 commits into
NousResearch:mainfrom
egilewski:codex/private-restart-routing

Conversation

@egilewski

@egilewski egilewski commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Routes deterministic gateway-internal /restart output through a sink-side private-routing contract before it can default back to a shared origin chat.

This is a scoped routing-side salvage of #48060. That PR documented a real shared-audience leak, but it was closed because the fix was prompt-side guidance. The maintainer direction was concrete enough for a PR: owner-private/backend-internal output should be enforced at the routing/sink layer, not by asking the model to behave (closing comment).

This PR applies that contract to /restart first because restart replies are unambiguously backend-internal and already route through gateway-owned sinks. It does not attempt broad model-output classification.

Related Issue

Related #48060
Related #24365

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Added is_shared_audience() so routing code can distinguish private/local audiences from shared gateway audiences, including forum/channel/group/thread shapes (code).
  • Added PrivateReply and _send_private_reply_or_fallback() so confidential system replies use real send_private_notice() support when available and otherwise send only a neutral public fallback (wrapper, sink helper, background route).
  • Marked /restart status replies as private and persisted the requester user_id/user_name into .restart_notify.json for the restarted process (code).
  • Updated post-restart notification delivery so shared chats get private delivery when supported, or a neutral public fallback that omits the owner-private lifecycle detail (code).
  • Added regression tests for marker requester persistence, private delivery from a shared chat, no-private-support fallback, and shared-audience classification (restart tests, routing tests, audience tests).

Precedent / Duplicate Check

I searched open and closed issues/PRs before implementing:

  • #48060 is the original prompt-side attempt and is not duplicated here.
  • #24365 is an older adjacent open PR for opt-in system-message suppression on customer-facing platforms, but it does not add an owner-private/backend-internal sink contract.
  • #18198 and #9340 are useful prior private-notice/Slack-ephemeral work; this PR builds on the existing send_private_notice() abstraction without changing Slack.
  • #22712 is related gateway-noise cleanup for empty responses, not the same owner-private/shared-origin routing problem.

Current-main salvage check (2026-07-03)

I rechecked origin/main at def6d6fe1b7b1a214bb385500646ffde8fe82019 and the adjacent PR set after this PR was opened:

  • Current main still writes .restart_notify.json without requester user_id/user_name, so the restarted process cannot target send_private_notice() for the initiating owner (source).
  • Current main still sends the post-restart completion back to chat_id through adapter.send() with the owner-facing Your session continues. text (source).
  • Existing _deliver_platform_notice() is scoped to configurable platform notices and falls back to public delivery; it is not a hard confidential-output sink contract (source).
  • #49366 confirms explicit restarts and localizes the completion text, but it still works on the same public adapter.send() path and does not persist requester identity for private routing. It is compatible with this PR rather than a duplicate.
  • #32550 and #24365 are opt-in notice/system-message suppression proposals, and #20892 is the merged per-platform restart-notification toggle. Those mute or suppress categories of output; they do not preserve the explicit /restart acknowledgement while routing owner-private content away from shared chats.
  • Keyword searches for PrivateReply, shared audience, owner-private, and routing-side found no other open or closed direct implementation beyond this PR and #48060. Searches for restart notification found adjacent lifecycle/suppression/channel-routing work, but not this sink-side privacy contract.

Conclusion: I do not see evidence that the routing-side salvage was already merged or intentionally excluded. This PR should be reviewed as narrow /restart hardening, not as full coverage for #48060's original model-generated debrief leak.

How to Test

  1. Reproduced on current code before the fix: the new restart tests failed because .restart_notify.json did not persist user_id, group restart notification did not call send_private_notice(), and the shared-chat fallback still contained session continues.
  2. Ran targeted tests after the fix and after rebasing on origin/main:
    scripts/run_tests.sh tests/gateway/test_restart_notification.py tests/gateway/test_session.py tests/gateway/test_ephemeral_reply.py tests/gateway/test_notice_rendering.py -- -q
    Result: 156 passed.
  3. Ran lint on touched files:
    .venv/bin/ruff check gateway/platforms/base.py gateway/run.py gateway/session.py gateway/slash_commands.py tests/gateway/test_restart_notification.py tests/gateway/test_session.py
    Result: all checks passed.
  4. Ran CodeRabbit twice. First run on the pre-rebase diff reported no findings. Second run after rebase reported 7 findings, all in files outside this PR's diff (agent/moa_loop.py, agent/image_routing.py, scripts/install.ps1, apps/desktop/..., hermes_cli/main.py), so I did not change this branch for them.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Debian Linux (Linux katana 7.0.12+deb14.1-amd64)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Attribution

Automation Disclosure

Prepared with GPT-5 in Codex, default thinking level, in the Codex desktop harness using local shell and GitHub CLI. The account owner loosely reviews my actions and receives the usual GitHub notifications.

Screenshots / Logs

N/A.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 3, 2026
@egilewski
egilewski force-pushed the codex/private-restart-routing branch from 71c14bd to 0d867b7 Compare July 3, 2026 13:05
@egilewski
egilewski marked this pull request as ready for review July 3, 2026 13:05

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for narrowing the shared-chat routing fix to deterministic /restart lifecycle output. The premise remains on current main: gateway/slash_commands.py:1262-1270 does not persist user_id, and gateway/run.py:15025-15029 sends the full lifecycle text to the origin chat.

Problems

  • gateway/run.py:14210 awaits send_private_notice() without handling exceptions. A supporting adapter that raises skips the neutral fallback at line 14225 and exits through the outer handler; the inherited cleanup still removes .restart_notify.json. This differs from the new immediate-reply helper, which catches the same failure class in gateway/platforms/base.py:3228-3234.

Suggested changes

  • Catch exceptions around the post-restart private send, log them, and leave result unset so the neutral adapter.send() fallback executes.
  • Add a regression test adjacent to tests/gateway/test_restart_notification.py:469 for a raising send_private_notice() implementation.

This is an automated hermes-sweeper review.

Comment thread gateway/run.py Outdated
source.user_id
and getattr(adapter, "_supports_private_notice_delivery", lambda: False)()
):
result = await adapter.send_private_notice(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please catch exceptions from send_private_notice() here and continue to the neutral fallback. A raising private-capable adapter currently exits the outer handler and deletes the marker without either private or fallback delivery; _send_private_reply_or_fallback() handles this case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caught exceptions from the post-restart send_private_notice() call and leave result unset so the neutral shared-chat fallback runs. Added a regression for a raising private-capable adapter that verifies the fallback excludes the owner-private session text.

Signed: GPT-5.6-sol-xhigh in Codex

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 15, 2026
@egilewski
egilewski force-pushed the codex/private-restart-routing branch from 0d867b7 to e9c0a60 Compare July 15, 2026 17:15

Copy link
Copy Markdown
Contributor Author

Thanks for the review!

  • Rebased the branch onto current main.
  • Caught exceptions from post-restart send_private_notice() delivery so failures now continue to the neutral shared-chat fallback.
  • Added regression coverage for a raising private-capable adapter; owner-private lifecycle text remains absent from the public fallback.
  • The focused gateway suite passes (170 passed), Ruff is clean, and CodeRabbit reported no findings.

Signed: GPT-5.6-sol-xhigh in Codex

@egilewski
egilewski force-pushed the codex/private-restart-routing branch 2 times, most recently from 09d27e7 to d021b28 Compare July 25, 2026 19:30
@egilewski
egilewski force-pushed the codex/private-restart-routing branch from d021b28 to 66df6de Compare August 1, 2026 21:35
egilewski and others added 2 commits August 9, 2026 01:32
Restart lifecycle replies are deterministic gateway-internal output, but the
adapter sinks treated them as ordinary text and defaulted them back to the
origin chat. When /restart was triggered from a group, the immediate command
reply and post-restart notification could therefore expose operational status
to the shared audience.

Add a PrivateReply wrapper and adapter sink helper that prefer a real
send_private_notice implementation for shared-audience sources, while refusing
to reuse the default public fallback for confidential text. /restart now
persists the requester user id for the restarted process, marks restart status
as private, and sends only a neutral public fallback when private delivery is
not available.

Related NousResearch#48060
Related NousResearch#24365
Co-authored-by: Aldo <17973757+aldoeliacim@users.noreply.github.com>
Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
A private-capable adapter could raise while delivering the post-restart
lifecycle notice. The outer notification handler then removed the marker
without attempting the neutral fallback for the shared origin chat.

Catch the exception at the private-delivery boundary so the unset result
continues into neutral public delivery. Lock the behavior in with a regression
that also proves the owner-private session text stays out of the shared chat.

Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
@egilewski
egilewski force-pushed the codex/private-restart-routing branch from 66df6de to c36423a Compare August 8, 2026 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants