Fall back to CLAUDE_CODE_SESSION_ID for CLI session attribution by evansenter · Pull Request #137 · evansenter/agent-event-bus

evansenter · 2026-08-10T00:04:27Z

Closes the last gap in #128. Verified on the Mac mini after evansenter/dotfiles#328 merged — the mapping that PR added never fires where it matters.

What's actually broken

#128 made the CLI read AGENT_EVENT_BUS_SESSION_ID when --session-id is omitted, and dotfiles#328 mapped CLAUDE_CODE_SESSION_ID onto it from ~/.exports. But ~/.exports is sourced from ~/.zshrc, and zsh reads .zshrc for interactive shells only. Tool-spawned subprocesses are non-interactive, so the mapping never runs and every publish from one lands as anonymous — precisely the bug #128 exists to fix.

Measured on the host:

printenv AGENT_EVENT_BUS_SESSION_ID  ->  (unset)
printenv CLAUDE_CODE_SESSION_ID      ->  000a712d-…   (= the SessionStart registered id)
zsh -i -c '…'                        ->  fires correctly
zsh    -c '…'                        ->  empty

Correcting the record

dotfiles#328's PR body scoped the residual risk to "the macOS login-shell chain", and I repeated that in the merge commit. That misattributed it. This is shell startup semantics, not an OS difference — it reproduces identically on Linux under zsh -c. The original container verification passed only because container checks either source .exports directly or run an interactive/login shell, so the .zshrc gating never applied.

Why fix it here rather than in the dotfiles

The CLI fallback holds for every shell, spawner, and machine. The failure we hit was "we assumed a file gets sourced and it isn't"; an rc-file fix trades that assumption for a different one about zsh startup ordering.
The dotfiles alternative — move the mapping to ~/.zshenv, which non-interactive zsh does read — is not the one-line move it appears to be: home/.zshenv is not tracked in that repo (only .exports, .zshrc, .zsh_prompt are), so it needs a new repo file, a bootstrap symlink, and reconciliation with the existing unmanaged ~/.zshenv.
The .exports mapping stays valid and harmless for interactive shells, and keeps working if this fallback is ever removed.

Precedence

Pinned by tests on both call sites (publish and events):

--session-id  >  $AGENT_EVENT_BUS_SESSION_ID  >  $CLAUDE_CODE_SESSION_ID

AGENT_EVENT_BUS_SESSION_ID stays ahead of the fallback because it's the tool-agnostic knob — an operator who sets it deliberately shouldn't be overridden by the ambient one Claude Code happens to inject.

Test-environment fix worth calling out

Added an autouse scrub of both names to test_cli.py. CLAUDE_CODE_SESSION_ID is injected into the subprocess running the suite, so without the scrub an ambient value silently satisfies the assertions that expect no attribution — the suite would pass on a developer's machine and fail in CI, or the reverse. The pre-existing test_no_env_no_flag_omits_session_id would have started failing for exactly this reason.

Testing

make check green: 576 passed (569 + 7).

Empirically confirmed in the broken shape (explicit var unset, non-interactive):

AGENT_EVENT_BUS_SESSION_ID: None
CLAUDE_CODE_SESSION_ID    : 067fd316-…
resolved attribution      : 067fd316-…

🤖 Generated with Claude Code

https://claude.ai/code/session_01AFp3uCezpvbeyAMYnNrQ4R

Generated by Claude Code

#128 made the CLI read AGENT_EVENT_BUS_SESSION_ID when --session-id is omitted, and dotfiles#328 mapped CLAUDE_CODE_SESSION_ID onto it from ~/.exports. Verified on the Mac mini after that merged: the mapping never fires where it matters. ~/.exports is sourced from ~/.zshrc, and zsh reads .zshrc for INTERACTIVE shells only. Tool-spawned subprocesses are non-interactive, so they never run the mapping and every publish from one landed as "anonymous" - the exact bug #128 exists to fix. Measured on the host: printenv AGENT_EVENT_BUS_SESSION_ID -> (unset) printenv CLAUDE_CODE_SESSION_ID -> 000a712d-... (= the registered id) zsh -i -c '...' -> fires correctly zsh -c '...' -> empty That is shell startup semantics, not an OS difference: it reproduces on Linux under `zsh -c`. dotfiles#328's PR body scoped the residual risk to "the macOS login-shell chain", which misattributed it - the container test passed only because container checks use an interactive/login shell or source .exports directly. Fixing it here rather than in shell rc plumbing: this holds for every shell, spawner, and machine, and the dotfiles alternative (move the mapping to ~/.zshenv, which non-interactive zsh does read) is not the one-line move it appears to be - home/.zshenv is not tracked in that repo, so it needs a new file plus a bootstrap symlink plus reconciling the existing unmanaged one. Precedence, pinned by tests on both call sites: explicit --session-id > AGENT_EVENT_BUS_SESSION_ID (the tool-agnostic knob, so a deliberate setting is never overridden) > CLAUDE_CODE_SESSION_ID. The dotfiles mapping stays valid and harmless for interactive shells. Also adds an autouse scrub of both names to test_cli.py. CLAUDE_CODE_SESSION_ID is injected into the subprocess running the suite, so without it an ambient value silently satisfies the assertions that expect NO attribution - the suite would pass locally and fail in CI, or the reverse. make check green: 576 passed (569 + 7). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFp3uCezpvbeyAMYnNrQ4R

claude · 2026-08-10T00:09:49Z

+    registers on the bus with client_id = the Claude Code session id, which the
+    bus adopts as its session id.
+    """
+    return os.environ.get("AGENT_EVENT_BUS_SESSION_ID") or os.environ.get("CLAUDE_CODE_SESSION_ID")


[Suggestion] register/unregister do not get the matching fallback for --client-id.

The invariant this helper documents — the two ids being the same value by construction — only holds if whoever registered passed --client-id with the Claude Code session id. That is currently an external, dotfiles-side obligation. If a SessionStart hook omits it, register_session mints a UUID (server.py:308), and every subsequent CLI publish attributes to a session id with no matching session row: it is stored, but list_sessions will not show it and session:<id> DMs will not route to it.

Defaulting the client_id in cmd_register (and cmd_unregister) from this same helper would close the loop inside this repo, so the fallback is guaranteed to name a session that actually exists. Strictly better than anonymous either way — hence a suggestion, not a blocker.

claude · 2026-08-10T00:09:55Z

+
+
+@pytest.fixture(autouse=True)
+def clean_session_id_env(monkeypatch):


[Suggestion] Consider hoisting this scrub to tests/conftest.py.

The reasoning in the comment above applies to the whole suite, not just this module — test_structured_payload.py:216 and test_signal_levels.py:116 also call cli.cmd_publish/cli.cmd_events under a mocked call_tool. They happen to be safe today only because none of their assertions check for the absence of a session id, so an ambient value lands in call_args unnoticed. The first assertion added there that does check absence would be silently machine-dependent in exactly the way this fixture exists to prevent.

As a conftest autouse fixture it protects every module by construction rather than by the accident of what the other modules currently assert.

claude · 2026-08-10T00:10:01Z

+    The fallback exists because setting the explicit var from a shell profile
+    is not reliable: the dotfiles that map one to the other live in ~/.exports,
+    which is sourced from ~/.zshrc - and zsh reads .zshrc for INTERACTIVE
+    shells only. Tool-spawned subprocesses are non-interactive, so the mapping


[Suggestion] This paragraph pins the rationale to a specific external file layout (~/.exports sourced from ~/.zshrc) in a repo that does not own it. If those dotfiles are ever reorganized — including via the ~/.zshenv move the PR body considers — this reads as stale history rather than a live constraint.

The durable half is the shell semantics: non-interactive shells do not read rc files, so any profile-based mapping is unreachable from a tool-spawned subprocess. Stating that, with the zsh -i -c vs zsh -c observation as the evidence, and dropping the specific filenames would keep the paragraph true regardless of how the dotfiles evolve. The PR description is the right home for the incident detail.

claude · 2026-08-10T00:10:07Z

+# CLAUDE_CODE_SESSION_ID (which Claude Code injects into every subprocess it
+# spawns). The fallback matters because shell-profile mappings of one to the
+# other typically live in an rc file that only INTERACTIVE shells read, so a
+# tool-spawned subprocess never runs them and publishes land as "anonymous".


[Suggestion] The Environment variables line in the Naming Conventions section enumerates only the AGENT_EVENT_BUS_* names. CLAUDE_CODE_SESSION_ID is now a variable this codebase reads, and it is the one consulted name that will never appear under that prefix — so someone auditing which env vars the tool depends on, working from that table, will miss it.

Worth a short parenthetical there — plus CLAUDE_CODE_SESSION_ID, read by the CLI as a session-attribution fallback — so the table stays the complete list. This block is a good explanation of why; the table is where people look for what.

claude

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review — Round 1

Summary

Adds CLAUDE_CODE_SESSION_ID as a last-resort fallback for CLI session attribution behind a single _session_id_from_env() helper, wired into both cmd_publish and cmd_events, with precedence pinned by tests on both call sites and an autouse env scrub so the suite is not ambient-dependent. The diagnosis in the PR body checks out and the fix is at the right layer — an rc-file mapping genuinely cannot reach a non-interactive tool-spawned subprocess, and that is shell startup semantics rather than an OS difference, so the correction to the record is right too.

I traced the downstream paths for the newly-attributed id and found no failure mode:

_publish_event_impl tolerates an unknown session id — _auto_heartbeat no-ops on a missing session (server.py:166-169) and add_event stores the value verbatim — so a fallback id that was never registered degrades to current behavior rather than erroring.
--resume with an unregistered id gets a server-side Session not found result (server.py:601-607), which cmd_events already converts to exit 1. That is the same exit status as the pre-PR --resume requires --session-id path, so no regression.
The same-value-by-construction claim in the new docstring holds: server.py:308 adopts client_id as the session id when one is supplied.
test_structured_payload.py and test_signal_levels.py also drive cmd_publish/cmd_events, but none of their assertions test for the absence of a session id, so the module-scoped scrub is sufficient today (see the inline note).

Precedence is pinned on both call sites, the neither-set case is pinned, and --resume via the fallback is pinned. Good coverage for the size of the change.

Verdict

APPROVE - No blocking findings. Four suggestions posted inline.

Note on mechanics: the heredoc form specified in the prompt is blocked by this environment (any heredoc containing a brace adjacent to a quote is rejected as expansion obfuscation), so the four findings were posted as individual inline review comments via the pulls/comments endpoint and this review body carries the verdict.

Automated review by Claude Code

The bridge merged in #135 with no install target: it ran only as a foreground `uv run agent-event-bus-bridge`, so it died with its shell and never came back after a reboot. The bus has had a LaunchAgent all along, which is why it has been up continuously; the bridge simply never had one. Adds com.evansenter.agent-event-bus-bridge.plist plus install/uninstall scripts and `make install-bridge` / `make uninstall-bridge`. A SEPARATE unit from the bus, not an addition to install-server: a bus host does not have to run a bridge, and the bridge is experimental while the bus is not. The unit pins --backend spool (tmux additionally needs wake/panes.json maintained by something session-side) and leaves AGENT_EVENT_BUS_URL unset so the bridge's own loopback default applies - the correct value on the machine hosting the bus. Boot ordering needs no launchd machinery: register_with_retry already backs off 1s->30s until the bus answers rather than exiting, and its docstring anticipates exactly the same-supervisor-launches-both case. A cold boot in the "wrong" order self-corrects, so the installer reports registered:false as a status line rather than an error. KeepAlive is safe with the singleton lock: flock releases when the dead process's fd closes, so the replacement acquires it cleanly, and the startup sweep reclaims the webhook row the dead instance left behind. ThrottleInterval is stated explicitly because it is load-bearing - it bounds how fast a crash loop rewrites the log. Documented caveat rather than a silent one: the bridge logs via basicConfig (stderr only), so launchd's capture IS its log, and launchd truncates that on every restart. Giving the bridge an append-mode file handler like the bus has is the follow-up that removes it. guide.md gains a supervision section and a four-item verification checklist - crash restart, boot order, reboot, clean unload. Two of those check claims the unit's own comments make (flock release, retry backoff) rather than taking them on trust; the suite mocks launchd entirely, so they have to be manual. Also takes three #137 review follow-ups deferred at that merge: - hoist the session-id env scrub from test_cli.py to conftest.py, so test_structured_payload.py and test_signal_levels.py are covered by construction rather than by the accident of what they currently assert - generalize _session_id_from_env's docstring off the specific ~/.exports and ~/.zshrc paths this repo does not own, keeping the durable half (rc files are interactive-only) - add CLAUDE_CODE_SESSION_ID to the CLAUDE.md env-var table, the one consulted name that will never carry the AGENT_EVENT_BUS_ prefix Not taken, from the same review: defaulting --client-id in register/unregister from the same helper. The gap it closes needs a SessionStart hook that omits --client-id, which is not the current setup (dotfiles session-start.sh:74 passes it), and auto-adopting the session id as client_id would change registration semantics - the (machine, client_id) dedupe means a manual `register` would resume the real session rather than mint a new one. Worth its own change, not a rider on this one. make check green: 576 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFp3uCezpvbeyAMYnNrQ4R

The bridge merged in #135 with no install target - it ran only as a foreground `uv run agent-event-bus-bridge`, so it died with its shell and never survived a reboot. The bus has had a LaunchAgent all along, which is why it stayed up; the bridge simply never had one. Adds com.evansenter.agent-event-bus-bridge.plist, install/uninstall scripts, and `make install-bridge` / `make uninstall-bridge`. A separate unit from the bus on purpose: a bus host need not run a bridge, and the bridge is experimental while the bus is not. Boot ordering needs no launchd machinery - register_with_retry already backs off 1s->30s until the bus answers, so a cold boot in the "wrong" order self-corrects and the installer reports registered:false as a status line rather than an error. KeepAlive is safe with the singleton lock: _acquire_singleton_locks runs before uvicorn.run, so a crash-looping replacement exits on the flock upstream of any bus mutation and cannot sweep a live instance's webhook row; main() also unregisters in its finally before closing the singleton fds. Round-1 review caught a regression this PR introduced: the new label is a SUPERSTRING of the bus label, and three pre-existing `launchctl list | grep` probes were unanchored, so a loaded bridge row satisfied the bus's own did-it-start checks. With the bridge up and the bus failing to load, `make restart` and install-launchagent.sh would report success and exit 0 while the bus was down. Every probe is now anchored (the Makefile needs $$ for a literal $, or the anchor is silently dropped). Also from that round: `make uninstall` now tears the bridge down first (KeepAlive would otherwise respawn it forever against a deleted bus); the guide's claim that no install target exists is corrected; the installer labels logs by what each file actually receives (bridge records go to stderr, so the .err file, not the .log); the health probe polls instead of a flat sleep that landed inside the restart-throttle window; and an import preflight turns a stale venv into a named error rather than a crash-loop that eats its own traceback. Documented caveat: the bridge logs via basicConfig (stderr only), so launchd's capture is its log and launchd truncates that on every restart. An append-mode file handler is the follow-up that removes it. guide.md gains a supervision section and a four-item manual verification checklist - crash restart, boot order, reboot, clean unload - two of which check claims the unit's own comments make, since the suite mocks launchd entirely. Includes three #137 follow-ups: the session-id env scrub hoisted to conftest.py, _session_id_from_env's docstring generalized off dotfile paths this repo does not own, and CLAUDE_CODE_SESSION_ID added to the CLAUDE.md env-var table. make check green: 576 passed.

Both were verified wrong on the real machine after #139 merged, and both were mine. launchd APPENDS to StandardOutPath/StandardErrorPath; it does not truncate. After a kill -9 respawn the bridge's .err still held the prior PID's startup lines and both webhook entries. So the crash-loop-eats-its-own-evidence caveat does not exist, the append-mode file handler it motivated is moot, and ThrottleInterval's justification drops that half. The same false claim lived in the BUS plist, which is where I copied it from - corrected there too. /health's `registered` is the STARTUP result, cached and never re-verified, so guide.md's boot-order check was not reproducible as written: against a bridge that install-bridge just left registered, unloading the bus leaves /health still reporting registered:true. The false state only appears when the bridge itself starts bus-less. Added the missing "restart the bridge" step and stated the corollary plainly - /health is not a bus-liveness probe; it answers "did I register at startup", not "am I registered now". (The /health bullet further up already said the row is not re-verified; the checklist I added contradicted it.) The .err-vs-.log split is unaffected and still documented: bridge records go to .err because basicConfig is stderr-only, .log carries uvicorn access lines. Verified on the bus host alongside these: #137's fallback attributes a non-interactive publish to the real session id (event 4638), and #139's crash restart reclaims exactly one webhook row (stale #2 removed, #3 registered). make check green: 576 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFp3uCezpvbeyAMYnNrQ4R

- guide.md: "every refusal tells you where you are the same way" sat under a five-item list whose newest entry - the deleted-session refusal added in round 12 - is the one refusal `_deleted_session_error` builds with no `cursor` key at all. A hook written literally from that paragraph gets a KeyError on that path rather than the null the next paragraph promises. Scoped to the four cursor refusals, with the deleted case recovering via register_session, which is what its own hint already says. test_every_refusal_reports_a_re_ackable_position enumerates exactly those four, so the code and the test already agreed - only the prose was out. - guide.md: the drain snippet never said a pass is bounded by `limit`, so a Stop hook written from it needs ten invocations to clear a 500-event backlog. Correct and lossless either way, but the reader deciding "is one call per hook enough?" decides it here, not seven sections up where has_more is explained for plain polling. - cli.py: `ack` is the one verb whose stated consumer is a hook, and the only one whose --session-id help stopped at $AGENT_EVENT_BUS_SESSION_ID. The #137 fallback exists for tool-spawned subprocesses, which is exactly that hook. Help and the missing-id error now name both, matching publish/events. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N11TJkcFv2Bn4YQvbYsQ37

Both were verified wrong on the real machine after #139 merged, and both were mine. launchd APPENDS to StandardOutPath/StandardErrorPath; it does not truncate. After a kill -9 respawn the bridge's .err still held the prior PID's startup lines and both webhook entries. So the crash-loop-eats-its-own-evidence caveat does not exist, the append-mode file handler it motivated is moot, and ThrottleInterval's justification drops that half. The same false claim lived in the BUS plist, which is where I copied it from - corrected there too. /health's `registered` is the STARTUP result, cached and never re-verified, so guide.md's boot-order check was not reproducible as written: against a bridge that install-bridge just left registered, unloading the bus leaves /health still reporting registered:true. The false state only appears when the bridge itself starts bus-less. Added the missing "restart the bridge" step and stated the corollary plainly - /health is not a bus-liveness probe; it answers "did I register at startup", not "am I registered now". (The /health bullet further up already said the row is not re-verified; the checklist I added contradicted it.) The .err-vs-.log split is unaffected and still documented: bridge records go to .err because basicConfig is stderr-only, .log carries uvicorn access lines. Verified on the bus host alongside these: #137's fallback attributes a non-interactive publish to the real session id (event 4638), and #139's crash restart reclaims exactly one webhook row (stale #2 removed, #3 registered). make check green: 576 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFp3uCezpvbeyAMYnNrQ4R

claude Bot reviewed Aug 10, 2026

View reviewed changes

claude Bot approved these changes Aug 10, 2026

View reviewed changes

evansenter merged commit 39d697a into main Aug 10, 2026
14 checks passed

This was referenced Aug 10, 2026

Supervise the RFC #122 bridge as a LaunchAgent #139

Merged

Cleanup pass before feature work #138

Merged

evansenter mentioned this pull request Aug 10, 2026

Harden the bridge installer; correct two claims the bus host disproved #141

Merged

claude Bot mentioned this pull request Aug 10, 2026

feat: add ack_events so a filtered drain can commit what it surfaced #143

Merged

evansenter mentioned this pull request Aug 10, 2026

publish_event: soft-deleted sessions can still write to the bus, silently #144

Open

claude Bot mentioned this pull request Aug 10, 2026

fix: flag publishes from soft-deleted sessions instead of dropping them (#144) #146

Open

Navigation Menu

Fall back to CLAUDE_CODE_SESSION_ID for CLI session attribution - #137

Fall back to CLAUDE_CODE_SESSION_ID for CLI session attribution#137
evansenter merged 1 commit into
mainfrom
claude/codebase-review-next-steps-vphsgo

evansenter commented Aug 10, 2026

Uh oh!

claude Bot Aug 10, 2026

Uh oh!

claude Bot Aug 10, 2026

Uh oh!

claude Bot Aug 10, 2026

Uh oh!

claude Bot Aug 10, 2026

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants



		@pytest.fixture(autouse=True)
		def clean_session_id_env(monkeypatch):

Conversation

evansenter commented Aug 10, 2026

What's actually broken

Correcting the record

Why fix it here rather than in the dotfiles

Precedence

Test-environment fix worth calling out

Testing

Uh oh!

claude Bot Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants