diff --git a/internal/cli/evalmeasure_test.go b/internal/cli/evalmeasure_test.go index b3e1321a9e..fad628a186 100644 --- a/internal/cli/evalmeasure_test.go +++ b/internal/cli/evalmeasure_test.go @@ -70,7 +70,7 @@ func TestEvalMeasureCmd_MissingRequiredFlags(t *testing.T) { require.Error(t, err) } -func TestEvalMeasureCmd_OutputDirIgnoresNestedTelemetry(t *testing.T) { +func TestEvalMeasureCmd_OutputDirIgnoresNestedTelemetry_LegacyFormat(t *testing.T) { fsDir := t.TempDir() outBase := t.TempDir() runDir := filepath.Join(outBase, "agent-triage-1-1") @@ -108,10 +108,11 @@ func TestEvalMeasureCmd_OutputDirIgnoresNestedTelemetry(t *testing.T) { assert.NotContains(t, string(b), "ffffffffffffffffffffffffffffffff") } -// TestEvalMeasureCmd_LocalFullsendDirManifestProducesJSONL locks the -// resolved-manifest path used before agents@v0 carries stock YAML: a local -// FULLSEND_DIR eval/measurements/.yaml must produce eval-measurements.jsonl. -func TestEvalMeasureCmd_LocalFullsendDirManifestProducesJSONL(t *testing.T) { +// TestEvalMeasureCmd_LocalFullsendDirManifestProducesJSONL_LegacyFormat +// locks the resolved-manifest path used before agents@v0 carries stock +// YAML: a local FULLSEND_DIR eval/measurements/.yaml must produce +// eval-measurements.jsonl. +func TestEvalMeasureCmd_LocalFullsendDirManifestProducesJSONL_LegacyFormat(t *testing.T) { fsDir := t.TempDir() outBase := t.TempDir() runDir := filepath.Join(outBase, "agent-triage-2-2") @@ -145,6 +146,78 @@ func TestEvalMeasureCmd_LocalFullsendDirManifestProducesJSONL(t *testing.T) { assert.Contains(t, buf.String(), "Wrote") } +func TestEvalMeasureCmd_OutputDirIgnoresNestedTelemetry_NewFormat(t *testing.T) { + fsDir := t.TempDir() + outBase := t.TempDir() + runDir := filepath.Join(outBase, "fs-tri-aabbccddee00") + nested := filepath.Join(runDir, "iteration-1", "output") + require.NoError(t, os.MkdirAll(nested, 0o755)) + + good, err := os.ReadFile(filepath.Join("..", "evalmeasure", "testdata", "complete.jsonl")) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(runDir, telemetry.TelemetryFile), good, 0o644)) + // Agent-planted copy: valid JSONL with a different trace id would produce + // a second row if scored. Nested path must be ignored. + planted := bytes.ReplaceAll(good, []byte("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"), []byte("ffffffffffffffffffffffffffffffff")) + require.NoError(t, os.WriteFile(filepath.Join(nested, telemetry.TelemetryFile), planted, 0o644)) + + reg, err := os.ReadFile(filepath.Join("..", "evalmeasure", "testdata", "sample-registry.yaml")) + require.NoError(t, err) + require.NoError(t, os.MkdirAll(filepath.Join(fsDir, "eval", "measurements"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fsDir, "eval", "measurements", "triage.yaml"), reg, 0o644)) + + cmd := newRootCmd() + buf := &bytes.Buffer{} + cmd.SetOut(buf) + cmd.SetErr(buf) + cmd.SetArgs([]string{ + "eval-measure", + "--agent", "triage", + "--fullsend-dir", fsDir, + "--output-dir", outBase, + }) + require.NoError(t, cmd.Execute()) + + b, err := os.ReadFile(filepath.Join(runDir, evalmeasure.MeasurementsFile)) + require.NoError(t, err) + assert.Contains(t, string(b), `"trace_id":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"`) + assert.NotContains(t, string(b), "ffffffffffffffffffffffffffffffff") +} + +func TestEvalMeasureCmd_LocalFullsendDirManifestProducesJSONL_NewFormat(t *testing.T) { + fsDir := t.TempDir() + outBase := t.TempDir() + runDir := filepath.Join(outBase, "fs-tri-1122334455ff") + require.NoError(t, os.MkdirAll(runDir, 0o755)) + + good, err := os.ReadFile(filepath.Join("..", "evalmeasure", "testdata", "complete.jsonl")) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(runDir, telemetry.TelemetryFile), good, 0o644)) + + reg, err := os.ReadFile(filepath.Join("..", "evalmeasure", "testdata", "sample-registry.yaml")) + require.NoError(t, err) + require.NoError(t, os.MkdirAll(filepath.Join(fsDir, "eval", "measurements"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fsDir, "eval", "measurements", "triage.yaml"), reg, 0o644)) + + cmd := newRootCmd() + buf := &bytes.Buffer{} + cmd.SetOut(buf) + cmd.SetErr(buf) + cmd.SetArgs([]string{ + "eval-measure", + "--agent", "triage", + "--fullsend-dir", fsDir, + "--output-dir", outBase, + "--offline", + }) + require.NoError(t, cmd.Execute()) + + b, err := os.ReadFile(filepath.Join(runDir, evalmeasure.MeasurementsFile)) + require.NoError(t, err) + assert.Contains(t, string(b), `"name":"trace_fitness"`) + assert.Contains(t, buf.String(), "Wrote") +} + func writeTwoTraceTelemetry(t *testing.T) string { t.Helper() src, err := os.ReadFile(filepath.Join("..", "evalmeasure", "testdata", "complete.jsonl")) diff --git a/internal/cli/run.go b/internal/cli/run.go index 3e4f7eb053..bbf169a54a 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -3,6 +3,8 @@ package cli import ( "bytes" "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -17,6 +19,7 @@ import ( "sort" "strings" "sync" + "sync/atomic" "time" "unicode/utf8" @@ -68,6 +71,10 @@ const ( // harness dynamically. defaultAgentsRepoOwner = "fullsend-ai" defaultAgentsRepoName = "agents" + + // maxSandboxNameLen is the maximum length of an OpenShell sandbox name. + // OpenShell enforces this at creation time. + maxSandboxNameLen = 19 ) // preflightCheckTimeout bounds the execution time for a validation_loop @@ -899,9 +906,10 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep workItemID := resolveWorkItemID() // 3. Create run directory and initialise tracer. - // Lowercase the agent segment so eval-measure --agent (also lowercased) - // matches the host runDir even when the CLI arg was mixed-case. - sandboxName := fmt.Sprintf("agent-%s-%d-%d", strings.ToLower(agentName), os.Getpid(), time.Now().Unix()) + sandboxName := generateSandboxName(agentName) + if len(sandboxName) > maxSandboxNameLen { + return fmt.Errorf("sandbox name %q is %d characters, exceeding the OpenShell limit of %d", sandboxName, len(sandboxName), maxSandboxNameLen) + } if outputBase == "" { outputBase = filepath.Join(os.TempDir(), "fullsend") } @@ -3329,6 +3337,49 @@ func injectTraceID(sandboxName, traceID string) error { return err } +// sandboxNameSeq is a monotonic counter appended to sandbox name hash +// inputs, ensuring uniqueness even when PID and wall-clock are identical +// (e.g., on coarse-clock VMs or within tight loops in tests). +var sandboxNameSeq atomic.Uint64 + +// generateSandboxName produces a unique sandbox name that fits within the +// OpenShell maximum of maxSandboxNameLen (19) characters. It embeds a +// truncated agent-name slug for debuggability (visible in logs, output dirs, +// and --keep-sandbox hints), then hashes the PID, nanosecond timestamp, and a +// monotonic counter to produce a collision-resistant identifier in the form +// "fs--" (19 characters total). +func generateSandboxName(agentName string) string { + slug := agentSlug(agentName) + seq := sandboxNameSeq.Add(1) + h := sha256.Sum256([]byte(fmt.Sprintf("%d-%d-%d", os.Getpid(), time.Now().UnixNano(), seq))) + hashLen := maxSandboxNameLen - len("fs-") - len(slug) - 1 // 1 for the dash after slug + return fmt.Sprintf("fs-%s-%s", slug, hex.EncodeToString(h[:])[:hashLen]) +} + +// agentSlug returns the first 3 lowercase alphanumeric characters of the +// agent name for embedding in sandbox names. Returns "unk" for empty or +// non-alphanumeric names. +func agentSlug(name string) string { + const slugLen = 3 + var slug []byte + for _, r := range strings.ToLower(name) { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') { + slug = append(slug, byte(r)) + if len(slug) == slugLen { + return string(slug) + } + } + } + if len(slug) == 0 { + return "unk" + } + // Pad short names by repeating the last character. + for len(slug) < slugLen { + slug = append(slug, slug[len(slug)-1]) + } + return string(slug) +} + // applySandboxImageOverride replaces image with the FULLSEND_SANDBOX_IMAGE env // var value when set. Returns the resolved image and whether an override was applied. func applySandboxImageOverride(image string) (string, bool) { diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index fba9cf0a60..b64c401dcc 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -5453,3 +5453,50 @@ func TestForceRemoveAll_NonExistent(t *testing.T) { // Removing a path that does not exist should succeed (same as os.RemoveAll). require.NoError(t, forceRemoveAll(filepath.Join(t.TempDir(), "does-not-exist"))) } + +func TestGenerateSandboxName_Length(t *testing.T) { + name := generateSandboxName("triage") + assert.LessOrEqual(t, len(name), maxSandboxNameLen, + "sandbox name %q (%d chars) exceeds %d-char OpenShell limit", + name, len(name), maxSandboxNameLen) +} + +func TestGenerateSandboxName_Prefix(t *testing.T) { + name := generateSandboxName("triage") + assert.True(t, strings.HasPrefix(name, "fs-tri-"), + "sandbox name %q should start with fs-tri- prefix", name) +} + +func TestGenerateSandboxName_Uniqueness(t *testing.T) { + seen := make(map[string]struct{}) + for range 50 { + name := generateSandboxName("code") + assert.LessOrEqual(t, len(name), maxSandboxNameLen) + _, dup := seen[name] + assert.False(t, dup, "duplicate sandbox name: %q", name) + seen[name] = struct{}{} + } +} + +func TestGenerateSandboxName_AgentSlug(t *testing.T) { + tests := []struct { + name string + agent string + prefix string + }{ + {"triage", "triage", "fs-tri-"}, + {"code", "code", "fs-cod-"}, + {"review", "review", "fs-rev-"}, + {"empty", "", "fs-unk-"}, + {"short_name", "ab", "fs-abb-"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + name := generateSandboxName(tt.agent) + assert.True(t, strings.HasPrefix(name, tt.prefix), + "generateSandboxName(%q) = %q, want prefix %q", tt.agent, name, tt.prefix) + assert.Equal(t, maxSandboxNameLen, len(name), + "sandbox name %q should be exactly %d chars", name, maxSandboxNameLen) + }) + } +} diff --git a/internal/evalmeasure/find.go b/internal/evalmeasure/find.go index 2ff007f9c9..274fc9d297 100644 --- a/internal/evalmeasure/find.go +++ b/internal/evalmeasure/find.go @@ -13,20 +13,34 @@ import ( // iteration-N/output/ are agent-writable and must not be scored. const PlatformTelemetryFile = "run-telemetry.jsonl" -// hostRunDirPattern matches agent--- from fullsend run. +// hostRunDirPattern matches the legacy agent--- format. // name is lowercased when the sandbox is created; charset matches run // (ToLower only), so pid/unix must be the trailing numeric pair. var hostRunDirPattern = regexp.MustCompile(`^agent-(.+)-([0-9]+)-([0-9]+)$`) +// newHostRunDirPattern matches the current fs-- format where +// slug is a 3-character lowercase alphanumeric abbreviation of the agent +// name. Both patterns are checked so existing run directories from before +// the naming change are still discovered. +var newHostRunDirPattern = regexp.MustCompile(`^fs-([a-z0-9]{3})-([0-9a-f]+)$`) + // FindPlatformTelemetry returns run-telemetry.jsonl files that sit at the // top of outputDir itself (when outputDir is a runDir) or at the top of // a host-created child runDir (when outputDir is the CI output base). // -// Host runDirs are named agent--- (see fullsend run). -// When agent is non-empty, only children matching that exact shape for -// the lowercased agent name are considered (so agent-code does not match -// agent-code-review-…). If several match, only the newest platform file -// is scored — leftover sibling directories from a previous job are ignored. +// Host runDirs use one of two naming schemes: +// - Legacy: agent--- (full agent name in the directory) +// - Current: fs-- (3-char agent slug + hash) +// +// When agent is non-empty, only children whose embedded agent name (legacy) +// or slug (current) matches are considered. For the current scheme, the +// slug is a lossy 3-character abbreviation — agents with the same prefix +// (e.g. "review" and "reverse" both map to "rev") are indistinguishable +// from the directory name alone. In practice this is acceptable because a +// single CI job runs one agent at a time. +// +// If several match, only the newest platform file is scored — leftover +// sibling directories from a previous job are ignored. // // Matching child runDirs outrank a root-level run-telemetry.jsonl under // outputDir so an agent-planted file at the CI output base cannot displace @@ -52,8 +66,9 @@ func FindPlatformTelemetry(outputDir, agent string) ([]string, error) { return nil, nil } -// findChildPlatformTelemetry looks for agent--- children. -// sawMatch is true when at least one directory matched the pattern (and +// findChildPlatformTelemetry looks for child directories matching either the +// legacy agent--- or current fs-- naming scheme. +// sawMatch is true when at least one directory matched a pattern (and // agent filter), whether or not PlatformTelemetryFile was present. func findChildPlatformTelemetry(outputDir, wantAgent string) (paths []string, sawMatch bool, err error) { entries, err := os.ReadDir(outputDir) @@ -63,6 +78,7 @@ func findChildPlatformTelemetry(outputDir, wantAgent string) (paths []string, sa } return nil, false, err } + wantSlug := agentSlug(wantAgent) var bestPath string var bestMod time.Time foundFile := false @@ -70,11 +86,7 @@ func findChildPlatformTelemetry(outputDir, wantAgent string) (paths []string, sa if !e.IsDir() { continue } - m := hostRunDirPattern.FindStringSubmatch(e.Name()) - if m == nil { - continue - } - if wantAgent != "" && m[1] != wantAgent { + if !matchesRunDir(e.Name(), wantAgent, wantSlug) { continue } sawMatch = true @@ -94,3 +106,45 @@ func findChildPlatformTelemetry(outputDir, wantAgent string) (paths []string, sa } return []string{bestPath}, sawMatch, nil } + +// matchesRunDir reports whether dirName matches either the legacy +// agent--- or current fs-- naming scheme, +// optionally filtering by agent name. +func matchesRunDir(dirName, wantAgent, wantSlug string) bool { + // Try legacy format first. + if m := hostRunDirPattern.FindStringSubmatch(dirName); m != nil { + return wantAgent == "" || m[1] == wantAgent + } + // Try current format. + if m := newHostRunDirPattern.FindStringSubmatch(dirName); m != nil { + return wantAgent == "" || m[1] == wantSlug + } + return false +} + +// agentSlug returns the first 3 lowercase alphanumeric characters of the +// agent name. Returns "" for empty names (no filtering). This mirrors the +// slug derivation in internal/cli.agentSlug but avoids a cross-package +// dependency — the two must stay in sync. +func agentSlug(name string) string { + if name == "" { + return "" + } + const slugLen = 3 + var slug []byte + for _, r := range strings.ToLower(name) { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') { + slug = append(slug, byte(r)) + if len(slug) == slugLen { + return string(slug) + } + } + } + if len(slug) == 0 { + return "unk" + } + for len(slug) < slugLen { + slug = append(slug, slug[len(slug)-1]) + } + return string(slug) +} diff --git a/internal/evalmeasure/find_test.go b/internal/evalmeasure/find_test.go index d9ceeb9bfb..1c65075f31 100644 --- a/internal/evalmeasure/find_test.go +++ b/internal/evalmeasure/find_test.go @@ -155,3 +155,103 @@ func TestFindPlatformTelemetry_EmptyMatchingRunDirIgnoresPlantedRoot(t *testing. require.NoError(t, err) assert.Empty(t, got, "matching empty runDir must not fall back to planted root") } + +// --- Tests for the current fs-- naming scheme --- + +func TestFindPlatformTelemetry_NewFormatRunDir(t *testing.T) { + t.Parallel() + outputDir := t.TempDir() + runDir := filepath.Join(outputDir, "fs-rev-a1b2c3d4e5f6") + require.NoError(t, os.MkdirAll(runDir, 0o755)) + platform := filepath.Join(runDir, PlatformTelemetryFile) + require.NoError(t, os.WriteFile(platform, []byte("platform\n"), 0o644)) + + got, err := FindPlatformTelemetry(outputDir, "review") + require.NoError(t, err) + require.Equal(t, []string{platform}, got) +} + +func TestFindPlatformTelemetry_NewFormatNoAgentFilter(t *testing.T) { + t.Parallel() + outputDir := t.TempDir() + runDir := filepath.Join(outputDir, "fs-tri-deadbeef1234") + require.NoError(t, os.MkdirAll(runDir, 0o755)) + platform := filepath.Join(runDir, PlatformTelemetryFile) + require.NoError(t, os.WriteFile(platform, []byte("platform\n"), 0o644)) + + got, err := FindPlatformTelemetry(outputDir, "") + require.NoError(t, err) + require.Equal(t, []string{platform}, got) +} + +func TestFindPlatformTelemetry_NewFormatAgentMismatch(t *testing.T) { + t.Parallel() + outputDir := t.TempDir() + runDir := filepath.Join(outputDir, "fs-cod-abcdef012345") + require.NoError(t, os.MkdirAll(runDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(runDir, PlatformTelemetryFile), []byte("x\n"), 0o644)) + + got, err := FindPlatformTelemetry(outputDir, "triage") + require.NoError(t, err) + assert.Empty(t, got, "fs-cod-* must not match agent=triage") +} + +func TestFindPlatformTelemetry_MixedOldAndNewFormat(t *testing.T) { + t.Parallel() + outputDir := t.TempDir() + // Legacy dir — older mod time. + oldDir := filepath.Join(outputDir, "agent-triage-1-1") + require.NoError(t, os.MkdirAll(oldDir, 0o755)) + oldFile := filepath.Join(oldDir, PlatformTelemetryFile) + require.NoError(t, os.WriteFile(oldFile, []byte("old\n"), 0o644)) + + // New-format dir — newer mod time. + newDir := filepath.Join(outputDir, "fs-tri-aabbccddee00") + require.NoError(t, os.MkdirAll(newDir, 0o755)) + newFile := filepath.Join(newDir, PlatformTelemetryFile) + require.NoError(t, os.WriteFile(newFile, []byte("new\n"), 0o644)) + + // Without agent filter: newest file wins. + got, err := FindPlatformTelemetry(outputDir, "") + require.NoError(t, err) + require.Len(t, got, 1) + + // With agent filter: both formats match "triage" (legacy full name, + // new slug "tri"); newest wins. + gotAgent, err := FindPlatformTelemetry(outputDir, "triage") + require.NoError(t, err) + require.Len(t, gotAgent, 1) +} + +func TestFindPlatformTelemetry_NewFormatIgnoresNestedCopy(t *testing.T) { + t.Parallel() + outputDir := t.TempDir() + runDir := filepath.Join(outputDir, "fs-rev-1234567890ab") + nested := filepath.Join(runDir, "iteration-1", "output") + require.NoError(t, os.MkdirAll(nested, 0o755)) + + platform := filepath.Join(runDir, PlatformTelemetryFile) + planted := filepath.Join(nested, PlatformTelemetryFile) + require.NoError(t, os.WriteFile(platform, []byte("platform\n"), 0o644)) + require.NoError(t, os.WriteFile(planted, []byte("planted\n"), 0o644)) + + got, err := FindPlatformTelemetry(outputDir, "") + require.NoError(t, err) + require.Equal(t, []string{platform}, got) +} + +func TestFindPlatformTelemetry_NewFormatPrefersRunDirOverPlantedRoot(t *testing.T) { + t.Parallel() + outputDir := t.TempDir() + planted := filepath.Join(outputDir, PlatformTelemetryFile) + require.NoError(t, os.WriteFile(planted, []byte("planted\n"), 0o644)) + + runDir := filepath.Join(outputDir, "fs-rev-ffeeddccbbaa") + require.NoError(t, os.MkdirAll(runDir, 0o755)) + platform := filepath.Join(runDir, PlatformTelemetryFile) + require.NoError(t, os.WriteFile(platform, []byte("platform\n"), 0o644)) + + got, err := FindPlatformTelemetry(outputDir, "review") + require.NoError(t, err) + require.Equal(t, []string{platform}, got) +}