From 13d2b48f33977042cbbde2f0a8dd8853006180c4 Mon Sep 17 00:00:00 2001 From: Christopher Tso Date: Sat, 4 Apr 2026 21:16:07 +0000 Subject: [PATCH 1/4] fix(core): deprecate camelCase targets aliases --- apps/cli/src/commands/eval/targets.ts | 8 +- .../core/src/evaluation/providers/targets.ts | 107 ++++++++++++++++++ .../core/src/evaluation/providers/types.ts | 2 + .../validation/targets-validator.ts | 15 ++- .../test/evaluation/providers/targets.test.ts | 28 ++++- .../validation/targets-validator.test.ts | 53 +++++++++ 6 files changed, 209 insertions(+), 4 deletions(-) diff --git a/apps/cli/src/commands/eval/targets.ts b/apps/cli/src/commands/eval/targets.ts index 3199bd339..53b1ed31c 100644 --- a/apps/cli/src/commands/eval/targets.ts +++ b/apps/cli/src/commands/eval/targets.ts @@ -198,7 +198,9 @@ export async function selectTarget(options: TargetSelectionOptions): Promise([ + ['providerBatching', 'provider_batching'], + ['subagentModeAllowed', 'subagent_mode_allowed'], + ['fallbackTargets', 'fallback_targets'], + ['resourceName', 'resource'], + ['baseUrl', 'base_url'], + ['apiKey', 'api_key'], + ['deploymentName', 'deployment'], + ['thinkingBudget', 'thinking_budget'], + ['maxTokens', 'max_output_tokens'], + ['apiFormat', 'api_format'], + ['timeoutSeconds', 'timeout_seconds'], + ['logDir', 'log_dir'], + ['logDirectory', 'log_directory'], + ['logFormat', 'log_format'], + ['logOutputFormat', 'log_output_format'], + ['systemPrompt', 'system_prompt'], + ['maxTurns', 'max_turns'], + ['maxBudgetUsd', 'max_budget_usd'], + ['dryRun', 'dry_run'], + ['subagentRoot', 'subagent_root'], + ['filesFormat', 'files_format'], + ['attachmentsFormat', 'attachments_format'], + ['cliUrl', 'cli_url'], + ['cliPath', 'cli_path'], + ['githubToken', 'github_token'], + ['sessionDir', 'session_dir'], + ['sessionId', 'session_id'], + ['sessionStateDir', 'session_state_dir'], + ['maxRetries', 'max_retries'], + ['retryInitialDelayMs', 'retry_initial_delay_ms'], + ['retryMaxDelayMs', 'retry_max_delay_ms'], + ['retryBackoffFactor', 'retry_backoff_factor'], + ['retryStatusCodes', 'retry_status_codes'], +]); + +const DEPRECATED_HEALTHCHECK_CAMEL_CASE_FIELDS = new Map([ + ['timeoutSeconds', 'timeout_seconds'], +]); + +function collectDeprecatedCamelCaseWarnings( + value: unknown, + location: string, + aliases: ReadonlyMap, +): TargetDeprecationWarning[] { + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + return []; + } + + const warnings: TargetDeprecationWarning[] = []; + for (const [camelCaseField, snakeCaseField] of aliases) { + if (Object.prototype.hasOwnProperty.call(value, camelCaseField)) { + warnings.push({ + location: `${location}.${camelCaseField}`, + message: `Deprecated camelCase field '${camelCaseField}' in targets.yaml. Use '${snakeCaseField}' instead.`, + }); + } + } + + return warnings; +} + +export function findDeprecatedCamelCaseTargetWarnings( + target: unknown, + location: string, +): readonly TargetDeprecationWarning[] { + const warnings = collectDeprecatedCamelCaseWarnings( + target, + location, + DEPRECATED_TARGET_CAMEL_CASE_FIELDS, + ); + + if (typeof target !== 'object' || target === null || Array.isArray(target)) { + return warnings; + } + + const healthcheck = (target as { healthcheck?: unknown }).healthcheck; + warnings.push( + ...collectDeprecatedCamelCaseWarnings( + healthcheck, + `${location}.healthcheck`, + DEPRECATED_HEALTHCHECK_CAMEL_CASE_FIELDS, + ), + ); + + return warnings; +} + +function emitDeprecatedCamelCaseTargetWarnings(definition: TargetDefinition): void { + for (const warning of findDeprecatedCamelCaseTargetWarnings( + definition, + `target "${definition.name}"`, + )) { + console.warn(`Warning: ${warning.message}`); + } +} + /** * Healthcheck configuration type derived from CliHealthcheckSchema. * Supports both HTTP and command-based healthchecks. @@ -797,7 +899,12 @@ export function resolveTargetDefinition( definition: TargetDefinition, env: EnvLookup = process.env, evalFilePath?: string, + options?: { readonly emitDeprecationWarnings?: boolean }, ): ResolvedTarget { + if (options?.emitDeprecationWarnings !== false) { + emitDeprecatedCamelCaseTargetWarnings(definition); + } + const parsed = BASE_TARGET_SCHEMA.parse(definition); if (parsed.workspace_template !== undefined || parsed.workspaceTemplate !== undefined) { throw new Error( diff --git a/packages/core/src/evaluation/providers/types.ts b/packages/core/src/evaluation/providers/types.ts index 774f32c07..2897b2a4d 100644 --- a/packages/core/src/evaluation/providers/types.ts +++ b/packages/core/src/evaluation/providers/types.ts @@ -293,6 +293,8 @@ export interface TargetDefinition { // Provider batching readonly provider_batching?: boolean | undefined; readonly providerBatching?: boolean | undefined; + readonly subagent_mode_allowed?: boolean | undefined; + readonly subagentModeAllowed?: boolean | undefined; // Azure fields readonly endpoint?: string | unknown | undefined; readonly base_url?: string | unknown | undefined; diff --git a/packages/core/src/evaluation/validation/targets-validator.ts b/packages/core/src/evaluation/validation/targets-validator.ts index 7e1e8299b..ac3857a0a 100644 --- a/packages/core/src/evaluation/validation/targets-validator.ts +++ b/packages/core/src/evaluation/validation/targets-validator.ts @@ -2,7 +2,11 @@ import { readFile } from 'node:fs/promises'; import path from 'node:path'; import { parse } from 'yaml'; -import { CLI_PLACEHOLDERS, COMMON_TARGET_SETTINGS } from '../providers/targets.js'; +import { + CLI_PLACEHOLDERS, + COMMON_TARGET_SETTINGS, + findDeprecatedCamelCaseTargetWarnings, +} from '../providers/targets.js'; import { KNOWN_PROVIDERS, PROVIDER_ALIASES } from '../providers/types.js'; import type { ValidationError, ValidationResult } from './types.js'; @@ -522,6 +526,15 @@ export async function validateTargetsFile(filePath: string): Promise ({ text: 'ok', reasoningText: undefined, @@ -677,6 +677,32 @@ describe('resolveTargetDefinition', () => { ).toThrow(/workspace_template has been removed/i); }); + it('warns when deprecated camelCase target fields are used', () => { + const warnSpy = spyOn(console, 'warn').mockImplementation(() => {}); + + resolveTargetDefinition( + { + name: 'deprecated-camel-case', + provider: 'openai', + baseUrl: '${{ OPENAI_BASE_URL }}', + apiKey: '${{ OPENAI_API_KEY }}', + model: '${{ OPENAI_MODEL }}', + maxTokens: 100, + }, + { + OPENAI_BASE_URL: 'https://api.openai.com/v1', + OPENAI_API_KEY: 'test-key', + OPENAI_MODEL: 'gpt-5-mini', + }, + ); + + expect(warnSpy).toHaveBeenCalledTimes(3); + expect(warnSpy.mock.calls[0]?.[0]).toContain("Deprecated camelCase field 'baseUrl'"); + expect(warnSpy.mock.calls[1]?.[0]).toContain("Deprecated camelCase field 'apiKey'"); + expect(warnSpy.mock.calls[2]?.[0]).toContain("Deprecated camelCase field 'maxTokens'"); + warnSpy.mockRestore(); + }); + it('resolves agentv target with model and default temperature', () => { const target = resolveTargetDefinition( { diff --git a/packages/core/test/evaluation/validation/targets-validator.test.ts b/packages/core/test/evaluation/validation/targets-validator.test.ts index 87d382a1f..ecbf4f529 100644 --- a/packages/core/test/evaluation/validation/targets-validator.test.ts +++ b/packages/core/test/evaluation/validation/targets-validator.test.ts @@ -87,4 +87,57 @@ describe('validateTargetsFile', () => { ), ).toBe(false); }); + + it('warns on deprecated camelCase target aliases', async () => { + const filePath = path.join(tempDir, 'camel-case-aliases.yaml'); + await writeFile( + filePath, + `targets: + - name: codex-target + provider: codex + timeoutSeconds: 30 + logDir: ./logs + systemPrompt: Be precise. + - name: cli-target + provider: cli + command: echo {PROMPT} + healthcheck: + command: echo ok + timeoutSeconds: 5 +`, + ); + + const result = await validateTargetsFile(filePath); + const warnings = result.errors.filter((error) => error.severity === 'warning'); + + expect(result.valid).toBe(true); + expect( + warnings.some( + (warning) => + warning.location === 'targets[0].timeoutSeconds' && + warning.message.includes("Use 'timeout_seconds' instead"), + ), + ).toBe(true); + expect( + warnings.some( + (warning) => + warning.location === 'targets[0].logDir' && + warning.message.includes("Use 'log_dir' instead"), + ), + ).toBe(true); + expect( + warnings.some( + (warning) => + warning.location === 'targets[0].systemPrompt' && + warning.message.includes("Use 'system_prompt' instead"), + ), + ).toBe(true); + expect( + warnings.some( + (warning) => + warning.location === 'targets[1].healthcheck.timeoutSeconds' && + warning.message.includes("Use 'timeout_seconds' instead"), + ), + ).toBe(true); + }); }); From 984b8228c779e5249790e530e13a953dfb5dd421 Mon Sep 17 00:00:00 2001 From: Christopher Tso Date: Sat, 4 Apr 2026 21:23:44 +0000 Subject: [PATCH 2/4] docs(repo): require e2e and review before PR handoff --- AGENTS.md | 2 ++ packages/core/src/evaluation/providers/targets.ts | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9e85a0790..e40a1641b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,6 +110,7 @@ cd ../agentv.worktrees/- - Subagents for: research, file exploration, running tests, code review. - For complex problems, throw more subagents at it — parallelize where possible. - Name subagents descriptively. +- Before declaring a repo change complete or opening/finalizing a PR, spawn a subagent for a final code review pass unless the user explicitly says not to. ### Autonomous Bug Fixes - When you spot a bug, just fix it. Don't ask for hand-holding. @@ -377,6 +378,7 @@ When working on a GitHub issue, **ALWAYS** follow this workflow: 6. **Before merging**, ensure: - **E2E verification completed** (see "Completing Work — E2E Checklist") + - For CLI/user-facing changes, run at least one manual end-to-end check of the actual command flow, not just unit/integration tests. - CI pipeline passes (all checks green) - Code has been reviewed if required - No merge conflicts with `main` diff --git a/packages/core/src/evaluation/providers/targets.ts b/packages/core/src/evaluation/providers/targets.ts index 127a7e55c..5c498c10b 100644 --- a/packages/core/src/evaluation/providers/targets.ts +++ b/packages/core/src/evaluation/providers/targets.ts @@ -582,10 +582,10 @@ const DEPRECATED_TARGET_CAMEL_CASE_FIELDS = new Map([ ['providerBatching', 'provider_batching'], ['subagentModeAllowed', 'subagent_mode_allowed'], ['fallbackTargets', 'fallback_targets'], - ['resourceName', 'resource'], + ['resourceName', 'endpoint'], ['baseUrl', 'base_url'], ['apiKey', 'api_key'], - ['deploymentName', 'deployment'], + ['deploymentName', 'model'], ['thinkingBudget', 'thinking_budget'], ['maxTokens', 'max_output_tokens'], ['apiFormat', 'api_format'], From 0fe9d065d2f205b9c1af3c55247a346b847e8915 Mon Sep 17 00:00:00 2001 From: Christopher Tso Date: Sat, 4 Apr 2026 21:36:23 +0000 Subject: [PATCH 3/4] docs(repo): align draft-pr verification workflow --- AGENTS.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e40a1641b..1661b32a3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -370,19 +370,24 @@ When working on a GitHub issue, **ALWAYS** follow this workflow: 4. **Implement the changes** and commit following the commit convention -5. **Push the branch and create a Pull Request**: +5. **Push regularly and open a draft Pull Request early**: ```bash git push -u origin - gh pr create --title "(scope): description" --body "Closes #" + gh pr create --draft --title "(scope): description" --body "Closes #" ``` + Push incremental commits to the draft PR as you work so progress is visible and recoverable. -6. **Before merging**, ensure: +6. **Before marking the PR ready for review or merging a low-risk change**, ensure: - **E2E verification completed** (see "Completing Work — E2E Checklist") - - For CLI/user-facing changes, run at least one manual end-to-end check of the actual command flow, not just unit/integration tests. + - For CLI or other user-facing changes, run at least one manual end-to-end check of the real user flow, not just unit/integration tests. + - A final subagent code review pass has been run and any findings addressed or called out. - CI pipeline passes (all checks green) - - Code has been reviewed if required - No merge conflicts with `main` +7. **Only after verification is complete**: + - Mark the draft PR ready for review, or + - Merge directly if the change is low risk and the repo policy allows it + The `in-progress` label stays on the issue until the PR is merged and the issue is closed. Do not remove it manually. **IMPORTANT:** Never push directly to `main`. Always use branches and PRs. From dd1d86b113b84d6ce632233f9aa44c7b856ae168 Mon Sep 17 00:00:00 2001 From: Christopher Tso Date: Sat, 4 Apr 2026 21:59:55 +0000 Subject: [PATCH 4/4] docs(repo): require local cleanup after merge --- AGENTS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 1661b32a3..9bec0b95d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -388,6 +388,11 @@ When working on a GitHub issue, **ALWAYS** follow this workflow: - Mark the draft PR ready for review, or - Merge directly if the change is low risk and the repo policy allows it +8. **After merge, clean up local state**: + - Delete the local feature branch + - Remove the local worktree created for the issue + - Confirm the primary checkout is back on an up-to-date `main` + The `in-progress` label stays on the issue until the PR is merged and the issue is closed. Do not remove it manually. **IMPORTANT:** Never push directly to `main`. Always use branches and PRs.