Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions experimental/ssh/internal/client/agentshim.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ const (
)

type agentSpec struct {
name string
name string
telemetryName protos.SshAgentShimAgent
// contextFlag passes the context file via this CLI flag (Claude's --append-system-prompt-file).
contextFlag string
// contextHomeFile writes the context to this $HOME instructions file (codex's AGENTS.md).
Expand All @@ -94,8 +95,8 @@ type agentSpec struct {

// Currently limited to the agents whose Unity Gateway CLI command supports --workspace
var supportedAgents = []agentSpec{
{name: "claude", contextFlag: "--append-system-prompt-file"},
{name: "codex", contextHomeFile: ".codex/AGENTS.md"},
{name: "claude", telemetryName: protos.SshAgentShimAgentClaudeCode, contextFlag: "--append-system-prompt-file"},
{name: "codex", telemetryName: protos.SshAgentShimAgentCodex, contextHomeFile: ".codex/AGENTS.md"},
}

// SupportedAgentNames lists the agents the ssh command registers a subcommand for.
Expand Down Expand Up @@ -237,8 +238,15 @@ func logAgentShimEvent(ctx context.Context, agentName string, outcome agentShimO
// buildAgentShimEvent maps the agent name and outcome onto the telemetry event. It is
// separated from logAgentShimEvent so the field mapping can be unit tested.
func buildAgentShimEvent(agentName string, outcome agentShimOutcome, setupDuration time.Duration) *protos.SshAgentShimEvent {
// lookup agent by name so this function is the single decision point for
// telemetry and is easily testable
telemetryName := protos.SshAgentShimAgentUnspecified
agent, ok := agentByName(agentName)
if ok {
telemetryName = agent.telemetryName
}
return &protos.SshAgentShimEvent{
AgentName: agentName,
Agent: telemetryName,
IsSuccess: outcome.err == nil,
ErrorCategory: outcome.category(),
SetupDurationMs: setupDuration.Milliseconds(),
Expand Down
16 changes: 14 additions & 2 deletions experimental/ssh/internal/client/agentshim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -565,10 +565,22 @@ func TestProbeModelServicesPageError(t *testing.T) {
}

func TestBuildAgentShimEvent(t *testing.T) {
for _, agent := range supportedAgents {
Comment thread
rclarey marked this conversation as resolved.
Resolved
t.Run(agent.name+" has an appropriate enum for telemetry", func(t *testing.T) {
got := buildAgentShimEvent(agent.name, agentShimOutcome{}, 0)
assert.NotEqual(t, protos.SshAgentShimAgentUnspecified, got.Agent)
})
}

t.Run("maps an unknown agent to unspecified", func(t *testing.T) {
got := buildAgentShimEvent("wibble", agentShimOutcome{}, 0)
assert.Equal(t, protos.SshAgentShimAgentUnspecified, got.Agent)
})

t.Run("successful launch reports success and no category", func(t *testing.T) {
got := buildAgentShimEvent("claude", agentShimOutcome{}, 1500*time.Millisecond)
assert.Equal(t, &protos.SshAgentShimEvent{
AgentName: "claude",
Agent: protos.SshAgentShimAgentClaudeCode,
IsSuccess: true,
ErrorCategory: protos.SshAgentShimErrorCategoryUnspecified,
SetupDurationMs: 1500,
Expand All @@ -580,7 +592,7 @@ func TestBuildAgentShimEvent(t *testing.T) {
err: categorize(protos.SshAgentShimErrorCategoryGatewayAuthFailed, errors.New("boom")),
}, 200*time.Millisecond)
assert.Equal(t, &protos.SshAgentShimEvent{
AgentName: "codex",
Agent: protos.SshAgentShimAgentCodex,
IsSuccess: false,
ErrorCategory: protos.SshAgentShimErrorCategoryGatewayAuthFailed,
SetupDurationMs: 200,
Expand Down
21 changes: 18 additions & 3 deletions libs/telemetry/protos/ssh_agent_shim.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,21 @@ const (
SshAgentShimErrorCategoryUnknown SshAgentShimErrorCategory = "UNKNOWN"
)

// The agent for an `ssh agent-shim <agent>` command invocation. Values are in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[nit] Start the type comment with the type name, as SshAgentShimErrorCategory does. For example: "SshAgentShimAgent mirrors SshAgentShimAgentType.Type in the databricks_cli lumberjack proto." The comment says "JSON->proto serialization", but this step is deserialization. Remove the extra space in // The agent the user asked to launch. Add a doc comment to agentNameToAgentEnum that starts with the function name.

// screaming snake case to match what JSON->proto deserialization expects.
type SshAgentShimAgent string

const (
// Unknown agent was specified.
SshAgentShimAgentUnspecified SshAgentShimAgent = "TYPE_UNSPECIFIED"

// Claude Code CLI.
SshAgentShimAgentClaudeCode SshAgentShimAgent = "CLAUDE_CODE"

// Codex CLI.
SshAgentShimAgentCodex SshAgentShimAgent = "CODEX"
)

// SshAgentShimEvent is emitted when a user runs `ssh agent-shim <agent>` on the remote
// driver to launch a coding agent against the Unity AI Gateway.
//
Expand All @@ -89,9 +104,9 @@ const (
// uploaded from the shim just before the exec. A failed launch returns normally and is
// uploaded by cmd/root as usual.
type SshAgentShimEvent struct {
// Name of the agent the user asked to launch (e.g. "claude", "codex"). Drawn from a
// fixed set of CLI-defined identifiers, never user-authored text.
AgentName string `json:"agent_name"`
// The agent the user asked to launch, mapped to a string matching the proto enum
// (e.g. "CLAUDE_CODE", "CODEX", etc).
Agent SshAgentShimAgent `json:"agent"`

// Whether the agent was launched. False means setup failed before the agent ran.
// Populated on every event, so no omitempty: a genuine false must stay distinguishable
Expand Down
2 changes: 1 addition & 1 deletion libs/telemetry/protos/ssh_agent_shim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestSshAgentShimEventEncodesFailureExplicitly(t *testing.T) {
assert.Equal(t, false, got["is_success"], "is_success must be sent as false, not omitted")
// The remaining fields must appear in the payload at all: omitempty would drop the
// zero value and make a failure indistinguishable from an unreported one.
for _, field := range []string{"agent_name", "error_category", "setup_duration_ms"} {
for _, field := range []string{"agent", "error_category", "setup_duration_ms"} {
assert.Contains(t, got, field, "%s must be sent, not omitted", field)
}
}
Loading