Skip to content

fix(tui): guard /reload-mcp against null ctx.sid#17821

Open
0xDevNinja wants to merge 1 commit intoNousResearch:mainfrom
0xDevNinja:fix/17794-reload-mcp-null-sid-guard
Open

fix(tui): guard /reload-mcp against null ctx.sid#17821
0xDevNinja wants to merge 1 commit intoNousResearch:mainfrom
0xDevNinja:fix/17794-reload-mcp-null-sid-guard

Conversation

@0xDevNinja
Copy link
Copy Markdown
Contributor

@0xDevNinja 0xDevNinja commented Apr 30, 2026

What does this PR do?

/reload-mcp declared params.session_id: string but read it directly from ctx.sid, which is typed as string | null. Triggering the command before a session was active crashed the TUI. Mirrors the null-guard pattern that /rollback (and /save) already use — surface a sys message instead of hitting the RPC with a null id.

Related Issue

Fixes #17794

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • ui-tui/src/app/slash/commands/ops.ts — early-return with ctx.transcript.sys('/reload-mcp requires an active session') when ctx.sid is null, before constructing params.
  • ui-tui/src/__tests__/createSlashHandler.test.ts:
    • removed the table row that asserted reload.mcp is called with session_id: null (that was the bug — the typed string field was getting null).
    • added a positive test: active sid → RPC routed with the sid.
    • added a regression test: no sid → sys message, RPC not invoked.

How to Test

  1. cd ui-tui && npm install
  2. npm run build --prefix packages/hermes-ink
  3. npx vitest run createSlashHandler — 45 / 45 pass, including the two new cases.
  4. Full TUI suite (npx vitest run) — 539 / 539.
  5. npm run type-check — clean.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • Targeted vitest run + full TUI suite + tsc --noEmit pass locally
  • I've added tests for my changes
  • I've tested on my platform: macOS 15 (Darwin 23.2)

Documentation & Housekeeping

  • I've updated relevant documentation — N/A (no behavior change for users beyond the new sys message)
  • I've updated cli-config.yaml.example — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md — N/A
  • I've considered cross-platform impact — guard is platform-agnostic
  • I've updated tool descriptions/schemas — N/A

@0xDevNinja
fix(tui): guard /reload-mcp against null ctx.sid
64809e2 
`/reload-mcp` typed `params.session_id: string` but `ctx.sid` is
`string | null`, so triggering the command before a session was active
crashed the TUI. Apply the same null-guard pattern that `/rollback`
already uses: if `ctx.sid` is null, surface a sys message and skip the
RPC entirely.

Updated `createSlashHandler.test.ts`:
- removed the row that asserted `reload.mcp` is called with
  `session_id: null` (that was the bug)
- added a positive test (active session → RPC invoked with sid)
- added a regression test (no session → sys message, no RPC)

Fixes NousResearch#17794
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) duplicate This issue or pull request already exists labels Apr 30, 2026
@alt-glitch
Copy link
Copy Markdown
Collaborator

alt-glitch commented Apr 30, 2026

Likely duplicate of #17774 — same null-guard fix for /reload-mcp ctx.sid crash (#17794). PR #17822 also competes.

@alt-glitch alt-glitch mentioned this pull request Apr 30, 2026
Closed
@alt-glitch
Copy link
Copy Markdown
Collaborator

alt-glitch commented Apr 30, 2026

Duplicate of #17774 — same fix for #17773/#17794: null guard on ctx.sid in /reload-mcp.

@0xDevNinja
Copy link
Copy Markdown
Contributor Author

0xDevNinja commented Apr 30, 2026 •
edited
Loading

@alt-glitch Thanks for flagging — I checked all three.

  • fix(tui): allow nullable session_id in /reload-mcp params #17774 changes the type to session_id: string | null and forwards ctx.sid unchanged. That makes TypeScript happy but still sends null over the RPC, so the gateway receives a session-less reload request. The bug surface moves from the TUI to the server; it doesn't get fixed.
  • fix(tui): add null guard for ctx.sid in /reload-mcp slash command #17822 is the same null-guard approach as this PR but doesn't update createSlashHandler.test.ts — the existing table-driven row asserts reload.mcp is called with session_id: null, so that test will keep passing while the runtime no longer calls the RPC. (i.e. the test pins the buggy behaviour.)
  • This PR removes the buggy-row, adds a positive test (sid set → RPC routed) and a regression test (sid null → sys message, no RPC). 539 / 539 vitest + tsc clean.

Happy to defer to whichever the maintainer prefers — but #17774's approach doesn't match the typed session_id: string invariant the rest of the codebase relies on, and #17822 needs the test update to be safe to merge.

@Muno459 Muno459 mentioned this pull request May 4, 2026
Open
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TUI: /reload-mcp slash command crashes on null ctx.sid

2 participants

@0xDevNinja @alt-glitch