Skip to content

Fix local agent list hang#1042

Merged
willwashburn merged 1 commit into
mainfrom
codex/fix-local-agent-list-empty
Jun 3, 2026
Merged

Fix local agent list hang#1042
willwashburn merged 1 commit into
mainfrom
codex/fix-local-agent-list-empty

Conversation

@willwashburn
Copy link
Copy Markdown
Member

@willwashburn willwashburn commented Jun 3, 2026

User description

Summary

  • make local agent management commands connect to an existing broker instead of spawning an empty broker for read-only/list-style calls
  • disconnect one-shot local agent and metrics clients after the command finishes
  • add a regression test for local agent list using HarnessDriverClient.connect

Diagnosis

relay local agent list was using createRuntimeClient({ preferConnect: true }). When no connection file existed, that helper fell back to HarnessDriverClient.spawn(), which started an empty broker. The command could print [], but the spawned broker child, event stream, and lease timer kept the Node process alive.

Validation

  • npm run build:core
  • npx vitest run packages/cli/src/cli/commands/local-agent.test.ts
  • npm --prefix packages/cli run build
  • npx vitest run packages/cli/src/cli/commands/core.test.ts
  • node packages/cli/dist/cli/index.js local agent list exits immediately with the no-running-broker error in a fresh worktree

CodeAnt-AI Description

Stop read-only local commands from starting a broker

What Changed

  • local agent list now connects to an existing local broker instead of starting a new empty one, so it finishes normally after showing the result
  • local metrics now uses the same connect-only behavior and disconnects when done, which prevents the command from staying open after printing metrics
  • Both commands now close their broker connection after use
  • Added coverage for listing agents through an existing broker connection

Impact

✅ Fewer hangs after read-only local commands
✅ Faster exit for agent list
✅ Shorter local metrics runs

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@willwashburn willwashburn requested a review from khaliqgant as a code owner June 3, 2026 17:29
@codeant-ai
Copy link
Copy Markdown

codeant-ai Bot commented Jun 3, 2026

CodeAnt AI is reviewing your PR.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR migrates local agent and metrics CLI commands from createRuntimeClient to HarnessDriverClient.connect() to prevent them from starting an empty broker that could hang after printing results. Both command implementations now add proper resource cleanup via disconnect() in finally blocks, with test coverage validating the new connection behavior.

Changes

Local Command Broker Connection Migration

Layer / File(s) Summary
Local agent command HarnessDriverClient integration
packages/cli/src/cli/commands/local-agent.ts
Switches from createRuntimeClient to HarnessDriverClient.connect({ cwd }), removes the old runtime client import, and adds unconditional cleanup in a finally block with client?.disconnect?.() to ensure proper resource release even on errors.
Metrics command HarnessDriverClient integration
packages/cli/src/cli/commands/core.ts
Refactors the metrics subcommand to construct a HarnessDriverClient, fetch metrics with client.getMetrics(), log on success, exit with code 1 on failure, and ensure cleanup in a finally block.
Test coverage for local agent list
packages/cli/src/cli/commands/local-agent.test.ts
Adds a Vitest mock for @agent-relay/harness-driver with a shared harnessConnectMock, then verifies local agent list connects to the existing broker, logs the agent list, and calls disconnect without spawning a new broker.
Changelog documentation
CHANGELOG.md
Documents that local agent list and metrics commands no longer start an empty broker; they now connect only to an existing local broker to prevent hanging.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

size:XS

Suggested reviewers

  • khaliqgant

Poem

🐰 A broker once hung from an empty start,
Now our agents connect with the proper art—
HarnessDriverClient guides the way,
No empty waits, just talk and say!
Cleanup's guaranteed in finally's grace,
Metrics and agents know their place. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix local agent list hang' directly addresses the main issue resolved by the PR and accurately summarizes the primary change.
Description check ✅ Passed The PR description covers the required template sections with comprehensive details: a clear summary of changes, diagnosis of the root cause, and detailed validation steps performed.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-local-agent-list-empty

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@codeant-ai codeant-ai Bot added the size:M This PR changes 30-99 lines, ignoring generated files label Jun 3, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/cli/src/cli/commands/core.ts (1)

399-399: 💤 Low value

Consider matching the optional-chaining style used in local-agent.ts.

For consistency, local-agent.ts:69 uses client?.disconnect?.() while this line uses client?.disconnect(). Both work (since disconnect always exists on the client instance), but matching styles across the codebase improves readability.

🎨 Proposed diff for consistency
-        client?.disconnect();
+        client?.disconnect?.();
🤖 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` at line 399, The call uses
inconsistent optional-chaining style: change the invocation from
client?.disconnect() to use safe method chaining client?.disconnect?.() so it
matches the existing pattern (e.g., the other usage of client?.disconnect?.()),
ensuring the code defensively handles a missing disconnect property before
calling it.
🤖 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/cli/src/cli/commands/core.ts`:
- Line 399: The call uses inconsistent optional-chaining style: change the
invocation from client?.disconnect() to use safe method chaining
client?.disconnect?.() so it matches the existing pattern (e.g., the other usage
of client?.disconnect?.()), ensuring the code defensively handles a missing
disconnect property before calling it.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: eac472f7-0e78-46ba-9e04-6ad995845890

📥 Commits

Reviewing files that changed from the base of the PR and between a5ce5aa and 115b58b.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • packages/cli/src/cli/commands/core.ts
  • packages/cli/src/cli/commands/local-agent.test.ts
  • packages/cli/src/cli/commands/local-agent.ts

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Re-trigger cubic

@willwashburn willwashburn merged commit d18bd28 into main Jun 3, 2026
45 checks passed
@willwashburn willwashburn deleted the codex/fix-local-agent-list-empty branch June 3, 2026 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant