diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a0e6f6f..7e412fb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Changed +- Agent execution now performs a side-effect-free binary and version preflight + before each case. Host runtimes are never modified; isolated Claude Code, + Codex, and Qwen Code runtimes honor concrete `engine.version` selections, + and detected CLI versions are recorded as observed report metadata. The + preflight does not validate login state or make model requests. - Agent adapters now consume one protocol-aware model connection selected from provider configuration. OpenAI- and Anthropic-compatible endpoints under the same provider no longer pass through the legacy flattened credential view, diff --git a/docs/design/agent-configuration.md b/docs/design/agent-configuration.md index 27938ef1..045554ff 100644 --- a/docs/design/agent-configuration.md +++ b/docs/design/agent-configuration.md @@ -35,6 +35,17 @@ The engine determines the protocol. For example, `provider: dashscope` with a static and do not make a model request. The first real agent run is therefore the first validation of a delegated local login. +Before each case, skill-up runs a static runtime preflight: the adapter's +availability check followed by its `--version` command. With `environment.type: +none`, the host installation is never modified; a configured concrete version +is only validated against the host CLI. In an isolated runtime, supported npm +adapters (`claude_code`, `codex`, and `qwen_code`) install the configured +exact semantic version before the same preflight; tags and ranges are rejected. +QoderCLI's installer does not support version +selection, so `engine.version` remains unsupported there, while its detected +version is still reported. These commands do not inspect login state or make a +model request. + ## Current resolution order The runner path resolves values in these stages: @@ -44,8 +55,8 @@ The runner path resolves values in these stages: 3. Build one role-aware `ResolvedAgentConfig`. Legacy slash disambiguation is performed once at this point instead of mutating and later repairing the loaded eval config. -4. Resolve the provider-scoped `MODEL` value. A provider-scoped model - environment variable currently overrides the YAML model. +4. Resolve a provider-scoped `MODEL` value as a default when no explicit model + was selected from YAML or CLI. 5. Preserve explicit CLI `--model` and `--api-key` precedence. 6. Apply the selected adapter's declared protocol and capability contract, then resolve credentials and endpoints for that `(provider, protocol)`. @@ -66,9 +77,10 @@ CLI-mutated eval config. `result.json` retains the legacy `engine_name` and `model_name` fields while also recording credential-free `requested_configuration`, `applied_configuration`, and optional `observed_configuration` objects. Per-session results carry the requested and -applied values, capability warnings, and an observed `model` only when the -agent explicitly reports it. An absent observation remains unknown; skill-up -does not probe authentication or spend tokens to infer local CLI state. +applied values, capability warnings, an observed `model` only when the agent +explicitly reports it, and the CLI version returned by static runtime +inspection. An absent observation remains unknown; skill-up does not probe +authentication or spend tokens to infer local CLI state. An applied provider is recorded only when the adapter actively selects that provider. If Codex falls back to local provider selection, both the applied provider and model are empty and credentials resolved for the rejected provider @@ -149,12 +161,10 @@ Tagged Actions and historical commands therefore remain valid. - The legacy flattened credential lookup remains available to older internal callers, but adapter construction uses the protocol-aware connection. -- Provider-scoped `MODEL` currently overrides an explicit YAML model. -- `engine.version`, `engine.entry`, and `engine.model.params` now produce - warnings when ineffective, but their final implementation/removal is deferred - to the installation and schema phases. -- The actual installed CLI version is not yet recorded as observed runtime - configuration. +- `engine.entry` and `engine.model.params` produce warnings when ineffective; + their final implementation or removal is deferred to the schema phase. +- QoderCLI does not support selecting an installer version, so an explicit + `engine.version` is warned and ignored for that adapter. See [Issue #196](https://github.com/alibaba/skill-up/issues/196) for the staged cleanup plan. diff --git a/docs/guide/writing-evals.md b/docs/guide/writing-evals.md index 8083511d..634f0ae5 100644 --- a/docs/guide/writing-evals.md +++ b/docs/guide/writing-evals.md @@ -88,6 +88,7 @@ skills: # ========== 5. Agent Engine ========== engine: name: claude_code # claude_code / codex / qodercli (also accepts qoder-cli) / qwen_code (also accepts qwen-code, qwen) + version: 2.1.0 # Optional concrete CLI version; see version lifecycle below model: provider: anthropic name: claude-sonnet-4-6 @@ -136,6 +137,21 @@ remains a valid Skill. The same fields are supported by `judge.skills`. `cases.parallelism` is the file-level default. To override it for a single run, use `skill-up run --parallelism N` without modifying `eval.yaml`. Allowed range: **1 to 256**. +### Engine version lifecycle + +Before each case, skill-up checks that the selected CLI is installed and reads +its version with a static `--version` command. This does not validate login or +make a model request. With `environment.type: none`, skill-up never changes the +host installation; a configured concrete `engine.version` must already match. +In an isolated runtime, Claude Code, Codex, and Qwen Code install the requested +version before validation. QoderCLI versions are observed in reports, but its +installer does not support selecting `engine.version`, so an explicit value is +ignored with a warning. + +For supported adapters, `engine.version` must be one exact semantic version +(for example, `2.1.0` or `2.1.0-beta.1`); tags and ranges such as `latest` or +`^2.1.0` are rejected. + ### Engine kwargs (agent-specific switches) `engine.kwargs` is a free-form string map. Each agent reads only the keys it recognises; unknown keys are ignored. Unrecognised keys (typos like `bypas_sandbox`) emit a DEBUG log line — run with `-v` to surface them. CLI override: `--engine-kwarg key=value` (alias `--ek`), repeatable. Precedence: `--engine-kwarg` > `engine.kwargs` > default. diff --git a/docs/zh/guide/writing-evals.md b/docs/zh/guide/writing-evals.md index 7ba7bc67..0fe3e6b2 100644 --- a/docs/zh/guide/writing-evals.md +++ b/docs/zh/guide/writing-evals.md @@ -88,6 +88,7 @@ skills: # ========== 5. Agent Engine ========== engine: name: claude_code # claude_code / codex / qodercli（也兼容 qoder-cli）/ qwen_code（也兼容 qwen-code、qwen） + version: 2.1.0 # 可选的 CLI 具体版本；语义见下文 # kwargs: # edition: cn # qodercli 可选：global（默认）/ cn model: @@ -135,6 +136,17 @@ report: `cases.parallelism` 是配置文件中的默认用例并行数；临时运行时可以用 `skill-up run --parallelism N` 覆盖它，不需要修改 `eval.yaml`。命令行覆盖值必须在 1 到 256 之间。 +### Engine 版本生命周期 + +每个用例执行前，skill-up 会静态检查 CLI 是否安装，并通过 `--version` +读取版本；该过程不会验证登录状态，也不会发起模型请求。使用 +`environment.type: none` 时，skill-up 不会修改宿主机安装，配置的具体 +`engine.version` 必须与现有 CLI 匹配。在隔离 runtime 中，Claude Code、 +Codex 和 Qwen Code 会先安装指定版本再校验。QoderCLI 的实际版本仍会记录到 +报告，但其安装器不支持选择 `engine.version`，显式配置会被警告并忽略。 +对支持版本选择的 adapter，`engine.version` 必须是完整、精确的语义化版本 +（例如 `2.1.0` 或 `2.1.0-beta.1`）；`latest`、`^2.1.0` 等 tag 或范围不受支持。 + ### 采集 workspace 产物（`collect_artifacts`） `collect_artifacts` 用 glob 声明要从用例 workspace 采集的文件。每次 Agent 运行后——**无论成功、失败还是超时**——命中的文件都会被下载到： diff --git a/e2e/testdata/multiturn-session/session-engine.sh b/e2e/testdata/multiturn-session/session-engine.sh index 438a5809..1a56acd8 100755 --- a/e2e/testdata/multiturn-session/session-engine.sh +++ b/e2e/testdata/multiturn-session/session-engine.sh @@ -17,6 +17,11 @@ # otherwise the turn behaves like a brand-new conversation. set -euo pipefail +if [[ "${1:-}" == "--version" ]]; then + echo "qodercli 0.0.0-test" + exit 0 +fi + PROMPT="" RESUME="" while [[ $# -gt 0 ]]; do diff --git a/internal/agent/README.md b/internal/agent/README.md index 953608b8..22f8d3b8 100644 --- a/internal/agent/README.md +++ b/internal/agent/README.md @@ -108,6 +108,7 @@ type SessionResult struct { RequestedModel string AppliedModel string Model string // agent-reported observed model; empty means unknown + Version string // CLI version returned by static runtime inspection Warnings []string ExitCode int DurationMs int64 @@ -145,10 +146,17 @@ rejected Codex provider cannot leak its key or endpoint into local fallback. | Custom Engine | `custom` | pass through | no; use `engine.custom.http.url` | none; use `engine.custom.kwargs` | Unsupported kwargs and invalid supported-kwarg values are removed from the -adapter input after producing an actionable warning. `engine.version`, -`engine.entry`, and `engine.model.params` are also reported as currently -ineffective; version lifecycle enforcement is intentionally deferred to the -installation phase of issue #196. +adapter input after producing an actionable warning. `engine.entry` and +`engine.model.params` are also reported as currently ineffective. + +Before a case runs, `Preflight` invokes the adapter's availability check and, +for built-in CLI agents, a static `--version` command. It never checks login +state or makes a model request. A `none` runtime only validates the existing +host binary and never installs or upgrades it. Isolated runtimes install a +configured version first for Claude Code, Codex, and Qwen Code, then validate +and record the detected version. QoderCLI still reports its detected version, +but its installer cannot select a requested version, so the capability layer +warns and omits that constraint. ### SessionArtifacts diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 18743209..f26585c6 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -9,6 +9,7 @@ import ( "maps" "os" "path/filepath" + "regexp" "slices" "strings" @@ -54,7 +55,8 @@ type SessionResult struct { AppliedProvider string `json:"applied_provider,omitempty"` RequestedModel string `json:"requested_model,omitempty"` AppliedModel string `json:"applied_model,omitempty"` - Model string `json:"model,omitempty"` // Agent-reported observed model; empty when unavailable. + Model string `json:"model,omitempty"` // Agent-reported observed model; empty when unavailable. + Version string `json:"version,omitempty"` // CLI version observed by the static preflight. Warnings []string `json:"warnings,omitempty"` SessionID string `json:"session_id,omitempty"` ExitCode int `json:"exit_code"` @@ -80,6 +82,31 @@ type SessionResumer interface { RunTurn(ctx context.Context, rt Runtime, opts ExecOptions, message transcript.Message, sessionID string) (*SessionResult, error) } +// RuntimeObservation contains metadata obtained from side-effect-free runtime +// inspection. It does not include authentication or login state. +type RuntimeObservation struct { + Version string +} + +// RuntimeInspector is implemented by agents that can inspect runtime metadata +// after the executable availability check succeeds. +type RuntimeInspector interface { + InspectRuntime(ctx context.Context, rt Runtime) (RuntimeObservation, error) +} + +// Preflight checks executable availability and then collects optional static +// runtime metadata. It never probes authentication or makes a model request. +func Preflight(ctx context.Context, rt Runtime, ag Agent) (RuntimeObservation, error) { + if err := ag.Check(ctx, rt); err != nil { + return RuntimeObservation{}, err + } + inspector, ok := ag.(RuntimeInspector) + if !ok { + return RuntimeObservation{}, nil + } + return inspector.InspectRuntime(ctx, rt) +} + // SessionArtifacts holds artifacts produced during an agent session. type SessionArtifacts struct { WorkspaceDiff string `json:"workspace_diff,omitempty"` @@ -434,6 +461,23 @@ func formatAgentModel(provider, model string) string { return provider + "/" + model } +func versionedPackage(packageName, version string) string { + version = strings.TrimSpace(version) + if version == "" { + return packageName + } + return packageName + "@" + version +} + +func installedVersionGuard(binary, version string) string { + expected := normalizedConfiguredVersion(version) + if expected == "" { + return "" + } + return "if command -v " + binary + " >/dev/null 2>&1 && " + binary + + " --version 2>&1 | grep -Eq " + shellQuote(`(^|[^0-9A-Za-z])v?`+regexp.QuoteMeta(expected)+`([^0-9A-Za-z.+-]|$)`) + "; then exit 0; fi" +} + // shellQuote quotes a string for safe POSIX shell usage. Agent commands are // always composed for and executed by bash (via the Node/nvm bootstrap), so // POSIX quoting is correct even when skill-up itself runs on a Windows host. diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index eeb66435..481ec9db 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -559,6 +559,54 @@ func TestDetectAgentWithResolvedConfig_RequiresMaterialization(t *testing.T) { } } +func TestDetectAgentRejectsNonConcreteSupportedVersion(t *testing.T) { + t.Parallel() + + for _, version := range []string{"latest", "2.1", "1.2.3.4"} { + _, err := DetectAgent("codex", Config{Version: version}) + if err == nil || !strings.Contains(err.Error(), "exact semantic version") { + t.Fatalf("DetectAgent version %q error = %v, want exact semantic version error", version, err) + } + } +} + +func TestInstalledVersionGuardMatchesCompleteVersion(t *testing.T) { + t.Parallel() + + guard := installedVersionGuard("example", "1.2.3") + if guard == "" { + t.Fatal("installedVersionGuard returned an empty guard") + } + for _, test := range []struct { + name string + version string + want bool + }{ + {name: "exact", version: "1.2.3", want: true}, + {name: "v prefix", version: "v1.2.3", want: true}, + {name: "prerelease", version: "1.2.3-beta.1", want: false}, + {name: "build metadata", version: "1.2.3+build.1", want: false}, + {name: "four part", version: "1.2.3.4", want: false}, + } { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + cmd := "example() { printf '%s\\n' " + shellQuote(test.version) + "; }; " + guard + "; exit 1" + rt := &runtime.NoneRuntime{} + if err := rt.Create(context.Background()); err != nil { + t.Fatal(err) + } + defer func() { _ = rt.Close() }() + result, err := rt.Exec(context.Background(), cmd, ExecOptions{}) + if err != nil { + t.Fatalf("execute guard: %v", err) + } + if got := result.ExitCode == 0; got != test.want { + t.Fatalf("guard match for %q = %v, want %v", test.version, got, test.want) + } + }) + } +} + func TestDetectAgentWithResolvedConfig_ConsumesAppliedConnection(t *testing.T) { t.Parallel() diff --git a/internal/agent/capabilities.go b/internal/agent/capabilities.go index b75fd8a7..a4b4a4ca 100644 --- a/internal/agent/capabilities.go +++ b/internal/agent/capabilities.go @@ -41,7 +41,8 @@ const ( ) // Capabilities declares the configuration surface consumed by an adapter. -// Installation/version behavior remains intentionally deferred to issue #196 PR 5. +// Version support describes adapter selection and enforcement; static version +// observation is handled by the runtime preflight. type Capabilities struct { Protocol Protocol ModelPolicy ModelPolicy @@ -66,12 +67,13 @@ var ( func CapabilitiesForEngine(engineName string) Capabilities { switch engineName { case agentkind.ClaudeCode, agentkind.ClaudeCodeAlias: - return Capabilities{Protocol: ProtocolAnthropic, ModelPolicy: ModelPolicyPassthrough, SupportsBaseURL: true} + return Capabilities{Protocol: ProtocolAnthropic, ModelPolicy: ModelPolicyPassthrough, SupportsBaseURL: true, SupportsVersion: agentkind.SupportsVersion(engineName)} case agentkind.Codex: return Capabilities{ Protocol: ProtocolOpenAI, ModelPolicy: ModelPolicyCodexProvider, SupportsBaseURL: true, + SupportsVersion: agentkind.SupportsVersion(engineName), SupportedKwargs: slices.Clone(codexKwargs), } case agentkind.QoderCLI, agentkind.QoderAlias, agentkind.QoderCLIAlias: @@ -81,7 +83,7 @@ func CapabilitiesForEngine(engineName string) Capabilities { SupportedKwargs: slices.Clone(qoderKwargs), } case agentkind.QwenCode, agentkind.QwenCodeAlias, agentkind.QwenAlias: - return Capabilities{Protocol: ProtocolOpenAI, ModelPolicy: ModelPolicyPassthrough, SupportsBaseURL: true} + return Capabilities{Protocol: ProtocolOpenAI, ModelPolicy: ModelPolicyPassthrough, SupportsBaseURL: true, SupportsVersion: agentkind.SupportsVersion(engineName)} default: return Capabilities{ Protocol: ProtocolCustom, @@ -273,7 +275,7 @@ func validateBaseURL(params *credential.ResolvedAgentConfig, capabilities Capabi func validateDeferredFields(params *credential.ResolvedAgentConfig, capabilities Capabilities) { if params.Version != "" && !capabilities.SupportsVersion { params.Warnings = appendUniqueWarning(params.Warnings, fmt.Sprintf( - "engine %q does not yet enforce engine.version; the configured version %q is not guaranteed", + "engine %q does not support engine.version; the configured version %q is ignored", params.Engine, params.Version, )) } diff --git a/internal/agent/capabilities_test.go b/internal/agent/capabilities_test.go index bea52592..529ef392 100644 --- a/internal/agent/capabilities_test.go +++ b/internal/agent/capabilities_test.go @@ -23,20 +23,21 @@ func TestCapabilitiesForEngine(t *testing.T) { protocol Protocol modelPolicy ModelPolicy supportsBaseURL bool + supportsVersion bool kwarg string arbitraryKwargs bool }{ - {engine: "claude_code", protocol: ProtocolAnthropic, modelPolicy: ModelPolicyPassthrough, supportsBaseURL: true}, - {engine: "codex", protocol: ProtocolOpenAI, modelPolicy: ModelPolicyCodexProvider, supportsBaseURL: true, kwarg: KwargBypassSandbox}, + {engine: "claude_code", protocol: ProtocolAnthropic, modelPolicy: ModelPolicyPassthrough, supportsBaseURL: true, supportsVersion: true}, + {engine: "codex", protocol: ProtocolOpenAI, modelPolicy: ModelPolicyCodexProvider, supportsBaseURL: true, supportsVersion: true, kwarg: KwargBypassSandbox}, {engine: "qoder-cli", protocol: ProtocolQoder, modelPolicy: ModelPolicyQoderTier, kwarg: KwargEdition}, - {engine: "qwen", protocol: ProtocolOpenAI, modelPolicy: ModelPolicyPassthrough, supportsBaseURL: true}, + {engine: "qwen", protocol: ProtocolOpenAI, modelPolicy: ModelPolicyPassthrough, supportsBaseURL: true, supportsVersion: true}, {engine: "custom-agent", protocol: ProtocolCustom, modelPolicy: ModelPolicyPassthrough}, } for _, tt := range tests { t.Run(tt.engine, func(t *testing.T) { t.Parallel() got := CapabilitiesForEngine(tt.engine) - if got.Protocol != tt.protocol || got.ModelPolicy != tt.modelPolicy || got.SupportsBaseURL != tt.supportsBaseURL || got.ArbitraryKwargs != tt.arbitraryKwargs { + if got.Protocol != tt.protocol || got.ModelPolicy != tt.modelPolicy || got.SupportsBaseURL != tt.supportsBaseURL || got.SupportsVersion != tt.supportsVersion || got.ArbitraryKwargs != tt.arbitraryKwargs { t.Fatalf("CapabilitiesForEngine(%q) = %+v", tt.engine, got) } if tt.kwarg != "" && !slices.Contains(got.SupportedKwargs, tt.kwarg) { @@ -363,11 +364,14 @@ func TestResolveAdapterConfig_ValidatesExplicitSettingsWithoutAliasing(t *testin t.Fatalf("invalid or unsupported kwarg %q was not removed: %v", key, got.Kwargs) } } - for _, want := range []string{"engine.version", "engine.entry", "engine.model.params", "requires boolean", "requires positive integer", "does not support kwarg"} { + for _, want := range []string{"engine.entry", "engine.model.params", "requires boolean", "requires positive integer", "does not support kwarg"} { if !containsWarning(got.Warnings, want) { t.Fatalf("warnings = %v, want substring %q", got.Warnings, want) } } + if containsWarning(got.Warnings, "engine.version") { + t.Fatalf("warnings = %v, did not expect supported codex version warning", got.Warnings) + } if kwargs[KwargBypassSandbox] != "sensitive-invalid-value" || kwargs["typo"] != "value" { t.Fatalf("ResolveAdapterConfig mutated source kwargs: %v", kwargs) } @@ -397,6 +401,15 @@ func TestResolveAdapterConfig_QoderNormalizesUnsupportedEdition(t *testing.T) { } } +func TestResolveAdapterConfig_QoderWarnsForUnsupportedVersion(t *testing.T) { + t.Parallel() + + got := ResolveAdapterConfig(credential.ResolvedAgentConfig{Engine: "qodercli", Version: "1.2.3"}, nil) + if !containsWarning(got.Warnings, "engine.version") { + t.Fatalf("Warnings = %v, want unsupported version warning", got.Warnings) + } +} + func TestResolveAdapterConfig_CustomRejectsUnusedTopLevelSettings(t *testing.T) { t.Parallel() diff --git a/internal/agent/claude_code.go b/internal/agent/claude_code.go index d4318791..cb813134 100644 --- a/internal/agent/claude_code.go +++ b/internal/agent/claude_code.go @@ -38,6 +38,7 @@ func NewClaudeCodeAgent(cfg Config) *ClaudeCodeAgent { cfg.Name = "claude-code" } cfg.CheckCmd = "command -v claude" + cfg.VersionCmd = "claude --version" cfg.SkillPath = ".claude/skills" return &ClaudeCodeAgent{ @@ -56,7 +57,7 @@ func (a *ClaudeCodeAgent) Install(ctx context.Context, rt Runtime) error { installCmd := a.Cfg.InstallCmd if installCmd == "" { - installCmd = defaultClaudeCodeInstallCmd() + installCmd = defaultClaudeCodeInstallCmdForVersion(a.Cfg.Version) } execResult, err := rt.Exec(ctx, installCmd, opts) @@ -85,12 +86,20 @@ func buildClaudeMCPInstallCmd(server runtime.MCPServerConfig) (string, error) { } func defaultClaudeCodeInstallCmd() string { + return defaultClaudeCodeInstallCmdForVersion("") +} + +func defaultClaudeCodeInstallCmdForVersion(version string) string { lines := []string{ "set -e", - "if command -v claude >/dev/null 2>&1; then exit 0; fi", + } + if guard := installedVersionGuard("claude", version); guard != "" { + lines = append(lines, guard) + } else { + lines = append(lines, "if command -v claude >/dev/null 2>&1; then exit 0; fi") } lines = append(lines, nodeBootstrapLines(agentNodeDefaultVersion)...) - lines = append(lines, "npm install -g --include=optional "+shellQuote(claudeCodePackage)) + lines = append(lines, "npm install -g --include=optional "+shellQuote(versionedPackage(claudeCodePackage, version))) return strings.Join(lines, "\n") } diff --git a/internal/agent/claude_code_test.go b/internal/agent/claude_code_test.go index 7488ee8c..c57ab4cf 100644 --- a/internal/agent/claude_code_test.go +++ b/internal/agent/claude_code_test.go @@ -67,6 +67,17 @@ func TestClaudeCodeInstall_DefaultCommand(t *testing.T) { } } +func TestClaudeCodeInstall_ConfiguredVersion(t *testing.T) { + t.Parallel() + + cmd := defaultClaudeCodeInstallCmdForVersion("2.3.4") + for _, want := range []string{"claude --version", "'@anthropic-ai/claude-code@2.3.4'"} { + if !strings.Contains(cmd, want) { + t.Fatalf("install command missing %q:\n%s", want, cmd) + } + } +} + func TestClaudeCodeInstall_UsesDefaultCommand(t *testing.T) { t.Parallel() diff --git a/internal/agent/cli.go b/internal/agent/cli.go index 82ef26fa..b6ef6c07 100644 --- a/internal/agent/cli.go +++ b/internal/agent/cli.go @@ -11,6 +11,7 @@ import ( "text/template" "time" + "github.com/alibaba/skill-up/internal/agentkind" "github.com/alibaba/skill-up/internal/observability" "github.com/alibaba/skill-up/internal/platform" "github.com/alibaba/skill-up/internal/runtime" @@ -168,6 +169,8 @@ func (a *CLIAgent) Run(ctx context.Context, rt Runtime, opts ExecOptions, messag // the way a real shell would. var commandVRegexp = regexp.MustCompile(`^\s*command\s+-v\s+(\S+)(\s.*)?$`) +var semanticVersionRegexp = regexp.MustCompile(`(?:^|[^0-9A-Za-z.+-])v?(\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?)(?:$|[^0-9A-Za-z.+-])`) + // checkCommandForOS adapts a POSIX `command -v X` availability check to the // target OS. Windows cmd.exe has no `command` builtin; `where` is the // equivalent. Common POSIX-only redirect targets (`/dev/null`) are rewritten @@ -209,3 +212,46 @@ func (a *CLIAgent) Check(ctx context.Context, rt Runtime) error { return nil } + +// InspectRuntime executes the agent's static version command and returns the +// normalized version token. It does not inspect authentication or login state. +func (a *CLIAgent) InspectRuntime(ctx context.Context, rt Runtime) (RuntimeObservation, error) { + if a.Cfg.VersionCmd == "" { + return RuntimeObservation{}, nil + } + versionCmd := checkCommandForOS(a.Cfg.VersionCmd, rt.Shell().GOOS) + result, err := rt.Exec(ctx, versionCmd, a.mergeExecOptionsEnv(ctx, ExecOptions{}, nil, nil)) + if err != nil { + return RuntimeObservation{}, fmt.Errorf("version check failed: %w", err) + } + if result.ExitCode != 0 { + return RuntimeObservation{}, fmt.Errorf("version check failed for %s (exit %d): %s", a.Name(), result.ExitCode, result.Stderr) + } + detected := normalizeVersionOutput(result.Stdout + "\n" + result.Stderr) + if detected == "" { + if normalizedConfiguredVersion(a.Cfg.Version) != "" { + return RuntimeObservation{}, fmt.Errorf("version check for %s returned no version", a.Name()) + } + return RuntimeObservation{}, nil + } + if expected := normalizedConfiguredVersion(a.Cfg.Version); expected != "" && detected != expected { + return RuntimeObservation{}, fmt.Errorf("agent %s version mismatch: found %s, want %s", a.Name(), detected, expected) + } + return RuntimeObservation{Version: detected}, nil +} + +func normalizeVersionOutput(output string) string { + match := semanticVersionRegexp.FindStringSubmatch(output) + if len(match) < 2 || !agentkind.IsExactVersion(match[1]) { + return "" + } + return match[1] +} + +func normalizedConfiguredVersion(version string) string { + version = strings.TrimSpace(version) + if !agentkind.IsExactVersion(version) { + return "" + } + return strings.TrimPrefix(version, "v") +} diff --git a/internal/agent/cli_test.go b/internal/agent/cli_test.go index bc37cf40..412a3337 100644 --- a/internal/agent/cli_test.go +++ b/internal/agent/cli_test.go @@ -152,6 +152,115 @@ func TestCLIAgent_CheckNoCmd(t *testing.T) { } } +func TestCLIAgent_InspectRuntime(t *testing.T) { + t.Parallel() + + rt := &runtime.NoneRuntime{} + if err := rt.Create(context.Background()); err != nil { + t.Fatal(err) + } + defer func() { _ = rt.Close() }() + + ag := &CLIAgent{BaseAgent: BaseAgent{Cfg: Config{ + Name: "test-agent", + Version: "v1.2.3", + VersionCmd: "printf 'test-agent 1.2.3 (build 4)'", + }}} + observation, err := ag.InspectRuntime(context.Background(), rt) + if err != nil { + t.Fatalf("InspectRuntime failed: %v", err) + } + if observation.Version != "1.2.3" { + t.Fatalf("Version = %q, want 1.2.3", observation.Version) + } +} + +func TestNormalizeVersionOutput(t *testing.T) { + t.Parallel() + + tests := map[string]string{ + "plain": "tool 1.2.3", + "v prefix": "tool v1.2.3", + "prerelease": "tool 1.2.3-beta.1", + "build metadata": "tool 1.2.3+build.4", + "prerelease build": "tool 1.2.3-beta.1+build.4", + "four-part version": "tool 1.2.3.4", + "invalid prerelease": "tool 1.2.3-01", + } + wants := map[string]string{ + "plain": "1.2.3", + "v prefix": "1.2.3", + "prerelease": "1.2.3-beta.1", + "build metadata": "1.2.3+build.4", + "prerelease build": "1.2.3-beta.1+build.4", + } + for name, input := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + if got := normalizeVersionOutput(input); got != wants[name] { + t.Fatalf("normalizeVersionOutput(%q) = %q, want %q", input, got, wants[name]) + } + }) + } +} + +func TestCLIAgent_InspectRuntimeRejectsInvalidVersionObservation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + version string + versionCmd string + wantError string + }{ + {name: "mismatch", version: "1.2.4", versionCmd: "printf 'test-agent 1.2.3'", wantError: "version mismatch"}, + {name: "missing", version: "1.2.3", versionCmd: "printf 'unknown'", wantError: "returned no version"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + rt := &runtime.NoneRuntime{} + if err := rt.Create(context.Background()); err != nil { + t.Fatal(err) + } + defer func() { _ = rt.Close() }() + + ag := &CLIAgent{BaseAgent: BaseAgent{Cfg: Config{ + Name: "test-agent", + Version: tt.version, + VersionCmd: tt.versionCmd, + }}} + _, err := ag.InspectRuntime(context.Background(), rt) + if err == nil || !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("InspectRuntime error = %v, want %s", err, tt.wantError) + } + }) + } +} + +func TestCLIAgent_InspectRuntimeLeavesUnparseableVersionUnknownWithoutConstraint(t *testing.T) { + t.Parallel() + + rt := &runtime.NoneRuntime{} + if err := rt.Create(context.Background()); err != nil { + t.Fatal(err) + } + defer func() { _ = rt.Close() }() + + ag := &CLIAgent{BaseAgent: BaseAgent{Cfg: Config{ + Name: "test-agent", + VersionCmd: "printf 'development build'", + }}} + observation, err := ag.InspectRuntime(context.Background(), rt) + if err != nil { + t.Fatalf("InspectRuntime failed: %v", err) + } + if observation.Version != "" { + t.Fatalf("Version = %q, want unknown", observation.Version) + } +} + func TestCLIAgent_InstallSkillDefault(t *testing.T) { t.Parallel() diff --git a/internal/agent/codex.go b/internal/agent/codex.go index 473ae7e1..b811bead 100644 --- a/internal/agent/codex.go +++ b/internal/agent/codex.go @@ -94,6 +94,9 @@ func NewCodexAgent(cfg Config) *CodexAgent { if cfg.CheckCmd == "" { cfg.CheckCmd = "command -v codex" } + if cfg.VersionCmd == "" { + cfg.VersionCmd = "codex --version" + } if cfg.SkillPath == "" { cfg.SkillPath = ".codex/skills" } @@ -114,7 +117,7 @@ func (a *CodexAgent) Install(ctx context.Context, rt Runtime) error { installCmd := a.Cfg.InstallCmd if installCmd == "" { - installCmd = defaultCodexInstallCmd() + installCmd = defaultCodexInstallCmdForVersion(a.Cfg.Version) } execResult, err := rt.Exec(ctx, installCmd, opts) @@ -245,13 +248,17 @@ func buildCodexMCPRemoteBridgeScript(server runtime.MCPServerConfig) (string, er return script.String(), nil } -func defaultCodexInstallCmd() string { +func defaultCodexInstallCmdForVersion(configuredVersion string) string { + version := strings.TrimSpace(configuredVersion) + if version == "" { + version = codexDefaultVersion + } lines := []string{ "set -e", - "if command -v codex >/dev/null 2>&1 && codex --version 2>/dev/null | grep -q " + shellQuote(codexDefaultVersion) + "; then exit 0; fi", + installedVersionGuard("codex", version), } lines = append(lines, nodeBootstrapLines(agentNodeDefaultVersion)...) - lines = append(lines, "npm install -g --include=optional "+shellQuote("@openai/codex@"+codexDefaultVersion)) + lines = append(lines, "npm install -g --include=optional "+shellQuote(versionedPackage("@openai/codex", version))) return strings.Join(lines, "\n") } diff --git a/internal/agent/codex_test.go b/internal/agent/codex_test.go index 46947a35..f8196537 100644 --- a/internal/agent/codex_test.go +++ b/internal/agent/codex_test.go @@ -68,7 +68,7 @@ const ( testThreadID = "abc" ) -func TestNewCodexAgent(t *testing.T) { +func TestNewCodexAgent(t *testing.T) { //nolint:dupl // mirrors constructor defaults asserted for Qwen Code t.Parallel() ag := NewCodexAgent(Config{}) @@ -79,11 +79,23 @@ func TestNewCodexAgent(t *testing.T) { if ag.Cfg.CheckCmd != "command -v codex" { t.Fatalf("expected codex check cmd, got %s", ag.Cfg.CheckCmd) } + if ag.Cfg.VersionCmd != "codex --version" { + t.Fatalf("expected codex version cmd, got %s", ag.Cfg.VersionCmd) + } if ag.Cfg.SkillPath != ".codex/skills" { t.Fatalf("expected codex skill path, got %s", ag.Cfg.SkillPath) } } +func TestCodexInstall_ConfiguredVersion(t *testing.T) { + t.Parallel() + + cmd := defaultCodexInstallCmdForVersion("1.2.3") + if !strings.Contains(cmd, "'@openai/codex@1.2.3'") { + t.Fatalf("install command does not use configured version:\n%s", cmd) + } +} + func TestCodexCheckCredentials(t *testing.T) { t.Parallel() diff --git a/internal/agent/factory.go b/internal/agent/factory.go index 0ee4bf35..558315fd 100644 --- a/internal/agent/factory.go +++ b/internal/agent/factory.go @@ -14,6 +14,9 @@ func DetectAgent(engineName string, cfg Config) (Agent, error) { if cfg.Name == "" { cfg.Name = engineName } + if cfg.Version != "" && agentkind.SupportsVersion(engineName) && !agentkind.IsExactVersion(cfg.Version) { + return nil, fmt.Errorf("engine.version for %q must be an exact semantic version, got %q", engineName, cfg.Version) + } switch engineName { case agentkind.QoderCLIAlias, agentkind.QoderAlias, agentkind.QoderCLI: diff --git a/internal/agent/qodercli.go b/internal/agent/qodercli.go index 3adc35df..6bb18378 100644 --- a/internal/agent/qodercli.go +++ b/internal/agent/qodercli.go @@ -89,12 +89,19 @@ func qoderProfileForKwargs(kwargs map[string]string) qoderCLIProfile { // NewQoderCLIAgent creates a new QoderCLIAgent. func NewQoderCLIAgent(cfg Config) *QoderCLIAgent { profile := qoderProfileForKwargs(cfg.Kwargs) + // Qoder's installer does not accept a version selector. Keep engine.version + // as a reported unsupported request instead of enforcing a constraint that + // skill-up cannot materialize. + cfg.Version = "" if cfg.Name == "" { cfg.Name = "qodercli" } if cfg.CheckCmd == "" { cfg.CheckCmd = "command -v " + profile.binary } + if cfg.VersionCmd == "" { + cfg.VersionCmd = profile.binary + " --version" + } if cfg.RunCmd == "" { cfg.RunCmd = profile.binary + " -p \"%s\" 2>&1" } diff --git a/internal/agent/qodercli_test.go b/internal/agent/qodercli_test.go index 510b3796..e9103b71 100644 --- a/internal/agent/qodercli_test.go +++ b/internal/agent/qodercli_test.go @@ -28,6 +28,9 @@ func TestNewQoderCLIAgent(t *testing.T) { if ag.Cfg.CheckCmd != "command -v qodercli" { t.Errorf("expected CheckCmd 'command -v qodercli', got %s", ag.Cfg.CheckCmd) } + if ag.Cfg.VersionCmd != "qodercli --version" { + t.Errorf("expected VersionCmd 'qodercli --version', got %s", ag.Cfg.VersionCmd) + } if ag.Cfg.SkillPath != ".qoder/skills" { t.Errorf("expected SkillPath '.qoder/skills', got %s", ag.Cfg.SkillPath) @@ -44,6 +47,9 @@ func TestNewQoderCLIAgent_CNEdition(t *testing.T) { if ag.Cfg.CheckCmd != "command -v qodercn" { t.Fatalf("CheckCmd = %q, want qodercn", ag.Cfg.CheckCmd) } + if ag.Cfg.VersionCmd != "qodercn --version" { + t.Fatalf("VersionCmd = %q, want qodercn", ag.Cfg.VersionCmd) + } if ag.Cfg.RunCmd != `qodercn -p "%s" 2>&1` { t.Fatalf("RunCmd = %q, want qodercn command", ag.Cfg.RunCmd) } @@ -52,6 +58,18 @@ func TestNewQoderCLIAgent_CNEdition(t *testing.T) { } } +func TestNewQoderCLIAgent_DoesNotEnforceUnsupportedVersion(t *testing.T) { + t.Parallel() + + ag := NewQoderCLIAgent(Config{Version: "1.2.3"}) + if ag.Cfg.Version != "" { + t.Fatalf("Version = %q, want unsupported version constraint omitted", ag.Cfg.Version) + } + if ag.Cfg.VersionCmd != "qodercli --version" { + t.Fatalf("VersionCmd = %q, want static version observation", ag.Cfg.VersionCmd) + } +} + func TestQoderCLICheckCredentials(t *testing.T) { ag := NewQoderCLIAgent(Config{}) diff --git a/internal/agent/qwen_code.go b/internal/agent/qwen_code.go index dee54fff..fea3f5ab 100644 --- a/internal/agent/qwen_code.go +++ b/internal/agent/qwen_code.go @@ -49,6 +49,9 @@ func NewQwenCodeAgent(cfg Config) *QwenCodeAgent { if cfg.CheckCmd == "" { cfg.CheckCmd = "command -v qwen" } + if cfg.VersionCmd == "" { + cfg.VersionCmd = "qwen --version" + } if cfg.SkillPath == "" { cfg.SkillPath = ".qwen/skills" } @@ -69,7 +72,7 @@ func (a *QwenCodeAgent) Install(ctx context.Context, rt Runtime) error { installCmd := a.Cfg.InstallCmd if installCmd == "" { - installCmd = defaultQwenCodeInstallCmd() + installCmd = defaultQwenCodeInstallCmdForVersion(a.Cfg.Version) } execResult, err := rt.Exec(ctx, installCmd, opts) @@ -100,12 +103,20 @@ func buildQwenCodeMCPInstallCmd(server runtime.MCPServerConfig) (string, error) } func defaultQwenCodeInstallCmd() string { + return defaultQwenCodeInstallCmdForVersion("") +} + +func defaultQwenCodeInstallCmdForVersion(version string) string { lines := []string{ "set -e", - "if command -v qwen >/dev/null 2>&1; then exit 0; fi", + } + if guard := installedVersionGuard(qwenCodeBinary, version); guard != "" { + lines = append(lines, guard) + } else { + lines = append(lines, "if command -v qwen >/dev/null 2>&1; then exit 0; fi") } lines = append(lines, nodeBootstrapLines(agentNodeDefaultVersion)...) - lines = append(lines, "npm install -g "+shellQuote(qwenCodePackage)) + lines = append(lines, "npm install -g "+shellQuote(versionedPackage(qwenCodePackage, version))) return strings.Join(lines, "\n") } diff --git a/internal/agent/qwen_code_test.go b/internal/agent/qwen_code_test.go index 149a2895..eb5d8134 100644 --- a/internal/agent/qwen_code_test.go +++ b/internal/agent/qwen_code_test.go @@ -15,7 +15,7 @@ import ( "github.com/alibaba/skill-up/pkg/transcript" ) -func TestNewQwenCodeAgent(t *testing.T) { +func TestNewQwenCodeAgent(t *testing.T) { //nolint:dupl // mirrors constructor defaults asserted for Codex t.Parallel() ag := NewQwenCodeAgent(Config{}) @@ -26,11 +26,25 @@ func TestNewQwenCodeAgent(t *testing.T) { if ag.Cfg.CheckCmd != "command -v qwen" { t.Fatalf("expected qwen check cmd, got %s", ag.Cfg.CheckCmd) } + if ag.Cfg.VersionCmd != "qwen --version" { + t.Fatalf("expected qwen version cmd, got %s", ag.Cfg.VersionCmd) + } if ag.Cfg.SkillPath != ".qwen/skills" { t.Fatalf("expected qwen skill path, got %s", ag.Cfg.SkillPath) } } +func TestQwenCodeInstall_ConfiguredVersion(t *testing.T) { + t.Parallel() + + cmd := defaultQwenCodeInstallCmdForVersion("1.2.3") + for _, want := range []string{"qwen --version", "'@qwen-code/qwen-code@1.2.3'"} { + if !strings.Contains(cmd, want) { + t.Fatalf("install command missing %q:\n%s", want, cmd) + } + } +} + func TestQwenCodeCheckCredentials(t *testing.T) { t.Parallel() diff --git a/internal/agentkind/agentkind.go b/internal/agentkind/agentkind.go index 68f67575..f869d351 100644 --- a/internal/agentkind/agentkind.go +++ b/internal/agentkind/agentkind.go @@ -7,6 +7,11 @@ // comment. package agentkind +import ( + "regexp" + "strings" +) + // Built-in engine name constants. Aliases (with both - and _) are listed // because the YAML config and CLI flag historically accept either. const ( @@ -33,9 +38,33 @@ var builtinNames = map[string]struct{}{ QwenAlias: {}, } +const semverIdentifier = `(?:0|[1-9][0-9]*|[0-9]*[A-Za-z-][0-9A-Za-z-]*)` + +var exactVersionRegexp = regexp.MustCompile( + `^v?(?:0|[1-9][0-9]*)\.(?:0|[1-9][0-9]*)\.(?:0|[1-9][0-9]*)` + + `(?:-` + semverIdentifier + `(?:\.` + semverIdentifier + `)*)?` + + `(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$`, +) + // IsBuiltin reports whether name matches a built-in agent engine. // A built-in engine ignores any engine.custom block. func IsBuiltin(name string) bool { _, ok := builtinNames[name] return ok } + +// SupportsVersion reports whether the built-in adapter can select and enforce +// an engine version. +func SupportsVersion(name string) bool { + switch name { + case ClaudeCode, ClaudeCodeAlias, Codex, QwenCode, QwenCodeAlias, QwenAlias: + return true + default: + return false + } +} + +// IsExactVersion reports whether value is one complete semantic-version token. +func IsExactVersion(value string) bool { + return exactVersionRegexp.MatchString(strings.TrimSpace(value)) +} diff --git a/internal/agentkind/agentkind_test.go b/internal/agentkind/agentkind_test.go new file mode 100644 index 00000000..d46fafa5 --- /dev/null +++ b/internal/agentkind/agentkind_test.go @@ -0,0 +1,21 @@ +package agentkind + +import "testing" + +func TestVersionContract(t *testing.T) { + t.Parallel() + + if !SupportsVersion(Codex) || SupportsVersion(QoderCLI) { + t.Fatal("unexpected built-in version capabilities") + } + for _, version := range []string{"1.2.3", "v1.2.3", "1.2.3-beta.1+build.4"} { + if !IsExactVersion(version) { + t.Errorf("IsExactVersion(%q) = false, want true", version) + } + } + for _, version := range []string{"latest", "^1.2.3", "1.2", "1.2.3.4", "01.2.3", "1.2.3-01", "1.2.3-foo..bar"} { + if IsExactVersion(version) { + t.Errorf("IsExactVersion(%q) = true, want false", version) + } + } +} diff --git a/internal/cli/run.go b/internal/cli/run.go index 1328b4f5..b2ada59a 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -200,6 +200,9 @@ func loadAndPrepareConfig(ctx context.Context, cmd *cobra.Command, args []string if err := applyRunConfigOverrides(evalCfg, cmd); err != nil { //nolint:contextcheck // ctx accessed via cmd.Context() inside helpers return nil, nil, nil, err } + if err := config.NewValidator().ValidateEngineVersion(evalCfg.Engine); err != nil { + return nil, nil, nil, fmt.Errorf("validation failed: %w", err) + } modelFlag, _ := cmd.Flags().GetString("model") providerFlag, _ := cmd.Flags().GetString("provider") // The loader defers engine.custom env resolution and validation until the @@ -839,7 +842,7 @@ func loadFromEvalYAML(evalPath string) (string, []*config.CaseConfig, *config.Ev // blocks a filtered run. `skill-up validate` still validates the whole // suite (eval + every case) via ValidateAll. validator := config.NewValidator() - if err := validator.ValidateEvalConfig(result.Eval); err != nil { + if err := validator.ValidateEvalConfigBeforeEngineOverride(result.Eval); err != nil { return "", nil, nil, nil, fmt.Errorf("validation failed: %w", err) } diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index 12090712..ccb2f056 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -105,6 +105,73 @@ func TestRunEvalDryRunLoadsFiltersAndSkipsAgentSetup(t *testing.T) { } } +func TestRunEvalValidatesVersionAfterEngineOverride(t *testing.T) { + tests := []struct { + name string + yamlEngine string + cliEngine string + wantErr string + }{ + { + name: "supported YAML engine overridden by unsupported engine", + yamlEngine: "codex", + cliEngine: "qodercli", + }, + { + name: "unsupported YAML engine overridden by supported engine", + yamlEngine: "qodercli", + cliEngine: "codex", + wantErr: "must be an exact semantic version", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + writeAutoModeSkill(t, root) + evalsDir := filepath.Join(root, "evals") + casesDir := filepath.Join(evalsDir, "cases") + if err := os.MkdirAll(casesDir, 0o755); err != nil { + t.Fatal(err) + } + evalYAML := fmt.Sprintf(`schema_version: v1alpha1 +environment: + type: none +engine: + name: %s + version: latest +cases: + files: + - evals/cases/basic.yaml +`, tt.yamlEngine) + if err := os.WriteFile(filepath.Join(evalsDir, "eval.yaml"), []byte(evalYAML), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(casesDir, "basic.yaml"), []byte("id: basic\ninput:\n prompt: hello\n"), 0o600); err != nil { + t.Fatal(err) + } + + cmd := newRunPhaseTestCommand(t) + if err := cmd.Flags().Set("dry-run", testFlagBoolTrue); err != nil { + t.Fatal(err) + } + if err := cmd.Flags().Set("engine", tt.cliEngine); err != nil { + t.Fatal(err) + } + _, err := captureStdout(t, func() error { return runEval(cmd, []string{root}) }) + if tt.wantErr == "" { + if err != nil { + t.Fatalf("runEval() error = %v, want nil", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("runEval() error = %v, want containing %q", err, tt.wantErr) + } + }) + } +} + func TestRunEvalCLIModelSkipsSupersededYAMLModelReference(t *testing.T) { t.Setenv("MISSING_MODEL", "") root := t.TempDir() diff --git a/internal/config/validator.go b/internal/config/validator.go index 51132bee..0c44545a 100644 --- a/internal/config/validator.go +++ b/internal/config/validator.go @@ -81,6 +81,9 @@ func (v *Validator) ValidateEvalConfig(cfg *EvalConfig) error { if cfg.Engine.Name == "" { errs = append(errs, "engine.name is required") } + if cfg.Engine.Version != "" && agentkind.SupportsVersion(cfg.Engine.Name) && !agentkind.IsExactVersion(cfg.Engine.Version) { + errs = append(errs, fmt.Sprintf("engine.version for %q must be an exact semantic version, got %q", cfg.Engine.Name, cfg.Engine.Version)) + } // engine.custom validation is deferred to ResolveCustomEngineConfig, which // runs after CLI overrides settle the final engine name. @@ -108,6 +111,24 @@ func (v *Validator) ValidateEvalConfig(cfg *EvalConfig) error { return nil } +// ValidateEvalConfigBeforeEngineOverride validates suite-wide fields while +// deferring the engine-dependent version rule until a CLI override is applied. +func (v *Validator) ValidateEvalConfigBeforeEngineOverride(cfg *EvalConfig) error { + deferred := *cfg + deferred.Engine = cfg.Engine + deferred.Engine.Version = "" + return v.ValidateEvalConfig(&deferred) +} + +// ValidateEngineVersion validates engine.version after the final engine name +// is known, including any CLI engine override. +func (v *Validator) ValidateEngineVersion(engine EngineConfig) error { + if engine.Version != "" && agentkind.SupportsVersion(engine.Name) && !agentkind.IsExactVersion(engine.Version) { + return fmt.Errorf("validation errors:\n - engine.version for %q must be an exact semantic version, got %q", engine.Name, engine.Version) + } + return nil +} + // ValidateCaseConfig validates a case configuration. func (v *Validator) ValidateCaseConfig(cfg *CaseConfig) error { var errs []string diff --git a/internal/config/validator_test.go b/internal/config/validator_test.go index 4792e478..3a90a5bf 100644 --- a/internal/config/validator_test.go +++ b/internal/config/validator_test.go @@ -39,6 +39,27 @@ func TestValidator_ValidateEvalConfig(t *testing.T) { }, wantErr: false, }, + { + name: "supported engine requires exact version", + cfg: &EvalConfig{ + SchemaVersion: "v1alpha1", + Environment: Environment{Type: "none"}, + Engine: EngineConfig{Name: "codex", Version: "latest"}, + Cases: CasesConfig{Files: []string{"evals/cases/test.yaml"}}, + }, + wantErr: true, + errMsg: "must be an exact semantic version", + }, + { + name: "unsupported qoder version remains compatible", + cfg: &EvalConfig{ + SchemaVersion: "v1alpha1", + Environment: Environment{Type: "none"}, + Engine: EngineConfig{Name: "qodercli", Version: "latest"}, + Cases: CasesConfig{Files: []string{"evals/cases/test.yaml"}}, + }, + wantErr: false, + }, { name: "invalid eval-level output_matches regex", cfg: &EvalConfig{ @@ -794,6 +815,30 @@ func TestValidator_ValidateEvalConfig(t *testing.T) { } } +func TestValidator_ValidateEngineVersionAfterOverride(t *testing.T) { + t.Parallel() + + validator := NewValidator() + cfg := &EvalConfig{ + SchemaVersion: "v1alpha1", + Environment: Environment{Type: "none"}, + Engine: EngineConfig{Name: "codex", Version: "latest"}, + Cases: CasesConfig{Files: []string{"evals/cases/test.yaml"}}, + } + + if err := validator.ValidateEvalConfigBeforeEngineOverride(cfg); err != nil { + t.Fatalf("ValidateEvalConfigBeforeEngineOverride() error = %v, want deferred version validation", err) + } + if err := validator.ValidateEngineVersion(cfg.Engine); err == nil { + t.Fatal("ValidateEngineVersion() error = nil, want invalid codex version") + } + + cfg.Engine.Name = "qodercli" + if err := validator.ValidateEngineVersion(cfg.Engine); err != nil { + t.Fatalf("ValidateEngineVersion() error = %v, want unsupported qodercli version to be ignored", err) + } +} + // nolint:funlen,maintidx // table-driven test cases drive the line count; splitting hurts readability. func TestValidator_ValidateCaseConfig(t *testing.T) { t.Parallel() diff --git a/internal/evaluator/README.md b/internal/evaluator/README.md index 0bfe6588..155e04ee 100644 --- a/internal/evaluator/README.md +++ b/internal/evaluator/README.md @@ -193,7 +193,8 @@ executeCaseOnce(caseCfg, configName, overrideRT, overrideAgent) │ ├── runtime.NewRuntime + Create │ ├── setupCaseEnvironment │ │ ├── execute setup_steps -│ │ ├── inject credentials (API Key / Base URL) +│ │ ├── ag.Install (isolated runtimes only) +│ │ ├── agent.Preflight (binary check + static version observation) │ │ ├── ag.InstallMCP │ │ ├── ag.InstallSkill (with_skill only) │ │ └── fixtureRegistry.UploadAll diff --git a/internal/evaluator/evaluator.go b/internal/evaluator/evaluator.go index 482d7514..c46c3f25 100644 --- a/internal/evaluator/evaluator.go +++ b/internal/evaluator/evaluator.go @@ -396,12 +396,23 @@ func (e *defaultEvaluator) executeCaseOnce(ctx context.Context, caseCfg *config. runAgent = overrideAgent } - var rt runtime.Runtime + var ( + rt runtime.Runtime + observation agent.RuntimeObservation + ) if overrideRT != nil { rt = overrideRT + var err error + observation, err = agent.Preflight(ctx, rt, runAgent) + if err != nil { + result.Status = judge.StatusError + result.Error = fmt.Errorf("agent %s preflight failed: %w", runAgent.Name(), err) + result.Configuration = configName + return result + } } else { var err error - rt, err = e.prepareRuntimeForCase(ctx, caseCfg, configName, runAgent) + rt, observation, err = e.prepareRuntimeForCase(ctx, caseCfg, configName, runAgent) if err != nil { result.Status = judge.StatusError result.Error = err @@ -430,7 +441,9 @@ func (e *defaultEvaluator) executeCaseOnce(ctx context.Context, caseCfg *config. MaxTurns: caseMaxTurns(e.evalCfg, caseCfg), }, } - return e.executeMultiTurnCase(ctx, rt, caseCfg, configName, runAgent, agentExecOpts, startTime, judgeCfg, &result) + multiTurnResult := e.executeMultiTurnCase(ctx, rt, caseCfg, configName, runAgent, agentExecOpts, startTime, judgeCfg, &result) + multiTurnResult.Version = observation.Version + return multiTurnResult } logging.WarnContextf( ctx, @@ -471,6 +484,7 @@ func (e *defaultEvaluator) executeCaseOnce(ctx context.Context, caseCfg *config. agentSpan.End() finalizeArtifacts(sessionResult) result.SessionResult = normalizeSessionResult(sessionResult) + result.Version = observation.Version if sessionResult != nil && sessionResult.Artifacts != nil && len(sessionResult.Artifacts.GeneratedFiles) > 0 { e.ensureArtifactsInOutputDir(ctx, rt, configName, caseCfg.ID, "agent/run", agentArtifactDir, sessionResult) } @@ -1099,56 +1113,61 @@ func mergeFileContains(a, b []config.FileContainsCheck) []config.FileContainsChe return result } -func (e *defaultEvaluator) prepareRuntimeForCase(ctx context.Context, caseCfg *config.CaseConfig, configName string, ag agent.Agent) (runtime.Runtime, error) { +func (e *defaultEvaluator) prepareRuntimeForCase(ctx context.Context, caseCfg *config.CaseConfig, configName string, ag agent.Agent) (runtime.Runtime, agent.RuntimeObservation, error) { rtCfg := e.evalCfg.Environment.ToRuntimeConfig() rtCfg.Delete = e.deleteWorkspace mcpCfg, mcpEnv, err := e.provisionMCPConfigForCase(caseCfg) if err != nil { - return nil, err + return nil, agent.RuntimeObservation{}, err } rtCfg.Env = mergeEnvMaps(rtCfg.Env, mcpEnv) rt, err := runtime.NewRuntime(rtCfg) if err != nil { - return nil, fmt.Errorf("failed to create runtime: %w", err) + return nil, agent.RuntimeObservation{}, fmt.Errorf("failed to create runtime: %w", err) } if err := rt.Create(ctx); err != nil { _ = rt.Close() - return nil, fmt.Errorf("failed to create runtime workspace: %w", err) + return nil, agent.RuntimeObservation{}, fmt.Errorf("failed to create runtime workspace: %w", err) } - if err := e.setupCaseEnvironment(ctx, rt, caseCfg, configName, ag, mcpCfg); err != nil { + observation, err := e.setupCaseEnvironment(ctx, rt, caseCfg, configName, ag, mcpCfg) + if err != nil { _ = rt.Close() - return nil, fmt.Errorf("failed to setup case environment: %w", err) + return nil, agent.RuntimeObservation{}, fmt.Errorf("failed to setup case environment: %w", err) } - return rt, nil + return rt, observation, nil } -func (e *defaultEvaluator) setupCaseEnvironment(ctx context.Context, rt runtime.Runtime, caseCfg *config.CaseConfig, configName string, ag agent.Agent, mcpCfg runtime.MCPConfig) error { +func (e *defaultEvaluator) setupCaseEnvironment(ctx context.Context, rt runtime.Runtime, caseCfg *config.CaseConfig, configName string, ag agent.Agent, mcpCfg runtime.MCPConfig) (agent.RuntimeObservation, error) { //nolint:cyclop // linear setup pipeline keeps ordering and cleanup behavior explicit for i, step := range e.evalCfg.Environment.SetupSteps { result, err := rt.Exec(ctx, step.Run, runtime.ExecOptions{}) if err != nil { - return fmt.Errorf("setup step %d (%q) failed: %w", i+1, step.Run, err) + return agent.RuntimeObservation{}, fmt.Errorf("setup step %d (%q) failed: %w", i+1, step.Run, err) } if result.ExitCode != 0 { - return fmt.Errorf("setup step %d (%q) exited with code %d: %s", i+1, step.Run, result.ExitCode, result.Stderr) + return agent.RuntimeObservation{}, fmt.Errorf("setup step %d (%q) exited with code %d: %s", i+1, step.Run, result.ExitCode, result.Stderr) } } if e.evalCfg.Environment.Type != "none" { if err := ag.Install(ctx, rt); err != nil { - return fmt.Errorf("failed to install agent %s: %w", ag.Name(), err) + return agent.RuntimeObservation{}, fmt.Errorf("failed to install agent %s: %w", ag.Name(), err) } } + observation, err := agent.Preflight(ctx, rt, ag) + if err != nil { + return agent.RuntimeObservation{}, fmt.Errorf("agent %s preflight failed: %w", ag.Name(), err) + } if err := ag.InstallMCP(ctx, rt, mcpCfg); err != nil { - return fmt.Errorf("failed to install MCP servers: %w", err) + return agent.RuntimeObservation{}, fmt.Errorf("failed to install MCP servers: %w", err) } if configName != "without_skill" && e.loader != nil { for _, skillRef := range e.evalCfg.Skills { skillCfg := resolveSkillConfig(e.loader.SkillDir(), skillRef) if err := ag.InstallSkill(ctx, rt, skillCfg); err != nil { - return fmt.Errorf("failed to install skill %s: %w", skillRef.Path, err) + return agent.RuntimeObservation{}, fmt.Errorf("failed to install skill %s: %w", skillRef.Path, err) } logging.DebugContextf(ctx, "Evaluator: skill installed: %s", filepath.Base(skillCfg.Source)) } @@ -1157,11 +1176,11 @@ func (e *defaultEvaluator) setupCaseEnvironment(ctx context.Context, rt runtime. if e.fixtures != nil && e.loader != nil && e.skillDir != "" { fixtureBaseDir := e.loader.SkillDir() if err := e.fixtures.UploadAll(ctx, rt, caseCfg, e.skillDir, fixtureBaseDir); err != nil { - return fmt.Errorf("failed to upload fixtures: %w", err) + return agent.RuntimeObservation{}, fmt.Errorf("failed to upload fixtures: %w", err) } } - return nil + return observation, nil } func resolveSkillConfig(skillDir string, ref config.SkillRef) runtime.SkillConfig { diff --git a/internal/evaluator/evaluator_test.go b/internal/evaluator/evaluator_test.go index d2995f55..f3d2c2b6 100644 --- a/internal/evaluator/evaluator_test.go +++ b/internal/evaluator/evaluator_test.go @@ -38,10 +38,15 @@ type mockAgent struct { output string err error credErr error + checkErr error + inspectErr error + observation agent.RuntimeObservation skillErr error runFunc func(ctx context.Context, rt runtime.Runtime, opts agent.ExecOptions, messages []transcript.Message) (*agent.SessionResult, error) runCall atomic.Int32 credCall atomic.Int32 + checkCall atomic.Int32 + inspectCall atomic.Int32 installCall atomic.Int32 mcpCall atomic.Int32 skillCall atomic.Int32 @@ -75,7 +80,17 @@ func (m *mockAgent) InstallSkill(_ context.Context, _ runtime.Runtime, cfg runti m.mu.Unlock() return m.skillErr } -func (m *mockAgent) Check(_ context.Context, _ runtime.Runtime) error { return nil } + +func (m *mockAgent) Check(_ context.Context, _ runtime.Runtime) error { + m.checkCall.Add(1) + return m.checkErr +} + +func (m *mockAgent) InspectRuntime(_ context.Context, _ runtime.Runtime) (agent.RuntimeObservation, error) { + m.inspectCall.Add(1) + return m.observation, m.inspectErr +} + func (m *mockAgent) CheckCredentials(_ context.Context) error { m.credCall.Add(1) return m.credErr @@ -551,7 +566,7 @@ func TestSetupCaseEnvironmentRunsSetupAndInstallsAgentMCPAndSkill(t *testing.T) }, }) - err := e.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case-a"}, "with_skill", ag, runtime.MCPConfig{}) + _, err := e.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case-a"}, "with_skill", ag, runtime.MCPConfig{}) if err != nil { t.Fatalf("setupCaseEnvironment returned error: %v", err) } @@ -573,7 +588,7 @@ func TestSetupCaseEnvironmentRunsSetupAndInstallsAgentMCPAndSkill(t *testing.T) } withoutSkillAgent := &mockAgent{name: "agent"} - if err := e.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case-a"}, "without_skill", withoutSkillAgent, runtime.MCPConfig{}); err != nil { + if _, err := e.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case-a"}, "without_skill", withoutSkillAgent, runtime.MCPConfig{}); err != nil { t.Fatalf("setup without_skill returned error: %v", err) } if withoutSkillAgent.skillCall.Load() != 0 { @@ -593,7 +608,7 @@ func TestSetupCaseEnvironmentReportsSetupFailures(t *testing.T) { return runtime.ExecResult{ExitCode: 2, Stderr: "bad setup"}, nil }, } - err := e.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case-a"}, "with_skill", &mockAgent{name: "agent"}, runtime.MCPConfig{}) + _, err := e.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case-a"}, "with_skill", &mockAgent{name: "agent"}, runtime.MCPConfig{}) if err == nil || !strings.Contains(err.Error(), "bad setup") { t.Fatalf("setup error = %v, want stderr", err) } @@ -601,7 +616,7 @@ func TestSetupCaseEnvironmentReportsSetupFailures(t *testing.T) { rt.execFunc = func(context.Context, string, runtime.ExecOptions) (runtime.ExecResult, error) { return runtime.ExecResult{}, errors.New("boom") } - err = e.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case-a"}, "with_skill", &mockAgent{name: "agent"}, runtime.MCPConfig{}) + _, err = e.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case-a"}, "with_skill", &mockAgent{name: "agent"}, runtime.MCPConfig{}) if err == nil || !strings.Contains(err.Error(), "boom") { t.Fatalf("setup exec error = %v, want boom", err) } @@ -1313,8 +1328,13 @@ func TestRemoveDefaultRunSkillsBeforeJudge_RemovesOnlyDefaultTargets(t *testing. } func TestExecuteCase_NoJudge_DefaultPass(t *testing.T) { + ag := &mockAgent{ + name: "test", + output: "hello world", + observation: agent.RuntimeObservation{Version: "1.2.3"}, + } e := newTestEvaluator(EvalOptions{ - Agent: &mockAgent{name: "test", output: "hello world"}, + Agent: ag, }) caseCfg := &config.CaseConfig{ @@ -1331,6 +1351,32 @@ func TestExecuteCase_NoJudge_DefaultPass(t *testing.T) { if result.FinalMessage != "hello world" { t.Errorf("expected 'hello world', got %s", result.FinalMessage) } + if result.Version != "1.2.3" { + t.Errorf("expected observed version 1.2.3, got %q", result.Version) + } + if ag.checkCall.Load() != 1 || ag.inspectCall.Load() != 1 { + t.Errorf("preflight calls check/inspect = %d/%d, want 1/1", ag.checkCall.Load(), ag.inspectCall.Load()) + } +} + +func TestExecuteCase_PreflightFailureSkipsAgentRun(t *testing.T) { + t.Parallel() + + ag := &mockAgent{name: "test", checkErr: errors.New("not installed")} + e := newTestEvaluator(EvalOptions{Agent: ag}) + result := e.executeCase( + context.Background(), + &config.CaseConfig{ID: "case-preflight", Input: config.Input{Prompt: "hello"}}, + "with_skill", + &mockRuntime{workspace: t.TempDir()}, + nil, + ) + if result.Status != judge.StatusError || result.Error == nil || !strings.Contains(result.Error.Error(), "not installed") { + t.Fatalf("result = status %s error %v, want preflight error", result.Status, result.Error) + } + if ag.runCall.Load() != 0 { + t.Fatalf("Run calls = %d, want 0", ag.runCall.Load()) + } } func TestExecuteCase_StartsAgentRunSpanInSingleTrace(t *testing.T) { @@ -2296,12 +2342,18 @@ func TestSetupCaseEnvironmentAgentInstall(t *testing.T) { t.Fatal("NewEvaluator returned non-default evaluator") } - if err := eval.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case"}, "with_skill", ag, runtime.MCPConfig{}); err != nil { + if _, err := eval.setupCaseEnvironment(context.Background(), rt, &config.CaseConfig{ID: "case"}, "with_skill", ag, runtime.MCPConfig{}); err != nil { t.Fatalf("setupCaseEnvironment returned error: %v", err) } if got := ag.installCall.Load(); got != tt.wantInstalls { t.Fatalf("Install calls = %d, want %d", got, tt.wantInstalls) } + if got := ag.checkCall.Load(); got != 1 { + t.Fatalf("Check calls = %d, want 1", got) + } + if got := ag.inspectCall.Load(); got != 1 { + t.Fatalf("InspectRuntime calls = %d, want 1", got) + } }) } } diff --git a/internal/report/e2e_test.go b/internal/report/e2e_test.go index a3f0759c..0fa4ba09 100644 --- a/internal/report/e2e_test.go +++ b/internal/report/e2e_test.go @@ -56,12 +56,12 @@ func buildRealisticInput() Input { EngineName: "codex", ModelName: "openai/gpt-5.4", RequestedConfiguration: &AgentConfiguration{ - Role: "runner", Engine: "codex", Protocol: "openai", Provider: "openai", Model: "gpt-5.4", + Role: "runner", Engine: "codex", Protocol: "openai", Provider: "openai", Model: "gpt-5.4", Version: "1.2.3", }, AppliedConfiguration: &AgentConfiguration{ - Role: "runner", Engine: "codex", Protocol: "openai", Provider: "openai", Model: "gpt-5.4", + Role: "runner", Engine: "codex", Protocol: "openai", Provider: "openai", Model: "gpt-5.4", Version: "1.2.3", }, - ObservedConfiguration: &AgentConfiguration{Model: "gpt-5.4"}, + ObservedConfiguration: &AgentConfiguration{Model: "gpt-5.4", Version: "1.2.3"}, StartTime: start, EndTime: end, CaseResults: []CaseResult{ @@ -430,7 +430,7 @@ func TestE2E_HTMLReporter_FullPipeline(t *testing.T) { if !strings.Contains(content, "gpt-5.4") { t.Fatal("HTML should contain model name") } - for _, want := range []string{`"protocol":"openai"`, `"requested_model":"openai/gpt-5.4"`, `"applied_model":"openai/gpt-5.4"`, `"observed_model":"gpt-5.4"`} { + for _, want := range []string{`"protocol":"openai"`, `"requested_model":"openai/gpt-5.4"`, `"applied_model":"openai/gpt-5.4"`, `"observed_model":"gpt-5.4"`, `"requested_version":"1.2.3"`, `"applied_version":"1.2.3"`, `"observed_version":"1.2.3"`} { if !strings.Contains(content, want) { t.Fatalf("HTML should contain configuration field %s", want) } diff --git a/internal/report/html.go b/internal/report/html.go index 9bc69aad..e2a17228 100644 --- a/internal/report/html.go +++ b/internal/report/html.go @@ -65,6 +65,9 @@ type embeddedReportData struct { RequestedModel string `json:"requested_model"` AppliedModel string `json:"applied_model"` ObservedModel string `json:"observed_model"` + RequestedVersion string `json:"requested_version"` + AppliedVersion string `json:"applied_version"` + ObservedVersion string `json:"observed_version"` StartTime string `json:"start_time"` EvaluationWallTime string `json:"evaluation_wall_time"` AgentTokens int `json:"agent_tokens"` @@ -281,6 +284,9 @@ func (r *HTMLReporter) buildTemplateData(in Input) (htmlReportData, error) { RequestedModel: agentConfigurationModel(in.RequestedConfiguration), AppliedModel: agentConfigurationModel(in.AppliedConfiguration), ObservedModel: agentConfigurationModel(in.ObservedConfiguration), + RequestedVersion: agentConfigurationVersion(in.RequestedConfiguration), + AppliedVersion: agentConfigurationVersion(in.AppliedConfiguration), + ObservedVersion: agentConfigurationVersion(in.ObservedConfiguration), StartTime: in.StartTime.Format(time.RFC3339), EvaluationWallTime: fmt.Sprintf("%.1fs", in.TotalDuration().Seconds()), AgentTokens: in.TotalTokens, diff --git a/internal/report/markdown.go b/internal/report/markdown.go index f787c295..623299cd 100644 --- a/internal/report/markdown.go +++ b/internal/report/markdown.go @@ -62,6 +62,15 @@ func writeMarkdownHeader(sb *strings.Builder, in Input) { observed = "unknown" } fmt.Fprintf(sb, "- **Observed Model**: %s\n", markdownText(observed)) + if requestedVersion := agentConfigurationVersion(in.RequestedConfiguration); requestedVersion != "" { + fmt.Fprintf(sb, "- **Requested Version**: %s\n", markdownText(requestedVersion)) + } + if appliedVersion := agentConfigurationVersion(in.AppliedConfiguration); appliedVersion != "" { + fmt.Fprintf(sb, "- **Applied Version**: %s\n", markdownText(appliedVersion)) + } + if observedVersion := agentConfigurationVersion(in.ObservedConfiguration); observedVersion != "" { + fmt.Fprintf(sb, "- **Observed Version**: %s\n", markdownText(observedVersion)) + } sb.WriteString("\n") } diff --git a/internal/report/reporter.go b/internal/report/reporter.go index e096b41d..60f6b83f 100644 --- a/internal/report/reporter.go +++ b/internal/report/reporter.go @@ -53,6 +53,13 @@ func agentConfigurationModel(configuration *AgentConfiguration) string { return configuration.Provider + "/" + configuration.Model } +func agentConfigurationVersion(configuration *AgentConfiguration) string { + if configuration == nil { + return "" + } + return configuration.Version +} + // TotalDuration calculates the total wall-clock duration from StartTime to EndTime. // Falls back to summing individual case durations if StartTime/EndTime are not set. func (in Input) TotalDuration() time.Duration { @@ -139,11 +146,12 @@ type CaseResult struct { Error string `json:"error,omitempty"` Grading *judge.Result `json:"grading"` JudgeSkills []judge.SkillInfo `json:"judge_skills,omitempty"` - Configuration string `json:"configuration,omitempty"` // "with_skill" or "without_skill" - Prompt string `json:"prompt,omitempty"` // input prompt sent to the agent - Response string `json:"response,omitempty"` // agent final message - ObservedModel string `json:"observed_model,omitempty"` // model explicitly reported by the agent - TurnResults []CaseTurnResult `json:"turn_results,omitempty"` // per-turn outcomes; nil for single-turn + Configuration string `json:"configuration,omitempty"` // "with_skill" or "without_skill" + Prompt string `json:"prompt,omitempty"` // input prompt sent to the agent + Response string `json:"response,omitempty"` // agent final message + ObservedModel string `json:"observed_model,omitempty"` // model explicitly reported by the agent + ObservedVersion string `json:"observed_version,omitempty"` // CLI version reported by static runtime inspection + TurnResults []CaseTurnResult `json:"turn_results,omitempty"` // per-turn outcomes; nil for single-turn } // CaseTurnResult holds the outcome of a single turn for reporting purposes. diff --git a/internal/report/reporter_test.go b/internal/report/reporter_test.go index 46c2d77c..9ac7c7ac 100644 --- a/internal/report/reporter_test.go +++ b/internal/report/reporter_test.go @@ -548,11 +548,12 @@ func TestMarkdownReporter_DistinguishesRequestedAppliedAndObservedModel(t *testi path := filepath.Join(dir, "report.md") input := sampleInput() input.RequestedConfiguration = &AgentConfiguration{ - Role: "runner", Engine: "qoder-cli", Protocol: "qoder", Provider: "dashscope", Model: "qwen3.6-plus", + Role: "runner", Engine: "qoder-cli", Protocol: "qoder", Provider: "dashscope", Model: "qwen3.6-plus", Version: "1.2.3", } input.AppliedConfiguration = &AgentConfiguration{ Role: "runner", Engine: "qoder-cli", Protocol: "qoder", } + input.ObservedConfiguration = &AgentConfiguration{Version: "1.2.4"} if err := (&MarkdownReporter{OutputPath: path}).Write(context.Background(), input); err != nil { t.Fatalf("MarkdownReporter.Write failed: %v", err) } @@ -566,6 +567,8 @@ func TestMarkdownReporter_DistinguishesRequestedAppliedAndObservedModel(t *testi "- **Requested Model**: dashscope/qwen3.6-plus", "- **Applied Model**: none (delegated to local/default selection)", "- **Observed Model**: unknown", + "- **Requested Version**: 1.2.3", + "- **Observed Version**: 1.2.4", } { if !strings.Contains(content, want) { t.Fatalf("markdown missing %q:\n%s", want, content) diff --git a/internal/report/templates/report.html b/internal/report/templates/report.html index e9a5f9a5..73841c7a 100644 --- a/internal/report/templates/report.html +++ b/internal/report/templates/report.html @@ -275,6 +275,9 @@

Review Complete

else if (DATA.requested_model) meta += " | Applied model: none (local/default selection)"; else if (DATA.model_name) meta += " | Model: " + DATA.model_name; meta += " | Observed model: " + (DATA.observed_model || "unknown"); + if (DATA.requested_version) meta += " | Requested version: " + DATA.requested_version; + if (DATA.applied_version) meta += " | Applied version: " + DATA.applied_version; + if (DATA.observed_version) meta += " | Observed version: " + DATA.observed_version; meta += " | Started: " + DATA.start_time + " | Evaluation wall time: " + DATA.evaluation_wall_time; meta += " | Tested agent tokens: " + fmtInteger(DATA.agent_tokens); meta += " | Judge tokens: " + fmtInteger(DATA.judge_tokens); diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 2353db72..46b8d73d 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -579,6 +579,9 @@ func buildReportInput( Provider: resolved.AppliedProvider, Model: resolved.AppliedModel, } + if agent.CapabilitiesForEngine(resolved.Engine).SupportsVersion { + applied.Version = resolved.Version + } // Keep the legacy top-level model_name requested-value semantics for report // compatibility. Consumers that need invocation identity should use the // explicit applied_configuration object. Runtime identity is recorded only @@ -619,12 +622,11 @@ func buildReportInput( return input } -// observedRunnerConfiguration returns a run-level model only when every +// observedRunnerConfiguration returns each run-level field only when every // executed runner session explicitly reported the same non-empty value. A -// missing or mixed observation cannot safely describe the entire run. +// missing or mixed field cannot safely describe the entire run. func observedRunnerConfiguration(grouped map[string]*caseResults, caseIDs []string) *report.AgentConfiguration { - observed := "" - seen := false + var sessions []*agent.SessionResult for _, caseID := range caseIDs { results := grouped[caseID] if results == nil { @@ -634,41 +636,55 @@ func observedRunnerConfiguration(grouped map[string]*caseResults, caseIDs []stri if result == nil { continue } - seen = true - if result.SessionResult == nil || result.Model == "" { - return nil + sessions = append(sessions, result.SessionResult) + } + } + if len(sessions) == 0 { + return nil + } + unanimous := func(value func(*agent.SessionResult) string) string { + observed := "" + for _, session := range sessions { + if session == nil || value(session) == "" { + return "" } if observed == "" { - observed = result.Model + observed = value(session) continue } - if result.Model != observed { - return nil + if value(session) != observed { + return "" } } + return observed + } + configuration := &report.AgentConfiguration{ + Model: unanimous(func(session *agent.SessionResult) string { return session.Model }), + Version: unanimous(func(session *agent.SessionResult) string { return session.Version }), } - if !seen || observed == "" { + if configuration.Model == "" && configuration.Version == "" { return nil } - return &report.AgentConfiguration{Model: observed} + return configuration } func evalResultToCaseResult(res *evaluator.EvalResult) report.CaseResult { cr := report.CaseResult{ - CaseID: res.CaseID, - Title: res.CaseName, - Status: res.Status, - DurationMs: res.DurationMs, - Turns: res.Turns, - InputTokens: res.InputTokens, - OutputTokens: res.OutputTokens, - Grading: res.Grading, - JudgeSkills: res.JudgeSkills, - Configuration: res.Configuration, - Prompt: res.Prompt, - Response: responseContent(res), - ObservedModel: res.Model, - TurnResults: evalTurnResultsToReport(res.TurnResults), + CaseID: res.CaseID, + Title: res.CaseName, + Status: res.Status, + DurationMs: res.DurationMs, + Turns: res.Turns, + InputTokens: res.InputTokens, + OutputTokens: res.OutputTokens, + Grading: res.Grading, + JudgeSkills: res.JudgeSkills, + Configuration: res.Configuration, + Prompt: res.Prompt, + Response: responseContent(res), + ObservedModel: res.Model, + ObservedVersion: res.Version, + TurnResults: evalTurnResultsToReport(res.TurnResults), } judgeSession := res.JudgeSession if judgeSession == nil && res.Grading != nil { diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index e6b27ee7..aeaaec6c 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -17,6 +17,8 @@ import ( "github.com/alibaba/skill-up/internal/report" ) +const testObservedVersion = "1.2.3" + func TestNewRunner(t *testing.T) { evalCfg := &config.EvalConfig{ Judge: config.JudgeConfig{Type: "rule_based"}, @@ -436,6 +438,7 @@ func TestBuildReportInput_UsesResolvedRunnerIdentity(t *testing.T) { } resolved := credential.ResolvedAgentConfig{ Engine: "codex", + Version: testObservedVersion, Provider: "dashscope", Model: "qwen3.6-plus", BaseURL: "https://dashscope.example.com/v1", @@ -451,6 +454,9 @@ func TestBuildReportInput_UsesResolvedRunnerIdentity(t *testing.T) { if input.RequestedConfiguration.Model != "qwen3.6-plus" || input.AppliedConfiguration.Model != "qwen3.6-plus" || input.AppliedConfiguration.Protocol != "openai" { t.Fatalf("report requested/applied configuration = %+v / %+v", input.RequestedConfiguration, input.AppliedConfiguration) } + if input.RequestedConfiguration.Version != testObservedVersion || input.AppliedConfiguration.Version != testObservedVersion { + t.Fatalf("report requested/applied versions = %q / %q", input.RequestedConfiguration.Version, input.AppliedConfiguration.Version) + } if input.ObservedConfiguration != nil { t.Fatalf("ObservedConfiguration = %+v, want nil without agent-reported model", input.ObservedConfiguration) } @@ -484,21 +490,38 @@ func TestBuildReportInput_ObservesModelOnlyWhenEverySessionAgrees(t *testing.T) evalCfg := &config.EvalConfig{Engine: config.EngineConfig{Name: "custom"}} resolved := credential.ResolvedAgentConfig{Role: credential.AgentRoleRunner, Engine: "custom"} grouped := map[string]*caseResults{ - "one": {withSkill: &evaluator.EvalResult{SessionResult: &agent.SessionResult{Model: "reported-model"}}}, - "two": {withSkill: &evaluator.EvalResult{SessionResult: &agent.SessionResult{Model: "reported-model"}}}, + "one": {withSkill: &evaluator.EvalResult{SessionResult: &agent.SessionResult{Model: "reported-model", Version: testObservedVersion}}}, + "two": {withSkill: &evaluator.EvalResult{SessionResult: &agent.SessionResult{Model: "reported-model", Version: testObservedVersion}}}, } input := buildReportInput("s", grouped, []string{"one", "two"}, time.Time{}, time.Time{}, evalCfg, resolved) - if input.ObservedConfiguration == nil || input.ObservedConfiguration.Model != "reported-model" { - t.Fatalf("ObservedConfiguration = %+v, want unanimous reported model", input.ObservedConfiguration) + if input.ObservedConfiguration == nil || input.ObservedConfiguration.Model != "reported-model" || input.ObservedConfiguration.Version != testObservedVersion { + t.Fatalf("ObservedConfiguration = %+v, want unanimous reported model and version", input.ObservedConfiguration) } - if len(input.CaseResults) != 2 || input.CaseResults[0].ObservedModel != "reported-model" { + if len(input.CaseResults) != 2 || input.CaseResults[0].ObservedModel != "reported-model" || input.CaseResults[0].ObservedVersion != testObservedVersion { t.Fatalf("case observations = %+v", input.CaseResults) } grouped["two"].withSkill.Model = "" input = buildReportInput("s", grouped, []string{"one", "two"}, time.Time{}, time.Time{}, evalCfg, resolved) + if input.ObservedConfiguration == nil || input.ObservedConfiguration.Model != "" || input.ObservedConfiguration.Version != testObservedVersion { + t.Fatalf("ObservedConfiguration = %+v, want only unanimous version", input.ObservedConfiguration) + } + + grouped["two"].withSkill.Version = "" + input = buildReportInput("s", grouped, []string{"one", "two"}, time.Time{}, time.Time{}, evalCfg, resolved) if input.ObservedConfiguration != nil { - t.Fatalf("ObservedConfiguration = %+v, want nil for partial observation", input.ObservedConfiguration) + t.Fatalf("ObservedConfiguration = %+v, want nil for partial model and version observations", input.ObservedConfiguration) + } +} + +func TestBuildReportInput_DoesNotApplyUnsupportedVersion(t *testing.T) { + t.Parallel() + + evalCfg := &config.EvalConfig{Engine: config.EngineConfig{Name: "qodercli"}} + resolved := credential.ResolvedAgentConfig{Role: credential.AgentRoleRunner, Engine: "qodercli", Version: testObservedVersion} + input := buildReportInput("s", map[string]*caseResults{}, nil, time.Time{}, time.Time{}, evalCfg, resolved) + if input.RequestedConfiguration.Version != testObservedVersion || input.AppliedConfiguration.Version != "" { + t.Fatalf("report requested/applied versions = %q / %q", input.RequestedConfiguration.Version, input.AppliedConfiguration.Version) } }