feat(session): add max_duration_minutes hard TTL for session resets (#22081) - #2
Closed
Phoenix1819 wants to merge 1 commit into
Closed
feat(session): add max_duration_minutes hard TTL for session resets (#22081)#2Phoenix1819 wants to merge 1 commit into
Phoenix1819 wants to merge 1 commit into
Conversation
🚨 CRITICAL Supply Chain Risk DetectedThis PR contains a pattern that has been used in real supply chain attacks. A maintainer must review the flagged code carefully before merging. 🚨 CRITICAL: Install-hook file added or modifiedThese files can execute code during package installation or interpreter startup. Files: Scanner only fires on high-signal indicators: .pth files, base64+exec/eval combos, subprocess with encoded commands, or install-hook files. Low-signal warnings were removed intentionally — if you're seeing this comment, the finding is worth inspecting. |
🔎 Lint report:
|
| Rule | Count |
|---|---|
invalid-argument-type |
53 |
unresolved-import |
34 |
invalid-method-override |
26 |
unresolved-attribute |
23 |
invalid-assignment |
7 |
unsupported-operator |
2 |
unresolved-reference |
1 |
unused-type-ignore-comment |
1 |
invalid-type-form |
1 |
invalid-return-type |
1 |
not-subscriptable |
1 |
First entries
utils.py:203: [unresolved-import] unresolved-import: Cannot resolve imported module `ruamel.yaml`
tests/tools/test_file_tools.py:379: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `Overload[(key: SupportsIndex | slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> LiteralString, (key: SupportsIndex | slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> str]` cannot be called with key of type `Literal["old_string"]` on object of type `str`
gateway/platforms/bluebubbles.py:563: [invalid-method-override] invalid-method-override: Invalid override of method `send_document`: Definition is incompatible with `BasePlatformAdapter.send_document`
tests/gateway/test_telegram_thread_fallback.py:905: [invalid-argument-type] invalid-argument-type: Argument to `BasePlatformAdapter.__init__` is incorrect: Expected `Platform`, found `None`
acp_adapter/session.py:624: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `IterationBudget`, found `str | list[str] | bool | Unknown`
plugins/platforms/google_chat/adapter.py:2521: [invalid-method-override] invalid-method-override: Invalid override of method `send_voice`: Definition is incompatible with `BasePlatformAdapter.send_voice`
tests/skills/test_fetch_transcript.py:12: [unresolved-import] unresolved-import: Cannot resolve imported module `fetch_transcript`
hermes_cli/uninstall.py:324: [unresolved-attribute] unresolved-attribute: Module `winreg` has no member `HKEY_CURRENT_USER`
run_agent.py:4236: [invalid-argument-type] invalid-argument-type: Argument to bound method `SessionDB.append_message` is incorrect: Expected `str`, found `Unknown | None`
tests/tools/test_delegate.py:2057: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `Overload[(key: SupportsIndex | slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> LiteralString, (key: SupportsIndex | slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> str]` cannot be called with key of type `Literal["acp_command"]` on object of type `str`
tests/hermes_cli/test_apply_profile_override.py:18: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
hermes_cli/uninstall.py:328: [unresolved-attribute] unresolved-attribute: Module `winreg` has no member `QueryValueEx`
tests/tools/test_process_registry.py:831: [unresolved-import] unresolved-import: Cannot resolve imported module `psutil`
tests/tools/test_session_search.py:327: [unresolved-attribute] unresolved-attribute: Unresolved attribute `SessionDB` on type `ModuleType`
hermes_cli/uninstall.py:332: [unresolved-attribute] unresolved-attribute: Module `winreg` has no member `DeleteValue`
run_agent.py:12504: [invalid-argument-type] invalid-argument-type: Argument to bound method `SessionDB.update_token_counts` is incorrect: Expected `str`, found `str | Unknown | dict[Unknown | str, Unknown | str | dict[str, str]] | int | dict[Unknown, Unknown]`
acp_adapter/session.py:624: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `bool`, found `str | list[str] | bool | Unknown`
gateway/platforms/base.py:2831: [invalid-assignment] invalid-assignment: Invalid subscript assignment with key of type `Literal["stop_event"]` and value of type `Event` on object of type `dict[str, dict[Unknown, Unknown] | None]`
gateway/platforms/email.py:80: [invalid-assignment] invalid-assignment: Object of type `Literal["0"]` is not assignable to `Literal["0.13.0"]`
tests/tools/test_delegate.py:2057: [invalid-argument-type] invalid-argument-type: Method `__getitem__` of type `Overload[(i: SupportsIndex, /) -> Unknown, (s: slice[SupportsIndex | None, SupportsIndex | None, SupportsIndex | None], /) -> list[Unknown]]` cannot be called with key of type `Literal["acp_command"]` on object of type `list[Unknown]`
gateway/platforms/wecom.py:1438: [invalid-method-override] invalid-method-override: Invalid override of method `send_voice`: Definition is incompatible with `BasePlatformAdapter.send_voice`
tests/hermes_cli/test_destructive_slash_confirm_gate.py:32: [unresolved-attribute] unresolved-attribute: Attribute `get` is not defined on `str`, `list[Unknown]`, `list[str]`, `int` in union `str | dict[Unknown, Unknown] | list[Unknown] | ... omitted 25 union elements`
gateway/platforms/signal.py:1299: [invalid-method-override] invalid-method-override: Invalid override of method `send_image_file`: Definition is incompatible with `BasePlatformAdapter.send_image_file`
tools/session_search_tool.py:375: [invalid-argument-type] invalid-argument-type: Argument to bound method `SessionDB.search_messages` is incorrect: Expected `list[str]`, found `None | list[str]`
hermes_cli/gateway.py:3144: [unresolved-attribute] unresolved-attribute: Module `ctypes` has no member `windll`
... and 125 more
✅ Fixed issues (42):
| Rule | Count |
|---|---|
invalid-argument-type |
15 |
unresolved-attribute |
12 |
unresolved-import |
6 |
invalid-assignment |
3 |
no-matching-overload |
2 |
unresolved-reference |
2 |
unsupported-operator |
1 |
call-non-callable |
1 |
First entries
plugins/platforms/google_chat/adapter.py:2942: [unresolved-import] unresolved-import: Module `hermes_cli.config` has no member `prompt`
acp_adapter/session.py:623: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `list[str]`, found `str | list[str] | bool`
plugins/platforms/google_chat/adapter.py:526: [unresolved-attribute] unresolved-attribute: Attribute `Credentials` is not defined on `None` in union `Unknown | None`
tests/run_agent/test_concurrent_interrupt.py:192: [unsupported-operator] unsupported-operator: Operator `+=` is not supported between objects of type `None` and `Literal[1]`
hermes_cli/main.py:5350: [unresolved-import] unresolved-import: Cannot resolve imported module `openai`
run_agent.py:6811: [invalid-argument-type] invalid-argument-type: Argument to function `build_anthropic_client` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`
acp_adapter/session.py:623: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `int | float`, found `str | list[str] | bool`
gateway/platforms/telegram.py:1258: [invalid-argument-type] invalid-argument-type: Argument to constructor `int.__new__` is incorrect: Expected `str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc`, found `str | None`
plugins/platforms/google_chat/adapter.py:903: [unresolved-attribute] unresolved-attribute: Attribute `PermissionDenied` is not defined on `None` in union `Unknown | None`
tui_gateway/entry.py:84: [no-matching-overload] no-matching-overload: No overload of bound method `dict.get` matches arguments
plugins/platforms/google_chat/adapter.py:2944: [unresolved-import] unresolved-import: Module `hermes_cli.config` has no member `print_info`
plugins/platforms/google_chat/adapter.py:782: [unresolved-attribute] unresolved-attribute: Attribute `SubscriberClient` is not defined on `None` in union `Unknown | None`
tests/gateway/test_tts_media_routing.py:106: [unresolved-attribute] unresolved-attribute: Object of type `bound method _MediaRoutingAdapter.send_document(chat_id: str, file_path: str, caption: str | None = None, file_name: str | None = None, reply_to: str | None = None, **kwargs) -> CoroutineType[Any, Any, SendResult]` has no attribute `assert_not_awaited`
gateway/platforms/base.py:2793: [invalid-assignment] invalid-assignment: Invalid subscript assignment with key of type `Literal["stop_event"]` and value of type `Event` on object of type `dict[str, dict[str, str] | None]`
gateway/platforms/base.py:2797: [invalid-argument-type] invalid-argument-type: Argument to bound method `BasePlatformAdapter._keep_typing` is incorrect: Expected `int | float`, found `dict[str, str] | None`
run_agent.py:12805: [invalid-argument-type] invalid-argument-type: Argument to function `_is_oauth_token` is incorrect: Expected `str`, found `str | dict[Unknown, Unknown] | Any | ... omitted 3 union elements`
run_agent.py:12808: [invalid-argument-type] invalid-argument-type: Argument to function `len` is incorrect: Expected `Sized`, found `(str & ~AlwaysFalsy) | (dict[Unknown, Unknown] & ~AlwaysFalsy) | (Any & ~AlwaysFalsy) | ... omitted 3 union elements`
tests/gateway/test_tts_media_routing.py:96: [invalid-assignment] invalid-assignment: Object of type `AsyncMock` is not assignable to attribute `send_voice` of type `def send_voice(self, chat_id: str, audio_path: str, caption: str | None = None, reply_to: str | None = None, **kwargs) -> CoroutineType[Any, Any, SendResult]`
acp_adapter/session.py:609: [no-matching-overload] no-matching-overload: No overload of bound method `MutableMapping.update` matches arguments
plugins/platforms/google_chat/adapter.py:2946: [unresolved-import] unresolved-import: Module `hermes_cli.config` has no member `print_warning`
tests/gateway/test_tts_media_routing.py:81: [unresolved-attribute] unresolved-attribute: Object of type `bound method _MediaRoutingAdapter.send_document(chat_id: str, file_path: str, caption: str | None = None, file_name: str | None = None, reply_to: str | None = None, **kwargs) -> CoroutineType[Any, Any, SendResult]` has no attribute `assert_awaited_once_with`
plugins/platforms/google_chat/adapter.py:2943: [unresolved-import] unresolved-import: Module `hermes_cli.config` has no member `prompt_yes_no`
plugins/platforms/google_chat/adapter.py:789: [unresolved-attribute] unresolved-attribute: Attribute `NotFound` is not defined on `None` in union `Unknown | None`
acp_adapter/session.py:623: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `list[str] | None`, found `str | list[str] | bool`
acp_adapter/session.py:623: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `dict[str, Any]`, found `str | list[str] | bool`
... and 17 more
Unchanged: 4053 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
Owner
Author
|
Note on design intent: during review, you might wonder why mode="none" does not skip the max_duration check entirely. That is intentional — max_duration is a hard ceiling independent of mode. If an operator sets mode="none" but also caps max_duration at 8 hours, the cap should still apply. The test test_returns_max_duration_when_hard_ttl_expired documents this behaviour. |
Phoenix1819
pushed a commit
that referenced
this pull request
May 12, 2026
Adds a TestCheckSendMessage class with 7 focused tests pinning the
four passing conditions and the failure modes:
- HERMES_KANBAN_TASK grants access (the new branch)
- HERMES_KANBAN_TASK short-circuits before consulting
session_context or gateway.status (so workers don't depend on
those import paths being healthy)
- HERMES_SESSION_PLATFORM=telegram grants access
- HERMES_SESSION_PLATFORM=local falls through to gateway check
- is_gateway_running()=True grants access
- All signals absent → False
- gateway.status ImportError is swallowed → False
Pinning the short-circuit (test #2) is the load-bearing one — it
documents the contract that worker-side availability cannot regress
to depending on gateway-side state lookups.
Phoenix1819
pushed a commit
that referenced
this pull request
May 14, 2026
…rch#25071) * tui: make URLs clickable + hover-highlight in any terminal Problem ------- URLs printed by `hermes --tui` were not clickable in basic macOS Terminal.app. Cmd+click did nothing, the cursor didn't change shape — like nothing was detected — even though arrow buttons and other Box onClick handlers worked fine. Root cause ---------- Two layers of dead plumbing: 1. `<Link>` only emitted the underlying `<ink-link>` (which carries the hyperlink metadata into the screen buffer) when `supportsHyperlinks()` said yes. On Apple_Terminal that's false, so the per-cell hyperlink field stayed empty, so `Ink.getHyperlinkAt()` had nothing to return on click. The visible underline was just decorative. 2. `Ink.openHyperlink()` calls `this.onHyperlinkClick?.(url)`, but `onHyperlinkClick` was never assigned anywhere in the codebase. The click pipeline (`App.tsx → onOpenHyperlink → Ink.openHyperlink`) ran but bailed silently on the optional chain. Bonus discovery: even when wired up, there was no hover affordance — terminal apps can't change the system mouse cursor, so users had no visual signal that a cell was clickable. Arrow buttons in the chrome worked because they had explicit `<Box onClick>` styling; inline link URLs didn't. Fix --- - `Link.tsx`: always emit `<ink-link>` regardless of terminal capability. The renderer's `wrapWithOsc8Link` already gates the actual OSC 8 escape on `supportsHyperlinks()` further down — so terminals that don't understand OSC 8 still don't see the escape, but the screen-buffer metadata (which the click dispatcher reads) is now populated everywhere. - `ink.tsx + root.ts`: add `onHyperlinkClick?: (url: string) => void` to `Options` / `RenderOptions`, wire it to the existing `Ink.onHyperlinkClick` field in the constructor. - `src/lib/openExternalUrl.ts`: small platform-aware opener using `child_process.spawn` with arg-array (no shell) — http(s) only, rejects `file:`, `javascript:`, `data:`, etc., so a hostile model can't trigger arbitrary local handlers via `<Link url="file:///...">`. Detached + stdio ignore so closing the TUI doesn't kill the browser and Chrome stderr doesn't leak into the alt screen. - `entry.tsx`: pass `onHyperlinkClick: openExternalUrl` to `ink.render`. - `hyperlinkHover.ts` + Ink hover wiring: track the URL under the pointer in `Ink.hoveredHyperlink`, update it from `dispatchHover`, and inverse- highlight every cell of the matching link in the render-pass overlay (same pattern as `applySearchHighlight`). This is the cursor-hover affordance for clickable links — terminals don't expose cursor shape, so we light up the link itself. - `types/hermes-ink.d.ts`: add `onHyperlinkClick` to the `RenderOptions` shim so consumers (`entry.tsx`) type-check against the new option. Tests ----- - `src/lib/openExternalUrl.test.ts` (15 cases): http(s) accepted; file/js/ data/mailto/ftp/ssh rejected; macOS open(1), Windows cmd.exe start with empty title slot, Linux xdg-open dispatch; shell-metacharacter URLs pass through unmolested as a single argv element; synchronous spawn failure returns false. Verified empirically in Apple Terminal 455.1 (macOS 15.7.3): clicking a URL opens in default browser, hovering inverts the link cells, and moving away clears the highlight. Full TUI suite: 713 passing, 0 type errors. Reverts ------- The earlier attempt that version-gated Apple_Terminal in `supports-hyperlinks.ts` was based on a wrong assumption — Terminal.app silently strips OSC 8 sequences but does not render them as clickable hyperlinks. Reverted to the original allowlist. * tui: address Copilot review — explorer.exe on win32 + comment fixes - openExternalUrl: switch win32 from `cmd.exe /c start` to `explorer.exe`. cmd.exe's `start` builtin reparses the URL through cmd's tokenizer, so `&`, `|`, `^`, `<`, `>` either split the command or get reinterpreted — breaking both the protocol-allowlist safety story AND plain http(s) URLs with `&` in query strings. `explorer.exe <url>` invokes the registered protocol handler directly with no shell. - openExternalUrl.test.ts: rename the win32 test to reflect the new contract and add two regression tests — one with `&|^<>` metachars, one with the common analytics-URL `&` query-param pattern — both pinned to single-argv-element delivery via explorer.exe. - Link.tsx: fix misleading comment. OSC 8 escapes are emitted unconditionally by the renderer (`wrapWithOsc8Link` in render-node-to-output.ts, `oscLink` in log-update.ts). Non-supporting terminals silently strip the sequence, which is why hover/click affordance has to come from the in-process overlay rather than the terminal's own link rendering. Verified: 715/715 tests pass, type-check + build clean. * tui: address Copilot review #2 — async spawn errors + hover scope + docs 1. openExternalUrl: attach a no-op `'error'` listener on the spawned child BEFORE unref(). spawn() returns a ChildProcess synchronously even when the binary is missing (ENOENT on xdg-open / explorer.exe), unreachable, or otherwise unusable; the failure surfaces later as an 'error' event. An unhandled 'error' on an EventEmitter crashes Node, which would tear down the whole TUI. The listener is a deliberate no-op — we already returned `true` synchronously and the user just doesn't see the browser pop. 2. openExternalUrl.test.ts: add a regression test using a real EventEmitter to simulate the async-error path. Pins both the listener-attached contract and the "doesn't throw on emit" behavior. Was 17/17, now 18/18. 3. ink.tsx dispatchHover: bypass `getHyperlinkAt()` and read `cellAt(...).hyperlink` directly. `getHyperlinkAt` falls back to `findPlainTextUrlAt` for cells without an OSC 8 hyperlink, but the render-pass overlay (`applyHyperlinkHoverHighlight`) only matches on `cell.hyperlink === hoveredUrl` — so plain-text URLs would burn re-renders without ever producing the highlight. Hover is now a strictly 1:1 fit for what the overlay can paint. Plain-text URLs still get the click action via the existing dispatch path. 4. root.ts + ink.tsx doc comments: replace the misleading "typically `open` / `xdg-open` / `start` shell" wording with the actual safe recipe — argv-array spawn into `open` / `xdg-open` / `explorer.exe`, with an explicit warning that `cmd.exe /c start` reparses the URL through cmd's tokenizer and is unsafe + breaks `&`-query URLs. Verified: 716/716 tests pass, type-check + build clean. * tui: address Copilot review #3 — hover damage, alt-screen cleanup, opener allowlist 1. ink.tsx onRender: stop folding steady-state hover into hlActive. hlActive forces a full-screen damage diff so previous-frame inverted cells get re-emitted when the highlight set changes. The transition IS the trigger — enter / leave / change-to-other-link. While the pointer just sits on a link the painted cells don't change and the per-cell diff handles the no-op. Folding the steady state in would burn a full-screen diff on every frame. Added a lastRenderedHoveredHyperlink tracker and gate the hlActive bump on `hovered !== lastRendered`. 2. ink.tsx setAltScreenActive: clear hoveredHyperlink (and the tracker) when toggling alt-screen state. Hover dispatch is alt-screen-gated, so once we leave there's no path to clear it. Without this, remounting <AlternateScreen> would paint a phantom hover from the previous session until the next mouse-move arrived. 3. openExternalUrl.ts openCommand: allowlist linux + the BSD family for xdg-open and return null for everything else (aix, sunos, cygwin, haiku, etc.). Previously the default-fallback always returned xdg-open, which made the caller's `if (!command) return false` dead and yielded a misleading `true` on platforms that probably don't have xdg-open. New tests cover the null path AND the openExternalUrl-returns-false-without-spawning behavior. Verified: 718/718 tests pass, type-check + build clean. * tui: address Copilot review #4 — doc comment accuracy 1. openExternalUrl return-value doc: now lists all three false paths (URL rejected / no opener for platform / synchronous spawn throw) plus a note that async 'error' events still return true because the spawn was attempted. 2. ink.tsx onHyperlinkClick field doc: clarifies the callback receives either an OSC 8 hyperlink OR a plain-text URL detected by findPlainTextUrlAt — App.tsx routes both into the same callback. 3. hyperlinkHover applyHyperlinkHoverHighlight doc: drops the misleading 'caller forces full-frame damage' promise. Caller decides; for hover the current caller only forces full damage on transitions. No behavior change. 718/718 tests pass. * tui: address Copilot review #5 — lint fixes 1. ink.tsx: reorder `./hyperlinkHover.js` import before `./screen.js` to satisfy perfectionist/sort-imports. 2. Link.tsx: drop unused `fallback` parameter destructuring + the trailing `void (null as ...)` dead-statement (would trip no-unused-expressions). Kept `fallback?: ReactNode` on the Props interface as a documented compat shim so existing call sites still compile, with a comment explaining why it's no longer wired up. 3. openExternalUrl.test.ts: replace `typeof import('node:child_process').spawn` inline annotations (forbidden by @typescript-eslint/consistent-type-imports) with a `SpawnLike` type alias backed by a real `import type { spawn as SpawnFn }`. No behavior change. 718/718 tests pass, type-check clean, lint clean on all modified files.
Owner
Author
|
Cleaning up fork |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a configurable hard TTL (max_duration_minutes) to gateway sessions. When exceeded, the session is reset with a notification to the user.