A session can rename itself through the live Collins tool socket

[ghackett]

PR 3 of the session MCP tools plan (after #201's ancestry walk and #204's shim + protocol layer): the app-side socket service goes live, launched commands carry --mcp-config, and the first tool — set_session_title — works end to end through a real claude CLI.

What's here

• collins/mcpserver.py (new): SessionToolService, a Gio.SocketService on the per-instance Unix socket. Fully async on the GLib main loop — no threads; per connection the cycle is strictly read → reply → read, so a peer that floods requests without reading replies stalls only itself. Takes injected list_tools() / dispatch(pid, tool, args) callables, so the whole connection machinery is CI-testable without GTK. Untrusted-peer rules: a first frame that isn't a well-formed hello, or any framing violation, disconnects rather than guesses.
  • One hazard found live: Gio silently truncates a socket path past sun_path's 107 bytes, producing a listener no shim can ever dial (the shim degrades cleanly, so the feature would just silently not work). start() now refuses over-long paths loudly; real paths under $XDG_RUNTIME_DIR sit far below the limit.
• collins/providers.py: module-level MCP_CONFIG_PATH (set by app.py only once the whole chain is up) plus a supports_mcp_config capability flag mirroring supports_fork. New/resume/fork/continue commands and the chat argv all carry the flag; the attach branch carries nothing by construction (claude attach accepts no flags and joins a process that already has its servers) — tested.
• collins/app.py: service bring-up in do_startup (one failure path: log, leave MCP_CONFIG_PATH unset, commands go out exactly as before — the tools are conveniences, never load-bearing), the codebase's first do_shutdown to tear it down, and the dispatcher: /proc ancestry walk from the shim's hello pid to the tab whose shell spawned its claude, argument re-validation app-side (the socket is reachable by any local process), and the set_session_title handler.
• collins/window.py: MainWindow.rename_session_tab() — the rename dialog's save path without the dialog. The rename lands in the manual-name slot, so it permanently stops auto-titling for that session, exactly as a hand rename does.
• collins/terminal.py: TerminalTab.owns_pid_ancestors() over _candidate_pids() (both the pty's foreground pgrp leader and the spawned child, per the daemon-wrapper note there).
• Tests: 24 for the service (raw socket clients against a live GLib loop, hostile-peer cases, lifecycle) including two true end-to-end tests driving the real mcp_shim subprocess against the real service — the seam neither module's own tests crossed — plus 7 new provider tests for the flag.

Verified live (headless, CLI 2.1.226)

• A real claude -p --mcp-config … session listed the tool, called it through --allowedTools, and echoed the service's "Session renamed." reply; dispatch received the shim's pid.
• A throwaway app instance (fresh COLLINS_APP_ID, isolated data): service up, runtime dir keyed by app id, typed resume command carries the flag, a client fed into the tab's own terminal renamed the session through the socket (screenshots below), and a client not descended from any tab got the clean identity error — the expected shape for daemon-hosted bg jobs until the identity fallback lands.
• PR 2's deferred question answered: the CLI merges an stdio server's env block into the inherited environment (probe saw both the config var and a parent-env marker). Either behavior works for the shim, but now it's recorded.

Screenshots

Before: the tab's typed command carries --mcp-config. After: a client run from the tab's own shell called set_session_title — tab title and sidebar row both renamed, ok reply visible in the terminal.

Before	After

Per the spec, v1 pre-allows nothing — every first use goes through the CLI's own permission prompt. Next up: PR 4 (open_in_editor + show_image), PR 5 (notify_user).

🤖 Generated with Claude Code

https://claude.ai/code/session_019ScBAgKFEdAAeQmcYGoGCA

[ghackett]

@claude review

[claude]

Claude finished @ghackett's task in 4m 55s —— View job

---

Review complete

• Gather context (diff, changed files, related modules)
• Review collins/mcpserver.py (new socket service)
• Review collins/providers.py (mcp-config flag)
• Review collins/app.py (bring-up/shutdown/dispatcher)
• Review collins/window.py / collins/terminal.py
• Review tests
• Post final review

Solid, well-tested PR — the framing, lifecycle, and capability-flag plumbing are careful and the 24+7 new tests cover the protocol edges (bad hellos, oversize lines, concurrent connections, stop-mid-request, and the two real shim↔service end-to-end tests) well. One security gap worth fixing before more tools land on top of this identity mechanism, plus a couple of minor notes.

1. The shim's identity (pid) is self-reported and never verified against the real peer

collins/mcpserver.py:143-155 (_greet) takes pid straight from the client's hello frame — it only checks the type/range (isinstance(pid, int), not a bool, > 0), never that this is the pid of the process actually holding the other end of the socket. mcp_shim.py:132 sends os.getpid() in good faith, but nothing stops any other local process from opening the same socket and lying in that field.

That pid is then load-bearing for authorization: app.py's _mcp_tab_for_pid (app.py:855-871) walks /proc ancestry from it to decide which open Collins tab a call is allowed to act on, and _mcp_dispatch trusts that result completely. The module's own docstring already acknowledges "any local process of the user's can still connect" — but the mitigation described (disconnect on bad framing / a malformed hello) doesn't cover a well-formed hello with a spoofed pid. Concretely: any local process belonging to the same user can declare an arbitrary (real) pid belonging to another tab's claude process and be treated as if it were that tab's shim — bypassing the ownership check entirely, for set_session_title today and for open_in_editor / show_image / notify_user once PR4/PR5 land, which raises the stakes considerably.

Unix domain sockets give you a kernel-verified alternative for exactly this: Gio.SocketConnection.get_socket().get_credentials() returns the real peer pid/uid via SO_PEERCRED (Linux) rather than trusting the payload. Worth authenticating against that instead of (or in addition to) message["pid"].

Fix this →

2. No automated coverage for the dispatcher glue in app.py / window.py

_mcp_tab_for_pid, _mcp_dispatch (app.py:830-880), and MainWindow.rename_session_tab (window.py:3518-3529) have no unit tests — only the PR description's manual/live verification. The service and provider layers are thoroughly covered because they're GTK-free by design; this glue is GTK-bound so it's a harder ask, but the validation → ownership-resolution → error-message branching in _mcp_dispatch is exactly the kind of logic that's easy to regress silently (e.g. the unresolved-session_id and not-launched-from-a-tab error paths). Not a blocker, just a gap worth closing, possibly by factoring the pid→tab resolution into something fakeable without a real Gtk.Application.

Everything else checked out

• providers.py's _mcp_config_flag() gating (capability flag + MCP_CONFIG_PATH is not None) and the attach-branch exemption are correct and directly tested (test_attach_carries_no_mcp_config confirms ClaudeProvider.resume_command's attach path rebuilds its own command rather than reusing the super() result that already carries the flag).
• The sun_path 107-byte truncation guard in mcpserver.py:77-78 is a good catch — silent truncation would have been a nasty "feature quietly doesn't work" bug.
• stop() correctly drops live connections, not just the listener (regression-tested per the PR3/PR204 note in test_stop_closes_established_connections).
• do_shutdown ordering and the "best-effort, never load-bearing" failure handling in _start_mcp_service look right — a failed bring-up leaves MCP_CONFIG_PATH unset and launched commands unaffected.

Branch: session-mcp-pr3-socket-service

[ghackett]

Both review findings addressed in 5e7c015:

1. Spoofable hello pid → kernel-verified via SO_PEERCRED. _greet now checks the declared pid against connection.get_socket().get_credentials().get_unix_pid() and drops any mismatch. Unreadable credentials also fail the hello — this path is Linux-only already (the dispatcher walks /proc), so an unanswerable kernel means broken, not trustworthy. New tests: a client claiming pid 1 is disconnected, an honest os.getpid() hello is served, and the real-shim end-to-end tests exercise the check for free (the shim's hello carries its own pid).

2. Dispatcher glue coverage. The validate → identity → handler branching moved into GTK-free mcptools.run_tool_call(), so CI now pins the order (validation runs before identity resolution, so a bad call fails identically whoever makes it — no leaking whether a tab owns the caller through the error shape) and the shared error strings. app.py keeps only the widget-touching halves: the pid→tab walk and the set_session_title handler. Six new tests in test_mcptools.py.

Re-verified live after the changes: the headless throwaway-app probe still renames a real tab through the socket, and a non-tab client still gets the clean identity error. Full suite: 1240 passing.

🤖 Generated with Claude Code

https://claude.ai/code/session_019ScBAgKFEdAAeQmcYGoGCA