Add broker activity tail command#925
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a new ChangesAgent Activity CLI Command
Sequence Diagram(s)sequenceDiagram
participant CLI as registerActivityCommands
participant Session as runActivitySession
participant Resolver as Connection Resolver
participant WS as WebSocketFactory/WebSocket
participant Parser as JSON Parser
participant Formatter as formatActivityEvent
participant Output as stdout/JSON Lines
CLI->>Session: invoke with options
Session->>Resolver: resolve broker URL & api key
Session->>Session: validate sinceSeq & filters
Session->>WS: open connection with URL & API key header
WS->>Parser: message frames (text/binary)
Parser->>Parser: parse JSON, apply kind/name/streams filters
Parser->>Formatter: format matching events
Formatter->>Output: emit pretty line or JSON Line
WS->>Session: close (code/reason)
Session->>CLI: return exit code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/commands/activity.ts (1)
147-410: 💤 Low valueConsider refactoring to reduce complexity.
The static analysis tool flags this function with complexity 88 (max 15). While the code is a straightforward formatting dispatch table and works correctly, consider extracting formatters into a map structure to improve maintainability:
const formatters: Record<string, (event: ActivityEvent, nowIso: string) => string | null> = { relay_inbound: (event, nowIso) => { /* ... */ }, relaycast_published: (event, nowIso) => { /* ... */ }, // etc. }; export function formatActivityEvent(event: ActivityEvent, nowIso: string): string | null { const kind = readString(event, 'kind'); const formatter = formatters[kind ?? ''] ?? formatters.default; return formatter(event, nowIso); }This would reduce complexity metrics while maintaining readability.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/commands/activity.ts` around lines 147 - 410, The function formatActivityEvent is too complex; extract each case's logic into a Record-based dispatcher named formatters: Record<string, (event: ActivityEvent, nowIso: string) => string | null>, implement one entry per kind (e.g. "relay_inbound", "relaycast_published", "delivery_retry", etc.) that calls the existing helpers (formatLine, formatDelivery, detail, quotePreview, actorName, eventId, deliveryId, readString, readCount) and add a formatters.default for the current default branch; then simplify formatActivityEvent to read kind, select formatters[kind ?? ''] ?? formatters.default and return formatter(event, nowIso), keeping all formatting behavior identical to the switch-based implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/cli/commands/activity.ts`:
- Around line 147-410: The function formatActivityEvent is too complex; extract
each case's logic into a Record-based dispatcher named formatters:
Record<string, (event: ActivityEvent, nowIso: string) => string | null>,
implement one entry per kind (e.g. "relay_inbound", "relaycast_published",
"delivery_retry", etc.) that calls the existing helpers (formatLine,
formatDelivery, detail, quotePreview, actorName, eventId, deliveryId,
readString, readCount) and add a formatters.default for the current default
branch; then simplify formatActivityEvent to read kind, select formatters[kind
?? ''] ?? formatters.default and return formatter(event, nowIso), keeping all
formatting behavior identical to the switch-based implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2247bdf9-2d0c-44e4-9cd6-1c1372cdab25
📒 Files selected for processing (5)
CHANGELOG.mdsrc/cli/bootstrap.test.tssrc/cli/bootstrap.tssrc/cli/commands/activity.test.tssrc/cli/commands/activity.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2454032f1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return formatLine( | ||
| nowIso, | ||
| 'ERROR', | ||
| `${actorName(event)} ${readString(event, 'code') ?? 'worker_error'}${quotePreview(readString(event, 'message'), 120)}`, |
There was a problem hiding this comment.
Include nested worker_error payload details
worker_error events from the broker carry diagnostics under an error object ({"kind":"worker_error","name":...,"error":{...}} in crates/broker/src/runtime/worker_events.rs), but this formatter only reads top-level code/message. In practice, agent-relay activity will print a generic worker_error line and drop the actual failure reason whenever a worker emits an error, which defeats the command’s debugging purpose for one of its most important event types.
Useful? React with 👍 / 👎.
Summary:
Tests: