feat(agentx): add orchestrator detection with X-Orchestrator header#60
Merged
Conversation
Co-Authored-By: SageOx <ox@sageox.ai>
Orchestrator context is already conveyed via X-Orchestrator header on all SageOx API requests. The header-level telemetry is sufficient; duplicating it in every event payload is redundant. Co-Authored-By: SageOx <ox@sageox.ai>
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR centralizes User-Agent header construction, adds orchestrator detection and propagation into the user-agent context, introduces role-based agent distinctions, and registers two new orchestrators (OpenClaw, Conductor). It also adds orchestrator-aware header utilities and updates agent detection APIs and many agent implementations/tests. Changes
Sequence DiagramsequenceDiagram
participant Prime as Agent Prime
participant Registry as Agent Registry
participant Orchestrator as Orchestrator Agent
participant UserAgent as User-Agent Module
participant Client as HTTP Client
Prime->>Registry: DetectOrchestrator(ctx)
Registry->>Orchestrator: Detect(ctx, env)
Orchestrator-->>Registry: Detected (type)
Registry-->>Prime: Orchestrator Agent
Prime->>UserAgent: SetOrchestratorType(agentType)
UserAgent->>UserAgent: store orchestrator type, invalidate cache
Client->>UserAgent: SetHeaders(req.Header)
UserAgent->>UserAgent: compose UA tokens (include orchestrator if set)
UserAgent->>Client: set `User-Agent` and `X-Orchestrator` headers
Client->>Client: send HTTP request with headers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
internal/useragent/useragent.go (1)
58-71: Unnecessary cache invalidation inSetOrchestratorType.Line 70 clears
cached, butcachedis only used byString()which does not includeorchestratorTypein the User-Agent string. The invalidation is harmless but misleading — a future reader might assumeorchestratorTypeparticipates in the cached UA string.💡 Remove the unnecessary cache clear
func SetOrchestratorType(ot string) { if ot == "" { return } mu.Lock() defer mu.Unlock() if orchestratorType != "" { return } orchestratorType = ot - cached = "" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/useragent/useragent.go` around lines 58 - 71, SetOrchestratorType unnecessarily clears the package-level cached string even though cached is only used by String() and String() does not include orchestratorType; remove the misleading cache invalidation by deleting the line that sets cached = "" in SetOrchestratorType (leave the rest of the function, including mu locking, orchestratorType assignment, and the early returns, intact) so that cached is not touched when setting orchestratorType.pkg/agentx/orchestrators/conductor.go (2)
31-45: Same hardcoded string literal as OpenClaw — use the constant.Line 32 uses
"conductor"instead ofstring(agentx.AgentTypeConductor).♻️ Use the constant
func (a *ConductorAgent) Detect(_ context.Context, env agentx.Environment) (bool, error) { - if env.GetEnv("ORCHESTRATOR_ENV") == "conductor" { + if env.GetEnv("ORCHESTRATOR_ENV") == string(agentx.AgentTypeConductor) { return true, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/agentx/orchestrators/conductor.go` around lines 31 - 45, The Detect method in ConductorAgent uses the hardcoded string "conductor" — replace that literal with the canonical constant by calling string(agentx.AgentTypeConductor) in the first env check (inside ConductorAgent.Detect) so the comparison uses the shared AgentTypeConductor constant rather than a duplicated string; leave the other env checks unchanged.
59-61: Capabilities stub returns empty — consider a TODO.All capabilities are
false. If Conductor supports any of these features now or soon, this should be updated. A brief TODO comment would help track this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/agentx/orchestrators/conductor.go` around lines 59 - 61, The Capabilities method on ConductorAgent currently returns an empty agentx.Capabilities struct; update ConductorAgent.Capabilities to reflect any supported features (or at minimum add a clear TODO comment) so this isn't silently all-false—locate the Capabilities method on type ConductorAgent and either populate the returned agentx.Capabilities fields that Conductor supports or add a TODO comment above the function (e.g., "// TODO: populate supported capabilities such as X, Y") explaining what needs to be enabled and when.pkg/agentx/registry.go (1)
93-120: Clean refactor todetectByRole— non-deterministic winner when multiple match.The refactoring from inline detection to
detectByRoleis well-structured. One subtlety:registry.List()iterates amap[AgentType]Agent, so if multiple orchestrators (or agents) match, the winner is non-deterministic across runs. With only two orchestrators today this is unlikely to matter, but if it ever does, consider sortingList()output or adding a priority field.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/agentx/registry.go` around lines 93 - 120, detectByRole currently iterates registry.List() (which comes from a map) and can pick a non-deterministic winner when multiple agents match; make the selection deterministic by sorting the agents before iterating in detectByRole: obtain the slice from registry.List(), sort it by a stable key (e.g., implement/use an Agent.Priority() integer or fall back to Agent.Type() or name string) with a stable comparison, then iterate the sorted slice to call agent.Detect(ctx, env); update detectByRole to perform this sort so Agent selection is reproducible.pkg/agentx/orchestrators/openclaw.go (1)
32-46: Useagentx.AgentTypeOpenClawconstant instead of hardcoded"openclaw"string.Line 33 uses a hardcoded string literal instead of the defined constant. Since
AgentTypeis a string type alias, usingstring(agentx.AgentTypeOpenClaw)ensures the comparison stays synchronized with the constant definition. The same pattern applies to Conductor at line 32 inconductor.go.♻️ Use the constant
func (a *OpenClawAgent) Detect(_ context.Context, env agentx.Environment) (bool, error) { - if env.GetEnv("ORCHESTRATOR_ENV") == "openclaw" { + if env.GetEnv("ORCHESTRATOR_ENV") == string(agentx.AgentTypeOpenClaw) { return true, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/agentx/orchestrators/openclaw.go` around lines 32 - 46, Replace the hardcoded "openclaw" string in Detect with the AgentType constant: change the comparison in OpenClawAgent.Detect to use string(agentx.AgentTypeOpenClaw) instead of "openclaw"; also search for the same literal in conductor.go and replace it with string(agentx.AgentTypeOpenClaw) to keep comparisons synchronized with the AgentTypeOpenClaw constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/ox/doctor_ecosystem.go`:
- Around line 91-93: The request to GitHub must not include the X-Orchestrator
header; replace the call to useragent.SetHeaders(req.Header) in the GitHub API
request path with setting only the User-Agent header (e.g., read the user-agent
value via your useragent helper and set req.Header.Set("User-Agent", ...)) so
X-Orchestrator is not sent; update the code that currently calls
useragent.SetHeaders to instead set only the User-Agent header for the req used
in the GitHub call.
In `@internal/lfs/client.go`:
- Around line 125-128: The LFS batch request must not send the X-Orchestrator
header; remove the call to useragent.SetHeaders(req.Header) and instead only set
the User-Agent header on req (e.g., req.Header.Set("User-Agent", <useragent
string>)) so X-Orchestrator is not injected; locate the call to
useragent.SetHeaders in client.go alongside req.Header and c.authHeader and
replace it with code that obtains the user-agent string from the useragent
package (use its function that returns the UA string) and sets only the
"User-Agent" header.
In `@pkg/agentx/agent_test.go`:
- Around line 160-231: Refactor the three existing tests
(TestDetectOrchestrator, TestDetectOrchestrator_NoneRegistered,
TestDetect_SkipsOrchestrators) into a single table-driven test that iterates
cases for: (a) normal detection of a coding agent vs orchestrator using
reg.Detector().Detect() and Detector.DetectOrchestrator(), (b) no orchestrators
registered returning nil, and (c) handling when an agent's Detect() returns an
error—add a mockAgent case whose Detect() returns an error and assert the
detector skips it and continues; ensure you reference and exercise
detectByRole() behavior by creating entries with role=RoleOrchestrator and role
unset and asserting the expected Type() or nil result for each case.
In `@pkg/agentx/orchestrators/version_helpers.go`:
- Around line 11-23: The versionFromCommand function discards the caller's
context by using context.Background(); update its signature to accept a
context.Context (e.g., versionFromCommand(ctx context.Context, env
agentx.Environment, name string, args ...string)) and replace the env.Exec call
to use that ctx so timeouts/cancellation propagate (env.Exec(ctx, ...)). Also
update the caller DetectVersion to pass its ctx into versionFromCommand so
version detection won't hang if the caller cancels or times out.
---
Nitpick comments:
In `@internal/useragent/useragent.go`:
- Around line 58-71: SetOrchestratorType unnecessarily clears the package-level
cached string even though cached is only used by String() and String() does not
include orchestratorType; remove the misleading cache invalidation by deleting
the line that sets cached = "" in SetOrchestratorType (leave the rest of the
function, including mu locking, orchestratorType assignment, and the early
returns, intact) so that cached is not touched when setting orchestratorType.
In `@pkg/agentx/orchestrators/conductor.go`:
- Around line 31-45: The Detect method in ConductorAgent uses the hardcoded
string "conductor" — replace that literal with the canonical constant by calling
string(agentx.AgentTypeConductor) in the first env check (inside
ConductorAgent.Detect) so the comparison uses the shared AgentTypeConductor
constant rather than a duplicated string; leave the other env checks unchanged.
- Around line 59-61: The Capabilities method on ConductorAgent currently returns
an empty agentx.Capabilities struct; update ConductorAgent.Capabilities to
reflect any supported features (or at minimum add a clear TODO comment) so this
isn't silently all-false—locate the Capabilities method on type ConductorAgent
and either populate the returned agentx.Capabilities fields that Conductor
supports or add a TODO comment above the function (e.g., "// TODO: populate
supported capabilities such as X, Y") explaining what needs to be enabled and
when.
In `@pkg/agentx/orchestrators/openclaw.go`:
- Around line 32-46: Replace the hardcoded "openclaw" string in Detect with the
AgentType constant: change the comparison in OpenClawAgent.Detect to use
string(agentx.AgentTypeOpenClaw) instead of "openclaw"; also search for the same
literal in conductor.go and replace it with string(agentx.AgentTypeOpenClaw) to
keep comparisons synchronized with the AgentTypeOpenClaw constant.
In `@pkg/agentx/registry.go`:
- Around line 93-120: detectByRole currently iterates registry.List() (which
comes from a map) and can pick a non-deterministic winner when multiple agents
match; make the selection deterministic by sorting the agents before iterating
in detectByRole: obtain the slice from registry.List(), sort it by a stable key
(e.g., implement/use an Agent.Priority() integer or fall back to Agent.Type() or
name string) with a stable comparison, then iterate the sorted slice to call
agent.Detect(ctx, env); update detectByRole to perform this sort so Agent
selection is reproducible.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
cmd/ox/agent_prime.gocmd/ox/doctor_ecosystem.gointernal/api/ledger.gointernal/api/repo.gointernal/api/repos.gointernal/auth/client.gointernal/auth/device_flow.gointernal/auth/refresh.gointernal/doctorapi/client.gointernal/lfs/client.gointernal/lfs/transfer.gointernal/session/summarize.gointernal/telemetry/client.gointernal/useragent/useragent.gointernal/useragent/useragent_test.gointernal/uxfriction/client.gopkg/agentx/agent.gopkg/agentx/agent_test.gopkg/agentx/agents/aider.gopkg/agentx/agents/amp.gopkg/agentx/agents/claudecode.gopkg/agentx/agents/cline.gopkg/agentx/agents/codepuppy.gopkg/agentx/agents/cody.gopkg/agentx/agents/continue.gopkg/agentx/agents/copilot.gopkg/agentx/agents/cursor.gopkg/agentx/agents/droid.gopkg/agentx/agents/goose.gopkg/agentx/agents/kiro.gopkg/agentx/agents/opencode.gopkg/agentx/agents/windsurf.gopkg/agentx/orchestrators/conductor.gopkg/agentx/orchestrators/conductor_test.gopkg/agentx/orchestrators/openclaw.gopkg/agentx/orchestrators/openclaw_test.gopkg/agentx/orchestrators/version_helpers.gopkg/agentx/registry.gopkg/agentx/setup/setup.go
Co-Authored-By: SageOx <ox@sageox.ai>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/agentx/registry.go (1)
101-103: Clarify the env-priority comment for role-based detection.
detectByRoleis also used for orchestrators, so the comment should mention the role-specific override env var.✏️ Suggested comment tweak
-// Each agent's Detect() handles AGENT_ENV priority internally. +// Each agent's Detect() handles role-specific env priority internally (e.g., AGENT_ENV or ORCHESTRATOR_ENV).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/agentx/registry.go` around lines 101 - 103, Update the comment for the detectByRole function to clarify that it returns the first agent detected for the given AgentRole and that each agent's Detect() respects AGENT_ENV priority as well as a role-specific override environment variable (so this function is used for orchestrators too); mention the role-specific override env var name and that it takes precedence for role-based detection to make the behavior explicit (reference detectByRole and Detect()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/agentx/registry.go`:
- Around line 101-103: Update the comment for the detectByRole function to
clarify that it returns the first agent detected for the given AgentRole and
that each agent's Detect() respects AGENT_ENV priority as well as a
role-specific override environment variable (so this function is used for
orchestrators too); mention the role-specific override env var name and that it
takes precedence for role-based detection to make the behavior explicit
(reference detectByRole and Detect()).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/agentx/agent_test.gopkg/agentx/registry.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/agentx/agent_test.go
…creation Co-Authored-By: SageOx <ox@sageox.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement first-class orchestrator support in
agentxto distinguish platforms that launch coding agents (OpenClaw, Conductor) from the coding agents themselves. Orchestrators are now reported via theX-OrchestratorHTTP header on all SageOx API requests.Key changes:
AgentRoletype system (RoleAgent/RoleOrchestrator) to distinguish agent typesDetectOrchestrator()in registry + two orchestrator implementations (Conductor, OpenClaw)ORCHESTRATOR_ENVenv var for explicit orchestrator identification (separate fromAGENT_ENV)X-Orchestratorheader sent on all SageOx API requests viauseragent.SetHeaders()DetectOrchestrator()Test Plan
TestDetectOrchestrator,TestDetectOrchestrator_NoneRegistered,TestDetect_SkipsOrchestratorsTestSetHeaders_WithOrchestrator,TestSetHeaders_WithoutOrchestratorX-Orchestratorheader is sent on all SageOx API requests (auth, api, lfs, telemetry, friction, etc.)Co-Authored-By: SageOx ox@sageox.ai
Summary by CodeRabbit
New Features
Improvements
Tests