-
Notifications
You must be signed in to change notification settings - Fork 0
feat: auto-detect local vs remote bus in make logs
#120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8a4cfff
c770c0f
1195e48
e22f101
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,9 @@ | ||
| .PHONY: check fmt lint test clean install-server install-client uninstall dev venv restart logs | ||
|
|
||
| # Canonical paths (override with matching AGENT_EVENT_BUS_* env vars) | ||
| LOG_FILE := $(or $(AGENT_EVENT_BUS_LOG),$(HOME)/.claude/contrib/agent-event-bus/agent-event-bus.log) | ||
| ERR_FILE := $(or $(AGENT_EVENT_BUS_ERR),$(HOME)/.claude/contrib/agent-event-bus/agent-event-bus.err) | ||
|
|
||
| # Run all quality gates (format check, lint, tests) | ||
| check: fmt lint test | ||
|
|
||
|
|
@@ -124,7 +128,7 @@ restart: | |
| if launchctl list | grep -q "com.evansenter.agent-event-bus"; then \ | ||
| echo "Service restarted successfully"; \ | ||
| else \ | ||
| echo "Error: Service failed to start. Check ~/.claude/contrib/agent-event-bus/agent-event-bus.err"; \ | ||
| echo "Error: Service failed to start. Check $(ERR_FILE)"; \ | ||
| exit 1; \ | ||
| fi; \ | ||
| else \ | ||
|
|
@@ -138,11 +142,35 @@ restart: | |
| if systemctl --user is-active agent-event-bus &>/dev/null; then \ | ||
| echo "Service restarted successfully"; \ | ||
| else \ | ||
| echo "Error: Service failed to start. Check ~/.claude/contrib/agent-event-bus/agent-event-bus.err"; \ | ||
| echo "Error: Service failed to start. Check $(ERR_FILE)"; \ | ||
| exit 1; \ | ||
| fi; \ | ||
| fi | ||
|
|
||
| # Tail the event bus log | ||
| # Tail the event bus log (auto-detects local vs remote bus) | ||
| # Override with BUS_HOST=<tailscale-host> to force a remote tail. | ||
| # Auto-detect parses `claude mcp list` text output (agent-event-bus: URL ...); | ||
| # detection silently falls through to local if that format ever changes. | ||
| # Remote tail path is hardcoded to the canonical default: we can't know the | ||
| # remote bus's AGENT_EVENT_BUS_LOG from here, so a remote override is not | ||
| # honored by `make logs`. Run the tail directly over SSH in that case. | ||
| logs: | ||
| @tail -f ~/.claude/contrib/agent-event-bus/agent-event-bus.log | ||
| @HOST="$(BUS_HOST)"; \ | ||
| if [ -z "$$HOST" ]; then \ | ||
| CLAUDE_CMD=$$(command -v claude || echo "$$HOME/.local/bin/claude"); \ | ||
| if [ -x "$$CLAUDE_CMD" ]; then \ | ||
| URL=$$("$$CLAUDE_CMD" mcp list 2>/dev/null | awk '/^agent-event-bus:/ {print $$2}'); \ | ||
| fi; \ | ||
| if [ -z "$$URL" ]; then \ | ||
| URL="$$AGENT_EVENT_BUS_URL"; \ | ||
| fi; \ | ||
| if echo "$$URL" | grep -qE '^https?://'; then \ | ||
| HOST=$$(echo "$$URL" | sed -E 's|https?://||; s|/.*||; s|:[0-9]+$$||; s|^\[||; s|\]$$||'); \ | ||
| fi; \ | ||
| fi; \ | ||
| if [ -z "$$HOST" ] || [ "$$HOST" = "localhost" ] || [ "$$HOST" = "127.0.0.1" ] || [ "$$HOST" = "::1" ] || [ "$$HOST" = "0.0.0.0" ]; then \ | ||
| tail -f $(LOG_FILE); \ | ||
| else \ | ||
| echo "Tailing remote bus at $$HOST (Ctrl-C to exit)..."; \ | ||
| ssh -t -- "$$HOST" 'tail -f ~/.claude/contrib/agent-event-bus/agent-event-bus.log'; \ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The log path is now repeated on lines 163 and 166 (and appears in the restart error message earlier in the file). Extracting a Make variable at the top of the Makefile would keep the three references in sync. Minor DRY nit.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Expanded scope per user direction — see PR comment above. TL;DR: introduced |
||
| fi | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] The awk parser depends on the current claude mcp list text format (colon-terminated name in field 1, URL in field 2). If that output ever changes (extra prefix, different separator, extra decoration), detection silently fails and falls through to the local tail, which reproduces the exact bug this PR is fixing. A short code comment noting the format dependency would help future debugging, or using claude mcp get agent-event-bus with structured output (if supported) would be a more stable contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved the format-dependency note into the target's header comment block (Makefile:146-148). Kept the text parser since
claude mcp listhas no structured output flag today. Resolved in c770c0f.