feat: authoritative session attribution via agent-shim (replaces scrollback guessing)#315
Open
mgabor3141 wants to merge 8 commits into
Open
feat: authoritative session attribution via agent-shim (replaces scrollback guessing)#315mgabor3141 wants to merge 8 commits into
mgabor3141 wants to merge 8 commits into
Conversation
Contributor
Try this PRcurl -sSfL https://gmux.app/install-pr.sh | sh -s -- 315Built from |
…shim Inject a tiny JS preload (hook.mjs) into node/bun agents so gmux learns the held conversation file from the agent's own fs writes, replacing post-hoc scrollback matching (ADR 0009). - cli/gmux/internal/agentshim: embeds hook.mjs, materializes it to a readable content-addressed cache path, builds append-safe NODE_OPTIONS=--import / BUN_OPTIONS=--preload env + GMUX_RUNNER_SOCK. - adapter.SessionShimmer capability gates injection to jsonl agents (pi opts in); shells never get NODE_OPTIONS. - ptyserver injects the preload in New() and serves POST /shim/event, recording the path via session.State.SetSessionFile, which emits a session_file event on /events (first-attribution + rebind). - Shim ignores reads, keys on writes, dedupes node's append->write re-entry, disarms descendants by stripping its env.
Wire the runner's session_file event (from the agent-shim preload) into the daemon as authoritative attribution that overrides scrollback: - Subscriptions.OnSessionFile dispatches the session_file SSE event with its sessionID; main.go wires it to FileMonitor.AttributeFromShim. - AttributeFromShim records filePath->sessionID in attributions + a parallel shimFiles map, derives title/slug/status immediately, and handles /resume rebind by dropping the session's prior file. - Scrollback demotion: shim-attributed files are excluded as candidates (already in attributions); shim-attributed sessions are excluded from the scrollback candidate pool so it can't second-guess them. - Cleanup: session death and stale-clear drop shimFiles entries; NotifyNewSession processes any attribution that landed before registration (session_file only fires on change). Tests: authoritative override, rebind, death cleanup, scrollback exclusion, SSE dispatch.
Live testing surfaced a pre-write mis-attribution window: a freshly launched shimmed pi (no conversation yet) was attributed by scrollback to a stale old session file in the ~30s before its first write, and the stale attribution then lingered alongside the correct shim one. - agent-shim 'hello' now emits a 'shim' event (ptyserver -> session_file SSE); the daemon marks the session shim-covered (FileMonitor. MarkShimCovered) and suppresses scrollback for it. A covered session with no conversation correctly stays unattributed until it writes. - AttributeFromShim now clears ALL other files attributed to the session (stale scrollback guess or prior shim file on /resume), not just prior shim files — the shim is authoritative for the one file the agent holds. - Coverage cleared on session death. Verified live against real pi: fresh session shows only shim-attributed (no scrollback line); /resume produces a clean rebind; N:1 (two runners on one file) degrades to last-binder-wins with no conflict.
Make attribution survive a daemon restart without persisted state: the runner replays its current shim coverage and held session file to every new /events subscriber, so a reconnecting/restarted daemon re-learns attribution authoritatively the instant it resubscribes. - session.State retains shimActive (set on hello) and exposes ShimSnapshot(); emits are now lock-safe. - ptyserver handleEvents replays a 'shim' and 'session_file' event to each newly-connected subscriber before streaming live events. - attributions.json is retired: FileMonitor no longer loads or writes it (persistAttributionsLocked is now a no-op; NewFileMonitor starts empty). Shimmed sessions restore via re-announce; unshimmed sessions re-derive via the scrollback fallback. persist.go is kept (and still tested) but marked deprecated. Verified live: after a daemon restart with no attributions.json on disk, the held file is re-attributed immediately on resubscribe. Unit test TestShimReannounceOnReconnect covers the replay.
…persistence Tidy-up after retiring attributions.json and demoting scrollback: - Annotate the scrollback path as the deprecated FALLBACK: FileAttributor interface, the shared attribution helpers, tryAttributeUnmatched, and fetchScrollbackText. It now serves only sessions with no shim signal (unshimmed builds, injection failures, pre-hello runners). - Log successful scrollback attributions with a '(FALLBACK)' marker so we can monitor how often it actually fires in production. - Delete the now-dead attributions.json persistence: persist.go + persist_test.go removed; persistAttributionsLocked (no-op) and its call sites removed; ApplyPersistedAttributions and its tests removed; Watch's onFirstScan is passed nil. NewFileMonitorWithAttributions stays as a test-only seed seam. Full suite green across gmuxd, cli/gmux, and packages/adapter; go vet clean.
f0766e9 to
b6dee62
Compare
Document the shim-based attribution decision as ADR 0010 (the precursor 'attribution is the foundation' draft was never merged; 0009 is now the verb-first CLI ADR). Update UBIQUITOUS_LANGUAGE.md: the Attribution entry now reflects shim-primary / scrollback-fallback, and add an Agent-shim component term. (CHANGELOG.md only points to the external gmux.app/changelog; nothing to update locally.)
b6dee62 to
ea41874
Compare
Slim the wordy doc comments added with the agent-shim work so the point isn't buried (filemon AttributeFromShim/MarkShimCovered/tryAttributeUnmatched, the FileAttributor fallback note, ptyserver replay, agentshim PreloadEnv). Also correct the shim-coverage description: GMUX_RUNNER_SOCK is set by the runner (not the daemon), so re-announce works regardless of daemon timing; the fallback is only for agents the shim can't cover.
ea41874 to
ef557ee
Compare
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.
Problem
The gmux sidebar is conversation-keyed: a slot means "this is conversation X" so we can monitor X's session file and resume the right conversation if the runner dies. But gmux doesn't own the conversation identity — pi/claude/codex pick their session file after launch (only once there's an event), so gmux can't know it at spawn time.
Today gmux guesses by matching terminal scrollback against candidate session files. That guess is post-hoc, ambiguous for small/overlapping sessions, blind to
/resumerebinds, and assumes one runner per conversation. It's the root cause behind several sidebar bugs (sessions disappearing on relaunch, dismissed sessions reappearing, renamed sessions sorting to the bottom) — all attribution bugs in storage costume (see ADR 0009).Solution: authoritative attribution via an agent-shim
A tiny, readable JS preload (
hook.mjs) is injected into node/bun agent processes by the runner:NODE_OPTIONS="… --import file:///abs/hook.mjs"BUN_OPTIONS="… --preload /abs/hook.mjs"(both set, append-safe; each runtime honours its own)The shim wraps the agent's
fswrite surface and, whenever a*.jsonlsession file is written, POSTs{op,path,pid}to the runner's unix socket. The runner records it on session state and emits asession_fileevent; the daemon turns that into an authoritative attribution that overrides scrollback. The agent itself told us which file it holds — no guessing.Key design points:
/resumemakes the agentreaddir+ bulk-read every session file for its picker — pure noise. A real resume/rebind always ends in a write to the chosen file, so writes alone catch attribution and rebind.GMUX_RUNNER_SOCKis present, then strips it (and the injected*_OPTIONS) fromprocess.envso child processes the agent spawns inherit a clean env. Injection is gated on a newadapter.SessionShimmercapability (pi opts in); shells never getNODE_OPTIONS.hello(fired at startup, before any write) tells the daemon to suppress scrollback for that session until the real file is reported — closing the pre-write window where scrollback would otherwise mis-attribute a fresh session to a stale old file./eventssubscriber, so a restarted daemon re-learns attribution instantly on resubscribe — which let us retireattributions.jsonentirely (no persisted attribution state).Benefits
/resumerebind: a new file for the same runner drops the old attribution authoritatively.attributionsmap stays 1:1 — representing true N:1 is future work, but the signal is now authoritative per-runner).attributions.jsonpersistence; the same shim works for any JSONL agent (pi/claude/codex), so the per-adapter attribution surface collapses toward a thin path-recognizer.Fallback
Scrollback attribution is kept but demoted. It now serves only sessions with no shim signal: shells (which never used it), agent builds where the shim couldn't be injected, and runners that started before any daemon could hand them
GMUX_RUNNER_SOCK. The path is annotatedDeprecated/FALLBACK (FileAttributor, the shared helpers,tryAttributeUnmatched,fetchScrollbackText), and successful fallback attributions now log a(FALLBACK)marker so we can monitor how often we actually rely on it in production.Testing
/resumerebind, session-death cleanup, scrollback exclusion of shim-covered sessions, thesession_file/shimSSE dispatch, append-safe env injection, descendant disarm, and an end-to-endptyservertest driving a realnodeagent (TestShimReportsSessionFile,TestShimReannounceOnReconnect).piunder a dev daemon:shim-attributed(zero scrollback lines) — hello-gating working;/resumeproduced a clean rebind (019ed2c9 → 019ed018) with the title following;attributions.jsonon disk, the held file was re-attributed immediately on resubscribe.go test ./...green acrossgmuxd,cli/gmux,packages/adapter;go vetclean.Notes / follow-ups
go:embed, materialized to a content-addressed cache path) — intentionally inspectable by users.hello, so it falls to scrollback on a later daemon connect. That's the fallback's job; the(FALLBACK)log lets us measure it.