diff --git a/cliv2-private/go.mod b/cliv2-private/go.mod index 802da0c5e4..273423d00d 100644 --- a/cliv2-private/go.mod +++ b/cliv2-private/go.mod @@ -249,6 +249,7 @@ require ( github.com/tmc/langchaingo v0.1.14 // indirect github.com/toon-format/toon-go v0.0.0-20251202084852-7ca0e27c4e8c // indirect github.com/ulikunitz/xz v0.5.15 // indirect + github.com/vercel/detect-agent v1.2.0 // indirect github.com/vincent-petithory/dataurl v1.0.0 // indirect github.com/writeas/go-strip-markdown v2.0.1+incompatible // indirect github.com/xanzy/ssh-agent v0.3.3 // indirect diff --git a/cliv2-private/go.sum b/cliv2-private/go.sum index ff4536f446..7c5c601bbe 100644 --- a/cliv2-private/go.sum +++ b/cliv2-private/go.sum @@ -669,6 +669,8 @@ github.com/toon-format/toon-go v0.0.0-20251202084852-7ca0e27c4e8c h1:D8lDFovBMZy github.com/toon-format/toon-go v0.0.0-20251202084852-7ca0e27c4e8c/go.mod h1:j/BOnpF2ihnz4lELs99h9mwGJBx/zdleOUCnLLRPCsc= github.com/ulikunitz/xz v0.5.15 h1:9DNdB5s+SgV3bQ2ApL10xRc35ck0DuIX/isZvIk+ubY= github.com/ulikunitz/xz v0.5.15/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= +github.com/vercel/detect-agent v1.2.0 h1:cv+oFiSknLWbsidRlZWXcrdHP9FXVeeZRb2ib67Zd8k= +github.com/vercel/detect-agent v1.2.0/go.mod h1:hQafiuMWbYQSNhDa/2/cxz4V/3m6WRiQWkcllhNULp8= github.com/vincent-petithory/dataurl v1.0.0 h1:cXw+kPto8NLuJtlMsI152irrVw9fRDX8AbShPRpg2CI= github.com/vincent-petithory/dataurl v1.0.0/go.mod h1:FHafX5vmDzyP+1CQATJn7WFKc9CvnvxyvZy6I1MrG/U= github.com/writeas/go-strip-markdown v2.0.1+incompatible h1:IIqxTM5Jr7RzhigcL6FkrCNfXkvbR+Nbu1ls48pXYcw= diff --git a/cliv2/go.mod b/cliv2/go.mod index 0fae5fc906..5c0004a5ad 100644 --- a/cliv2/go.mod +++ b/cliv2/go.mod @@ -30,6 +30,7 @@ require ( github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 + github.com/vercel/detect-agent v1.2.0 golang.org/x/mod v0.38.0 ) diff --git a/cliv2/go.sum b/cliv2/go.sum index 741ff85ef1..3f1d87c5df 100644 --- a/cliv2/go.sum +++ b/cliv2/go.sum @@ -605,6 +605,8 @@ github.com/tklauser/numcpus v0.10.0 h1:18njr6LDBk1zuna922MgdjQuJFjrdppsZG60sHGfj github.com/tklauser/numcpus v0.10.0/go.mod h1:BiTKazU708GQTYF4mB+cmlpT2Is1gLk7XVuEeem8LsQ= github.com/ulikunitz/xz v0.5.15 h1:9DNdB5s+SgV3bQ2ApL10xRc35ck0DuIX/isZvIk+ubY= github.com/ulikunitz/xz v0.5.15/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= +github.com/vercel/detect-agent v1.2.0 h1:cv+oFiSknLWbsidRlZWXcrdHP9FXVeeZRb2ib67Zd8k= +github.com/vercel/detect-agent v1.2.0/go.mod h1:hQafiuMWbYQSNhDa/2/cxz4V/3m6WRiQWkcllhNULp8= github.com/vincent-petithory/dataurl v1.0.0 h1:cXw+kPto8NLuJtlMsI152irrVw9fRDX8AbShPRpg2CI= github.com/vincent-petithory/dataurl v1.0.0/go.mod h1:FHafX5vmDzyP+1CQATJn7WFKc9CvnvxyvZy6I1MrG/U= github.com/writeas/go-strip-markdown v2.0.1+incompatible h1:IIqxTM5Jr7RzhigcL6FkrCNfXkvbR+Nbu1ls48pXYcw= diff --git a/cliv2/internal/persona/agent/agent.go b/cliv2/internal/persona/agent/agent.go index 2cb22fa0ab..d105f6c282 100644 --- a/cliv2/internal/persona/agent/agent.go +++ b/cliv2/internal/persona/agent/agent.go @@ -1,133 +1,91 @@ -// This file is adapted from @vercel/detect-agent (v1.2.3) by Vercel, Inc. -// Source: https://github.com/vercel/vercel/blob/0d0b990edda112c5cc91e95e0d054878542fe3be/packages/detect-agent/src/index.ts -// -// Original work Copyright 2017 Vercel, Inc. -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// This Go port has been modified from the original TypeScript source. - +// Package agent identifies the coding-tool Harness driving the current CLI +// invocation (e.g. Claude Code, Cursor, Codex), split into a canonical name +// and, when one can be determined, a version. package agent import ( - "os" + "regexp" "strings" -) -// Agent is the canonical name of a known AI coding agent / harness. -type Agent string - -const ( - AgentCursor Agent = "cursor" - AgentCursorCLI Agent = "cursor-cli" - AgentClaude Agent = "claude" - AgentCowork Agent = "cowork" - AgentDevin Agent = "devin" - AgentReplit Agent = "replit" - AgentGemini Agent = "gemini" - AgentCodex Agent = "codex" - AgentAntigravity Agent = "antigravity" - AgentAugmentCLI Agent = "augment-cli" - AgentOpenCode Agent = "opencode" - AgentGitHubCopilot Agent = "github-copilot" - AgentV0 Agent = "v0" + detectagent "github.com/vercel/detect-agent" ) -// devinMarkerPath is a filesystem marker present inside the Devin sandbox. -const devinMarkerPath = "/opt/.devin" - -// lookup abstracts the environment so detection can be tested without mutating -// the real process environment. -type lookup struct { - getenv func(string) string - fileExists func(string) bool -} - -func osLookup() lookup { - return lookup{ - getenv: os.Getenv, - fileExists: func(path string) bool { - _, err := os.Stat(path) - return err == nil - }, +// DetectAgent resolves the Harness identifier the current process declared +// via AI_AGENT, or that detect-agent recognised from an environment +// signature. The value is returned exactly as detect-agent produced it: an +// explicit AI_AGENT declaration passes through verbatim (trimmed), a +// signature match is already canonical. No version split or canonicalisation +// is applied here — see SplitVersion. +func DetectAgent() (string, bool) { + details, err := detectagent.Detect() + if err != nil { + return "", false } + return details.Name, true } -// signature describes how a single agent is recognised from the environment. -type signature struct { - agent Agent - match func(l lookup) bool -} +// versionSuffix recognises a version-shaped suffix on a Harness identifier, +// one of two ways: +// +// - fused onto the name with '_' or '/', with an optional trailing +// role/surface segment (captured only so it isn't absorbed into the +// version, then discarded). These are ordinary characters that can end a +// real name on their own, so at least two numeric components are +// required to avoid matching a name that merely ends in a single digit. +// - detect-agent's documented name@version convention for custom AI_AGENT +// declarations (README example: "devin@1"). '@' is never part of a +// Harness name, so the separator alone is the version signal and a +// single numeric component is enough. +// +// Exactly one of the two version groups is populated per match. +var versionSuffix = regexp.MustCompile(`^(?P.+?)(?:[_/](?P\d+(?:[.\-]\d+){1,3})(?:[_/](?P[A-Za-z0-9-]+))?|@(?P\d+(?:[.\-]\d+){0,3}))$`) -// anySet reports true if any of the given environment variables is set to a -// non-empty value. -func anySet(l lookup, keys ...string) bool { - for _, k := range keys { - if l.getenv(k) != "" { - return true - } - } - return false -} +// knownNames maps a normalised (lowercased, separators stripped) form of +// every Harness name detect-agent knows about to that name's canonical +// spelling, so a version-stripped fragment resolves to the same vocabulary +// detect-agent itself uses rather than a hand-written alias list. +var knownNames = buildKnownNames() -// signatures is the ordered list of agent detectors. Order matters: the first -// match wins, so more specific signatures must precede more generic ones. -var signatures = []signature{ - {AgentCursor, func(l lookup) bool { return anySet(l, "CURSOR_TRACE_ID") }}, - {AgentCursorCLI, func(l lookup) bool { - return anySet(l, "CURSOR_AGENT") || l.getenv("CURSOR_EXTENSION_HOST_ROLE") == "agent-exec" - }}, - {AgentGemini, func(l lookup) bool { return anySet(l, "GEMINI_CLI") }}, - {AgentCodex, func(l lookup) bool { return anySet(l, "CODEX_SANDBOX", "CODEX_CI", "CODEX_THREAD_ID") }}, - {AgentAntigravity, func(l lookup) bool { return anySet(l, "ANTIGRAVITY_AGENT") }}, - {AgentAugmentCLI, func(l lookup) bool { return anySet(l, "AUGMENT_AGENT") }}, - {AgentOpenCode, func(l lookup) bool { return anySet(l, "OPENCODE_CLIENT") }}, - // Claude Code: the "cowork" surface is a more specific variant and must be - // checked first. - {AgentCowork, func(l lookup) bool { - return anySet(l, "CLAUDECODE", "CLAUDE_CODE") && anySet(l, "CLAUDE_CODE_IS_COWORK") - }}, - {AgentClaude, func(l lookup) bool { return anySet(l, "CLAUDECODE", "CLAUDE_CODE") }}, - {AgentReplit, func(l lookup) bool { return anySet(l, "REPL_ID") }}, - {AgentGitHubCopilot, func(l lookup) bool { - return anySet(l, "COPILOT_MODEL", "COPILOT_ALLOW_ALL", "COPILOT_GITHUB_TOKEN") - }}, - {AgentDevin, func(l lookup) bool { return l.fileExists(devinMarkerPath) }}, +func buildKnownNames() map[string]string { + m := make(map[string]string, len(detectagent.KnownAgents)) + for _, name := range detectagent.KnownAgents { + m[normalize(name)] = name + } + return m } -// DetectAgent resolves the active AI agent, if any, from the current process -// environment. -func DetectAgent() (Agent, bool) { - return detectAgent(osLookup()) +func normalize(s string) string { + return strings.NewReplacer("_", "", "-", "", "/", "").Replace(strings.ToLower(s)) } -// detectAgent resolves the active AI agent, if any. +// SplitVersion splits a version-shaped suffix off a raw Harness identifier, +// canonicalising the remaining name fragment against detect-agent's known +// vocabulary when it resolves to one. It returns the identifier unchanged +// with an empty version when no version-shaped suffix is present. // -// The AI_AGENT environment variable is the explicit, highest-priority signal: -// when set it is trusted verbatim (with a couple of canonicalisations). When it -// is absent, detection falls back to per-agent environment / filesystem -// signatures. -func detectAgent(l lookup) (Agent, bool) { - if name := strings.TrimSpace(l.getenv("AI_AGENT")); name != "" { - return canonicalAgent(name), true +// '_'/'/' are ordinary characters that can end a real name on their own, so +// that split is only trusted when the fragment resolves to a known +// Harness — never a guess. '@' is never part of a Harness name (detect-agent's +// own documented convention), so it is unambiguous even for a Harness we +// don't recognise: the fragment is still split off, just left uncanonicalised. +func SplitVersion(raw string) (name string, version string) { + m := versionSuffix.FindStringSubmatch(raw) + if m == nil { + return raw, "" } - for _, s := range signatures { - if s.match(l) { - return s.agent, true + fragment := m[versionSuffix.SubexpIndex("name")] + canonical, known := knownNames[normalize(fragment)] + + if v := m[versionSuffix.SubexpIndex("version2")]; v != "" { + if known { + fragment = canonical } + return fragment, strings.ReplaceAll(v, "-", ".") } - return "", false -} - -// canonicalAgent normalises an explicitly declared AI_AGENT value onto the set -// of canonical agent names. -func canonicalAgent(name string) Agent { - if name == "github-copilot-cli" { - return AgentGitHubCopilot + if !known { + return raw, "" } - return Agent(name) + return canonical, strings.ReplaceAll(m[versionSuffix.SubexpIndex("version1")], "-", ".") } diff --git a/cliv2/internal/persona/agent/agent_test.go b/cliv2/internal/persona/agent/agent_test.go deleted file mode 100644 index f773d890cd..0000000000 --- a/cliv2/internal/persona/agent/agent_test.go +++ /dev/null @@ -1,51 +0,0 @@ -package agent - -import "testing" - -func newLookup(env map[string]string, files map[string]bool) lookup { - return lookup{ - getenv: func(k string) string { return env[k] }, - fileExists: func(p string) bool { return files[p] }, - } -} - -func TestDetectAgent_Signatures(t *testing.T) { - tests := []struct { - name string - env map[string]string - files map[string]bool - want Agent - }{ - {name: "none", want: ""}, - {name: "explicit AI_AGENT", env: map[string]string{"AI_AGENT": " windsurf "}, want: "windsurf"}, - {name: "AI_AGENT overrides signatures", env: map[string]string{"AI_AGENT": "v0", "CLAUDECODE": "1"}, want: AgentV0}, - {name: "github-copilot-cli canonicalised", env: map[string]string{"AI_AGENT": "github-copilot-cli"}, want: AgentGitHubCopilot}, - {name: "cursor", env: map[string]string{"CURSOR_TRACE_ID": "x"}, want: AgentCursor}, - {name: "cursor-cli via CURSOR_AGENT", env: map[string]string{"CURSOR_AGENT": "1"}, want: AgentCursorCLI}, - {name: "cursor-cli via host role", env: map[string]string{"CURSOR_EXTENSION_HOST_ROLE": "agent-exec"}, want: AgentCursorCLI}, - {name: "cursor-cli host role mismatch", env: map[string]string{"CURSOR_EXTENSION_HOST_ROLE": "editor"}, want: ""}, - {name: "gemini", env: map[string]string{"GEMINI_CLI": "1"}, want: AgentGemini}, - {name: "codex", env: map[string]string{"CODEX_THREAD_ID": "abc"}, want: AgentCodex}, - {name: "antigravity", env: map[string]string{"ANTIGRAVITY_AGENT": "1"}, want: AgentAntigravity}, - {name: "augment", env: map[string]string{"AUGMENT_AGENT": "1"}, want: AgentAugmentCLI}, - {name: "opencode", env: map[string]string{"OPENCODE_CLIENT": "1"}, want: AgentOpenCode}, - {name: "claude", env: map[string]string{"CLAUDECODE": "1"}, want: AgentClaude}, - {name: "cowork", env: map[string]string{"CLAUDE_CODE": "1", "CLAUDE_CODE_IS_COWORK": "1"}, want: AgentCowork}, - {name: "replit", env: map[string]string{"REPL_ID": "1"}, want: AgentReplit}, - {name: "github copilot", env: map[string]string{"COPILOT_MODEL": "gpt"}, want: AgentGitHubCopilot}, - {name: "devin via marker file", files: map[string]bool{devinMarkerPath: true}, want: AgentDevin}, - {name: "empty AI_AGENT falls through", env: map[string]string{"AI_AGENT": " ", "GEMINI_CLI": "1"}, want: AgentGemini}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, ok := detectAgent(newLookup(tt.env, tt.files)) - if (tt.want != "") != ok { - t.Fatalf("ok = %v, want %v (agent %q)", ok, tt.want != "", tt.want) - } - if got != tt.want { - t.Fatalf("agent = %q, want %q", got, tt.want) - } - }) - } -} diff --git a/cliv2/internal/persona/agent/vercel_contract_test.go b/cliv2/internal/persona/agent/vercel_contract_test.go new file mode 100644 index 0000000000..76fb38cc6d --- /dev/null +++ b/cliv2/internal/persona/agent/vercel_contract_test.go @@ -0,0 +1,187 @@ +package agent + +import ( + "os" + "strings" + "testing" + + detectagent "github.com/vercel/detect-agent" +) + +// isolateEnv wipes the entire process environment for the duration of t and +// restores it on cleanup, so ambient markers from the shell actually running +// `go test` (CLAUDECODE, CURSOR_TRACE_ID, ...) can't leak into detectagent's +// signature detection. See persona_test.go's copy for the full rationale. +func isolateEnv(t *testing.T) { + t.Helper() + saved := os.Environ() + os.Clearenv() + t.Cleanup(func() { + os.Clearenv() + for _, kv := range saved { + k, v, _ := strings.Cut(kv, "=") + // t.Setenv would register its own cleanup here, which runs + // immediately after this loop and unsets the var we just + // restored. os.Setenv writes it back for good. + //nolint:usetesting // restoring the caller's real env from within a Cleanup, not setting up a test's env + if err := os.Setenv(k, v); err != nil { + t.Errorf("failed to restore env var %q: %v", k, err) + } + } + }) +} + +// TestIsolateEnvRestoresCallerEnvironment guards against isolateEnv's cleanup +// wiping out variables it was supposed to restore: t.Setenv registers its own +// unset-cleanup, so calling it from inside isolateEnv's t.Cleanup would undo +// the restore it just performed. +func TestIsolateEnvRestoresCallerEnvironment(t *testing.T) { + t.Setenv("ISOLATE_ENV_SENTINEL", "outer") + + t.Run("inner", func(t *testing.T) { + isolateEnv(t) + t.Setenv("AI_AGENT", "whatever") + }) + + if got := os.Getenv("ISOLATE_ENV_SENTINEL"); got != "outer" { + t.Fatalf("sentinel env var not restored after isolateEnv cleanup: got %q, want %q", got, "outer") + } +} + +// TestVercelDetectDoesNotNormalizeExplicitAIAgent proves the vercel package's +// contract directly, independent of our own split/canonicalise code: an +// explicit AI_AGENT declaration is returned byte-for-byte, version and role +// still fused in, while the signature-fallback path (no AI_AGENT) returns +// vercel's own clean vocabulary name. This is why SplitVersion exists. +func TestVercelDetectDoesNotNormalizeExplicitAIAgent(t *testing.T) { + t.Run("explicit AI_AGENT is returned verbatim, not standardised", func(t *testing.T) { + isolateEnv(t) + t.Setenv("AI_AGENT", "claude-code_2-1-233_agent") + + details, err := detectagent.Detect() + if err != nil { + t.Fatalf("Detect() error = %v", err) + } + if details.Name != "claude-code_2-1-233_agent" { + t.Fatalf("Name = %q, want raw AI_AGENT value unchanged", details.Name) + } + }) + + t.Run("signature fallback returns vercel's own clean name", func(t *testing.T) { + isolateEnv(t) + t.Setenv("CLAUDECODE", "1") + + details, err := detectagent.Detect() + if err != nil { + t.Fatalf("Detect() error = %v", err) + } + if details.Name != "claude_code" { + t.Fatalf("Name = %q, want vercel's canonical %q", details.Name, "claude_code") + } + if want := KnownAgentsName("CLAUDE"); details.Name != want { + t.Fatalf("Name = %q does not match KnownAgents[%q] = %q", details.Name, "CLAUDE", want) + } + }) +} + +// KnownAgentsName is a tiny test-only accessor so the assertion above reads +// against the vocabulary directly rather than a hardcoded duplicate string. +func KnownAgentsName(key string) string { + return detectagent.KnownAgents[key] +} + +// TestSplitVersionReconcilesBothPaths feeds SplitVersion the raw, unprocessed +// output vercel actually returns for each path proven above, and shows both +// converge on the same canonical name despite vercel returning two different +// strings for the same tool. +func TestSplitVersionReconcilesBothPaths(t *testing.T) { + explicitName, explicitVersion := SplitVersion("claude-code_2-1-233_agent") + fallbackName, fallbackVersion := SplitVersion("claude_code") + + if explicitName != "claude_code" { + t.Fatalf("explicit path: name = %q, want %q", explicitName, "claude_code") + } + if explicitVersion != "2.1.233" { + t.Fatalf("explicit path: version = %q, want %q", explicitVersion, "2.1.233") + } + if fallbackName != "claude_code" { + t.Fatalf("fallback path: name = %q, want %q", fallbackName, "claude_code") + } + if fallbackVersion != "" { + t.Fatalf("fallback path: version = %q, want empty (signature path never carries one)", fallbackVersion) + } + if explicitName != fallbackName { + t.Fatalf("paths diverged: %q != %q", explicitName, fallbackName) + } +} + +// TestSplitVersion_AtSeparator covers detect-agent's own documented +// name@version convention for custom AI_AGENT declarations (see the +// "Recommended Naming Convention" section of the vercel/detect-agent +// README), which '_' and '/' alone did not handle. +func TestSplitVersion_AtSeparator(t *testing.T) { + cases := []struct { + name string + raw string + wantName string + wantVersion string + }{ + { + name: "known harness, dotted version", + raw: "devin@2.1", + wantName: "devin", + wantVersion: "2.1", + }, + { + name: "known harness, three-component version", + raw: "claude_code@2.1.233", + wantName: "claude_code", + wantVersion: "2.1.233", + }, + { + // detect-agent's README uses this exact string ("devin@1") as its + // example of the convention; '@' is never part of a Harness name, + // so the separator alone is enough signal — no minimum digit-group + // count is needed the way there is for '_'/'/'. + name: "single-component version, detect-agent's own README example", + raw: "devin@1", + wantName: "devin", + wantVersion: "1", + }, + { + // '_' and '/' remain ordinary characters that can end a real name + // on their own, so the ambiguity guard still applies to them. + name: "single-component underscore version still does not split", + raw: "devin_1", + wantName: "devin_1", + wantVersion: "", + }, + { + // Unlike the '_'/'/' cases, '@' is unambiguous on its own, so the + // split happens even for a harness detect-agent doesn't know — + // the name fragment is just left uncanonicalised. + name: "unrecognised harness still splits on '@'", + raw: "custom-agent@2.0", + wantName: "custom-agent", + wantVersion: "2.0", + }, + { + name: "unrecognised harness, single-component version", + raw: "custom-agent@2", + wantName: "custom-agent", + wantVersion: "2", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + gotName, gotVersion := SplitVersion(tc.raw) + if gotName != tc.wantName { + t.Errorf("name = %q, want %q", gotName, tc.wantName) + } + if gotVersion != tc.wantVersion { + t.Errorf("version = %q, want %q", gotVersion, tc.wantVersion) + } + }) + } +} diff --git a/cliv2/internal/persona/persona.go b/cliv2/internal/persona/persona.go index b473a0da63..114a80321e 100644 --- a/cliv2/internal/persona/persona.go +++ b/cliv2/internal/persona/persona.go @@ -12,9 +12,12 @@ const ( // keyInteractiveMode is the bitmask of which standard streams are attached to // a terminal (StdinTTY | StdoutTTY | StderrTTY). keyInteractiveMode = "persona.interactive_mode" - // keyAgent reports the canonical name of the AI coding agent driving the + // keyAgent reports the canonical name of the Harness driving the // invocation, when one is detected. keyAgent = "persona.agent" + // keyAgentVersion reports the Harness version, when one can be + // determined. Absent rather than empty when unknown. + keyAgentVersion = "persona.agent_version" ) // Report adds the persona extension values to the given analytics instance. @@ -23,7 +26,11 @@ func Report(a analytics.Analytics) { // A terminal on stdin is our signal that a human is driving the session. a.AddExtensionBoolValue(keyInteractive, mode.Has(interactive.StdinTTY)) a.AddExtensionIntegerValue(keyInteractiveMode, int(mode)) - if agent, ok := agent.DetectAgent(); ok { - a.AddExtensionStringValue(keyAgent, string(agent)) + if raw, ok := agent.DetectAgent(); ok { + name, version := agent.SplitVersion(raw) + a.AddExtensionStringValue(keyAgent, name) + if version != "" { + a.AddExtensionStringValue(keyAgentVersion, version) + } } } diff --git a/cliv2/internal/persona/persona_test.go b/cliv2/internal/persona/persona_test.go index c7b5777de6..54ef47a4e2 100644 --- a/cliv2/internal/persona/persona_test.go +++ b/cliv2/internal/persona/persona_test.go @@ -1,6 +1,8 @@ package persona import ( + "os" + "strings" "testing" "github.com/snyk/go-application-framework/pkg/analytics" @@ -30,6 +32,49 @@ func (f *fakeAnalytics) AddExtensionBoolValue(key string, value bool) { f.bool func (f *fakeAnalytics) AddExtensionIntegerValue(key string, value int) { f.integers[key] = value } func (f *fakeAnalytics) AddExtensionStringValue(key, value string) { f.strings[key] = value } +// isolateEnv wipes the entire process environment for the duration of t and +// restores it on cleanup. detect-agent's signature list (which env vars each +// Harness is recognised by) lives in a third-party vendored agents.json we +// don't own; naming those vars here would drift the moment they add one. A +// full wipe needs no such list: whatever ambient markers the shell actually +// running `go test` happens to set (CLAUDECODE, CURSOR_TRACE_ID, ...) are +// gone regardless of what detect-agent looks for, now or in the future. +func isolateEnv(t *testing.T) { + t.Helper() + saved := os.Environ() + os.Clearenv() + t.Cleanup(func() { + os.Clearenv() + for _, kv := range saved { + k, v, _ := strings.Cut(kv, "=") + // t.Setenv would register its own cleanup here, which runs + // immediately after this loop and unsets the var we just + // restored. os.Setenv writes it back for good. + //nolint:usetesting // restoring the caller's real env from within a Cleanup, not setting up a test's env + if err := os.Setenv(k, v); err != nil { + t.Errorf("failed to restore env var %q: %v", k, err) + } + } + }) +} + +// TestIsolateEnvRestoresCallerEnvironment guards against isolateEnv's cleanup +// wiping out variables it was supposed to restore: t.Setenv registers its own +// unset-cleanup, so calling it from inside isolateEnv's t.Cleanup would undo +// the restore it just performed. +func TestIsolateEnvRestoresCallerEnvironment(t *testing.T) { + t.Setenv("ISOLATE_ENV_SENTINEL", "outer") + + t.Run("inner", func(t *testing.T) { + isolateEnv(t) + t.Setenv("SOME_TEST_VAR", "whatever") + }) + + if got := os.Getenv("ISOLATE_ENV_SENTINEL"); got != "outer" { + t.Fatalf("sentinel env var not restored after isolateEnv cleanup: got %q, want %q", got, "outer") + } +} + // TestReport verifies that the public entrypoint wires the interactive and // mode signals onto the analytics instance. The per-mode and per-agent // behaviour is exercised in the interactive and agent subpackages. @@ -44,3 +89,111 @@ func TestReport(t *testing.T) { t.Fatalf("expected %q to be reported", keyInteractiveMode) } } + +// TestReport_Agent covers every real-world AI_AGENT shape observed in +// production analytics, plus the signature-detection and edge-case paths: +// the environment goes in, the recorded persona.agent / persona.agent_version +// values come out, and everything in between (detection, version split, +// canonicalisation) is free to change. +func TestReport_Agent(t *testing.T) { + cases := []struct { + name string + env map[string]string + wantAgent string + wantVersion string + }{ + { + name: "fused underscore, agent role", + env: map[string]string{"AI_AGENT": "claude-code_2-1-233_agent"}, + wantAgent: "claude_code", + wantVersion: "2.1.233", + }, + { + name: "fused underscore, harness role", + env: map[string]string{"AI_AGENT": "claude-code_2-1-229_harness"}, + wantAgent: "claude_code", + wantVersion: "2.1.229", + }, + { + // Different literal shape, same version, to prove both normalise + // to the identical bucket rather than merely similar ones. + name: "legacy slash format normalises to the same bucket", + env: map[string]string{"AI_AGENT": "claude-code/2.1.233/agent"}, + wantAgent: "claude_code", + wantVersion: "2.1.233", + }, + { + // Same canonical name as the fused AI_AGENT cases above, proving + // Claude Code converges to one bucket regardless of which + // detection path (declaration vs. signature) produced it. + name: "bare signature detection, no explicit declaration", + env: map[string]string{"AI_AGENT": "", "CLAUDECODE": "1"}, + wantAgent: "claude_code", + wantVersion: "", + }, + { + name: "harness that already groups correctly", + env: map[string]string{"AI_AGENT": "cursor-cli"}, + wantAgent: "cursor-cli", + wantVersion: "", + }, + { + name: "fused identifier with no version-shaped segment", + env: map[string]string{"AI_AGENT": "github_copilot_vscode_agent"}, + wantAgent: "github_copilot_vscode_agent", + wantVersion: "", + }, + { + name: "version-shaped but unrecognised harness", + env: map[string]string{"AI_AGENT": "mystery-tool_9-1_agent"}, + wantAgent: "mystery-tool_9-1_agent", + wantVersion: "", + }, + { + name: "redacted identifier", + env: map[string]string{"AI_AGENT": "***"}, + wantAgent: "***", + wantVersion: "", + }, + { + name: "version-shaped run with no trailing role segment", + env: map[string]string{"AI_AGENT": "claude-code_2-1-233"}, + wantAgent: "claude_code", + wantVersion: "2.1.233", + }, + { + // detect-agent's own README recommends '@' for custom AI_AGENT + // declarations (e.g. "custom-agent@2.0"); the split must honour + // that convention alongside Snyk's '_' and '/' formats. + name: "at-separated version, detect-agent's documented convention", + env: map[string]string{"AI_AGENT": "devin@2.1"}, + wantAgent: "devin", + wantVersion: "2.1", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + isolateEnv(t) + for k, v := range tc.env { + t.Setenv(k, v) + } + + a := newFakeAnalytics() + Report(a) + + if got := a.strings[keyAgent]; got != tc.wantAgent { + t.Fatalf("%s = %q, want %q", keyAgent, got, tc.wantAgent) + } + if tc.wantVersion == "" { + if v, ok := a.strings[keyAgentVersion]; ok { + t.Fatalf("expected %q to be absent, got %q", keyAgentVersion, v) + } + return + } + if got := a.strings[keyAgentVersion]; got != tc.wantVersion { + t.Fatalf("%s = %q, want %q", keyAgentVersion, got, tc.wantVersion) + } + }) + } +} diff --git a/cliv2/pkg/core/main.go b/cliv2/pkg/core/main.go index bd7350c914..1ae2ae7620 100644 --- a/cliv2/pkg/core/main.go +++ b/cliv2/pkg/core/main.go @@ -726,14 +726,15 @@ func mainWithErrorCode(additionalExts []workflow.ExtensionInit) int { func populateRedactionTerms(config configuration.Configuration, engine workflow.Engine) []string { knownTerms, _ := instrumentation.GetKnownCommandsAndFlags(engine) knownTerms = append(knownTerms, config.GetString(configuration.API_URL), config.GetString(configuration.ORGANIZATION), config.GetString(configuration.ORGANIZATION_SLUG), config.GetString(clientMachineIdConfigKey)) - // AI_AGENT is trusted verbatim into the persona.agent extension (see - // agent.canonicalAgent) for any harness not on its short canonical list, so its - // raw value needs the same exclusion as the client machine id above. - // GetUnknownParameters tokenizes its input on whitespace, so a multi-word - // value only excludes as a whole if each of its words is excluded too. + // AI_AGENT is trusted verbatim by agent.DetectAgent, and persona.Report + // falls back to that same raw value whenever the Harness name/version split + // or canonicalisation does not apply, so its raw value needs the same + // exclusion as the client machine id above. GetUnknownParameters tokenizes + // its input on whitespace, so a multi-word value only excludes as a whole if + // each of its words is excluded too. if detectedAgent, ok := agent.DetectAgent(); ok { - knownTerms = append(knownTerms, string(detectedAgent)) - knownTerms = append(knownTerms, strings.Fields(string(detectedAgent))...) + knownTerms = append(knownTerms, detectedAgent) + knownTerms = append(knownTerms, strings.Fields(detectedAgent)...) } termsToRedact := cliv2utils.GetUnknownParameters(os.Args[1:], os.Environ(), knownTerms) config.Set(logging.REDACTION_TERMS, termsToRedact) diff --git a/cliv2/scripts/prepare_licenses.go b/cliv2/scripts/prepare_licenses.go index e2011c221b..1357e0e8b2 100644 --- a/cliv2/scripts/prepare_licenses.go +++ b/cliv2/scripts/prepare_licenses.go @@ -66,6 +66,11 @@ func main() { "--save_path="+filepath.ToSlash(licensesEmbeddedDir), "--force", "--ignore", "github.com/snyk/cli/cliv2/", + // go-licenses can't find a license for this one at all (no LICENSE/ + // NOTICE/COPYING file in the repo), so it hard-fails save otherwise. + // package.json declares "license": "Apache-2.0"; the manual download + // below supplies that text directly. + "--ignore", "github.com/vercel/detect-agent", ); err != nil { log(fmt.Sprintf("Error running go-licenses save: %v", err)) os.Exit(1) @@ -79,6 +84,9 @@ func main() { {"https://raw.githubusercontent.com/alexbrainman/sspi/master/LICENSE", "github.com/alexbrainman/sspi"}, {"https://raw.githubusercontent.com/pmezard/go-difflib/master/LICENSE", "github.com/pmezard/go-difflib"}, {"https://go.dev/LICENSE?m=text", "go.dev"}, + // vercel/detect-agent has no LICENSE file upstream; its package.json + // declares "license": "Apache-2.0", so we supply that text directly. + {"https://www.apache.org/licenses/LICENSE-2.0.txt", "github.com/vercel/detect-agent"}, } for _, lic := range manualLicenses { if err := manualLicenseDownload(client, lic.url, lic.pkg); err != nil {