fix(hook): key session_map under the session readers resolve against - #138
Open
paskal wants to merge 1 commit into
Open
fix(hook): key session_map under the session readers resolve against#138paskal wants to merge 1 commit into
paskal wants to merge 1 commit into
Conversation
A window linked into ccgram's tmux session belongs to more than one session,
and tmux reports whichever one owns the firing pane. The hook keyed
session_map from that, while every reader resolves entries by
session_map_prefix_for(mux, config.tmux_session_name), i.e. "ccgram:<window_id>".
So adopting a window that another tool created — `tmux link-window` being the
natural way to hand ccgram an existing agent — writes a binding no reader can
find. Nothing errors: the hook logs "Updated session_map", the topic binds, and
`ccgram status` reports "Monitored sessions: 0" forever with no transcript
relay. Concretely this is what happens with agent-deck, which names a tmux
session per agent.
Ask tmux for #{window_linked_sessions} in the display-message call the resolver
already makes, and only when the window really is linked, confirm which session
to key under. Unlinked windows keep the pane's own session and take no extra
subprocess, so the single-session path is unchanged.
An alternative worth considering is fixing this on the reader side instead:
window ids are server-global, so matching entries by their "@n" suffix rather
than a session-name prefix would cover any multi-session arrangement. This
patch is the smaller change; happy to rework it that way if you prefer.
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
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.
Follow-up to #135, independent of it — same underlying setup (driving agents that another tool launched), different bug.
Problem
_resolve_window_idkeyssession_mapfrom the session tmux reports for the firing pane:Readers resolve entries by
session_map_prefix_for(mux, config.tmux_session_name), i.e.ccgram:<window_id>(window_resolver.py:39-41,status_cmd.py:151-152,session_map.prune_session_map).Those agree only while the window's sole session is ccgram's. A window linked in with
tmux link-windowbelongs to several sessions, and tmux reports the one that owns the pane. The hook then writesagentdeck_foo_1234:@12while every reader looks upccgram:@12.Nothing fails loudly. The hook logs
Updated session_map, the topic binds and shows status, andccgram statusreportsMonitored sessions: 0with no transcript ever relayed. Adding to the confusion,prune_session_mapskips the mis-keyed entry precisely because it doesn't match the prefix, so it lingers.This matters because adopting foreign windows is supported on purpose — the
manual_discoveredorigin marker exists for it — andlink-windowis the natural way to hand ccgram an agent that something else started. In my case that's agent-deck, which names a tmux session per agent, so every adopted window is affected.Change
Ask tmux for
#{window_linked_sessions}in thedisplay-messagecall the resolver already makes, and only when the window is genuinely linked, confirm which session to key under. Unlinked windows keep the pane's own session and cost no extra subprocess, so the ordinary single-session path is untouched — which is also why the existingTestTabDelimitedParsingcases still pass unchanged.Alternative
Possibly better as a reader-side fix: tmux window ids are server-global, so resolving entries by their
@Nsuffix rather than a session-name prefix would sidestep the ambiguity for any multi-session arrangement, not just linked windows. I went with the smaller change here, but happy to rework it that way if you prefer.Verified
tests/ccgram/test_hook.pypasses (72), including five new cases covering linked, unlinked, already-ours, tmux-failure and timeout. Full suite is green apart fromtest_send_failure_replies_error, which fails identically on an unmodified checkout here.ruff checkandruff format --checkclean.On a real machine: before this, an adopted agent-deck window bound its topic and stayed silent at
Monitored sessions: 0; after it, the binding is found and transcripts relay.