mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 08:02:43 +00:00
fix(orchestration): preserve authoritative model validation
This commit is contained in:
+25
-1
@@ -87,8 +87,11 @@ describe('worker launch model authority', () => {
|
||||
|
||||
expect(authority).toEqual({
|
||||
source: 'live',
|
||||
modelIds: ['opus[1m]', 'haiku', 'claude-opus-5[1m]', 'opus']
|
||||
modelIds: ['opus[1m]', 'haiku', 'claude-opus-5[1m]', 'opus', 'default']
|
||||
})
|
||||
expect(
|
||||
resolveWorkerLaunchPreferences({ agent: 'claude', model: 'default', authority }).preferences
|
||||
).toEqual({ model: 'default' })
|
||||
})
|
||||
|
||||
it('never asks an agent whose probe only extends the seed, and so refuses nothing', async () => {
|
||||
@@ -309,6 +312,27 @@ describe('worker launch model authority', () => {
|
||||
expect(second.modelIds).toContain('opus[1m]')
|
||||
})
|
||||
|
||||
it('does not reuse a catalog from a different agent command', async () => {
|
||||
const { runtime, discover } = probeRuntime(() => probeSuccess([liveModel('opus[1m]')]))
|
||||
const base = {
|
||||
catalog: CLAUDE_CATALOG,
|
||||
agent: 'claude' as const,
|
||||
runtime,
|
||||
worktreeSelector: 'id:wt_local'
|
||||
}
|
||||
|
||||
await resolveWorkerLaunchModelAuthority({ ...base, agentCommandOverride: 'claude-stable' })
|
||||
await resolveWorkerLaunchModelAuthority({ ...base, agentCommandOverride: 'claude-preview' })
|
||||
|
||||
expect(discover).toHaveBeenCalledTimes(2)
|
||||
expect(discover).toHaveBeenNthCalledWith(1, 'id:wt_local', 'claude', {
|
||||
agentCmdOverrides: { claude: 'claude-stable' }
|
||||
})
|
||||
expect(discover).toHaveBeenNthCalledWith(2, 'id:wt_local', 'claude', {
|
||||
agentCmdOverrides: { claude: 'claude-preview' }
|
||||
})
|
||||
})
|
||||
|
||||
it('shares one in-flight probe across dispatches that race it', async () => {
|
||||
let release: (() => void) | undefined
|
||||
const started = new Promise<void>((resolve) => {
|
||||
|
||||
+27
-18
@@ -7,11 +7,14 @@
|
||||
* worktree selector. Dynamic membership is combined with stable CLI aliases: a picker need not
|
||||
* display an alias such as `opus`, but the launch flag still accepts it.
|
||||
*
|
||||
* Two things answer `seed`, which claims no membership at all and so refuses nothing.
|
||||
* Several conditions answer `seed`, which claims no membership at all and so refuses nothing.
|
||||
*
|
||||
* A host that could not be listed: loss of contact is never evidence that a model does not exist
|
||||
* there (`docs/reference/ssh-execution-boundary.md`).
|
||||
*
|
||||
* A launch whose terminal-only arguments or environment are absent from the probe: that answer is
|
||||
* not evidence about the CLI invocation that will actually run.
|
||||
*
|
||||
* And an agent whose probe only EXTENDS the seed rather than replacing it — the Codex catalog is
|
||||
* explicit that its seed is short and that unknown ids must pass through, so a list that merges
|
||||
* into it cannot be read as complete. Only an agent whose discovery replaces the seed gets a
|
||||
@@ -54,9 +57,9 @@ export const SEED_WORKER_LAUNCH_MODEL_AUTHORITY: WorkerLaunchModelAuthority = {
|
||||
|
||||
type CachedModels = { expiresAt: number; models: readonly CommitMessageModelCapability[] }
|
||||
|
||||
/** Keyed by executing host, not by caller: one machine's CLI list is one fact. */
|
||||
const cachedByHost = new Map<string, CachedModels>()
|
||||
const inFlightByHost = new Map<string, Promise<readonly CommitMessageModelCapability[] | null>>()
|
||||
/** Callers that share an agent, execution host, and resolved command share one CLI fact. */
|
||||
const cachedByScope = new Map<string, CachedModels>()
|
||||
const inFlightByScope = new Map<string, Promise<readonly CommitMessageModelCapability[] | null>>()
|
||||
|
||||
function discoveredCatalogModel(model: CommitMessageModelCapability): CatalogModel {
|
||||
return {
|
||||
@@ -89,7 +92,8 @@ function liveWorkerLaunchModelAuthority(args: {
|
||||
...new Set([
|
||||
...listedIds,
|
||||
...args.models.flatMap(({ resolvedModel }) => (resolvedModel ? [resolvedModel] : [])),
|
||||
...listedAliases
|
||||
...listedAliases,
|
||||
...(args.catalog.launchModelAliases ?? [])
|
||||
])
|
||||
]
|
||||
}
|
||||
@@ -98,10 +102,13 @@ function liveWorkerLaunchModelAuthority(args: {
|
||||
async function probeHostModels(
|
||||
runtime: WorkerLaunchModelDiscoveryRuntime,
|
||||
agent: TuiAgent,
|
||||
target: WorkerLaunchModelDiscoveryTarget
|
||||
target: WorkerLaunchModelDiscoveryTarget,
|
||||
agentCommandOverride: string
|
||||
): Promise<readonly CommitMessageModelCapability[] | null> {
|
||||
try {
|
||||
const result = await runtime.discoverRuntimeCommitMessageModels(target, agent)
|
||||
const result = await runtime.discoverRuntimeCommitMessageModels(target, agent, {
|
||||
agentCmdOverrides: { [agent]: agentCommandOverride }
|
||||
})
|
||||
// `catalogOrigin: 'spec'` is the probe falling back to Orca's own list, not a CLI answer.
|
||||
return result.success && result.catalogOrigin === 'probe' && result.models.length > 0
|
||||
? result.models
|
||||
@@ -136,12 +143,12 @@ function withDiscoveryDeadline<T>(
|
||||
|
||||
function readCachedModels(scope: string): readonly CommitMessageModelCapability[] | null {
|
||||
const now = Date.now()
|
||||
for (const [key, entry] of cachedByHost) {
|
||||
for (const [key, entry] of cachedByScope) {
|
||||
if (entry.expiresAt <= now) {
|
||||
cachedByHost.delete(key)
|
||||
cachedByScope.delete(key)
|
||||
}
|
||||
}
|
||||
return cachedByHost.get(scope)?.models ?? null
|
||||
return cachedByScope.get(scope)?.models ?? null
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -151,10 +158,12 @@ function readCachedModels(scope: string): readonly CommitMessageModelCapability[
|
||||
export async function resolveWorkerLaunchModelAuthority(args: {
|
||||
catalog: AgentSessionOptionCatalog
|
||||
agent: TuiAgent
|
||||
agentCommandOverride?: string
|
||||
runtime: WorkerLaunchModelDiscoveryRuntime | null
|
||||
worktreeSelector: WorkerLaunchModelDiscoveryTarget | null
|
||||
}): Promise<WorkerLaunchModelAuthority> {
|
||||
const { catalog, agent, runtime, worktreeSelector: target } = args
|
||||
const agentCommandOverride = args.agentCommandOverride?.trim() ?? ''
|
||||
// Why: for an agent whose probe only EXTENDS the seed, the host's list is known not to be
|
||||
// exhaustive, so it can refuse nothing — and there is correspondingly nothing to ask it.
|
||||
if (!discoveredModelsReplaceSeed(agent, catalog)) {
|
||||
@@ -185,22 +194,22 @@ export async function resolveWorkerLaunchModelAuthority(args: {
|
||||
}
|
||||
return SEED_WORKER_LAUNCH_MODEL_AUTHORITY
|
||||
}
|
||||
const scope = `${agent} ${hostKey}`
|
||||
const scope = JSON.stringify([agent, hostKey, agentCommandOverride])
|
||||
const cached = readCachedModels(scope)
|
||||
if (cached) {
|
||||
return liveWorkerLaunchModelAuthority({ catalog, agent, models: cached })
|
||||
}
|
||||
let pending = inFlightByHost.get(scope)
|
||||
let pending = inFlightByScope.get(scope)
|
||||
if (!pending) {
|
||||
// Failures are never cached, so the next dispatch retries rather than inheriting a miss.
|
||||
pending = probeHostModels(runtime, agent, target).then((models) => {
|
||||
inFlightByHost.delete(scope)
|
||||
pending = probeHostModels(runtime, agent, target, agentCommandOverride).then((models) => {
|
||||
inFlightByScope.delete(scope)
|
||||
if (models) {
|
||||
cachedByHost.set(scope, { expiresAt: Date.now() + DISCOVERY_TTL_MS, models })
|
||||
cachedByScope.set(scope, { expiresAt: Date.now() + DISCOVERY_TTL_MS, models })
|
||||
}
|
||||
return models
|
||||
})
|
||||
inFlightByHost.set(scope, pending)
|
||||
inFlightByScope.set(scope, pending)
|
||||
}
|
||||
// A dispatch that gives up on the budget still leaves the probe running for the next one.
|
||||
const models = await withDiscoveryDeadline(pending, deadlineAt)
|
||||
@@ -219,6 +228,6 @@ export function describeWorkerLaunchModelRejection(args: {
|
||||
}
|
||||
|
||||
export function clearWorkerLaunchModelAuthorityCacheForTests(): void {
|
||||
cachedByHost.clear()
|
||||
inFlightByHost.clear()
|
||||
cachedByScope.clear()
|
||||
inFlightByScope.clear()
|
||||
}
|
||||
|
||||
@@ -13,7 +13,13 @@ import type { WorkerStartInput } from './worker-start-schema'
|
||||
* placement → selector mapping. A refactor that probed the worker's target worktree
|
||||
* instead of the coordinator's would still dispatch, and still pass every RPC-level test.
|
||||
*/
|
||||
function validationRuntime(): {
|
||||
function validationRuntime(
|
||||
settings: {
|
||||
agentCmdOverrides?: Record<string, string>
|
||||
agentDefaultArgs?: Record<string, string>
|
||||
agentDefaultEnv?: Record<string, Record<string, string>>
|
||||
} = {}
|
||||
): {
|
||||
runtime: OrcaRuntimeService
|
||||
discover: ReturnType<typeof vi.fn>
|
||||
resolveHostKey: ReturnType<typeof vi.fn>
|
||||
@@ -22,6 +28,7 @@ function validationRuntime(): {
|
||||
const discover = vi.fn(async () => ({ success: false, error: 'no CLI' }))
|
||||
const runtime = {
|
||||
validateOrchestrationAgentLauncher: vi.fn(),
|
||||
getClientSettings: vi.fn(() => settings),
|
||||
showTerminal: vi.fn(async () => ({ worktreeId: 'wt_coordinator' })),
|
||||
getOrchestrationDispatchAuthority: vi.fn(() => null),
|
||||
resolveRuntimeCommitMessageDiscoveryHostKey: resolveHostKey,
|
||||
@@ -166,6 +173,43 @@ describe('worker start placement to probe selector', () => {
|
||||
expect(discover).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it.each([
|
||||
{ label: 'default arguments', settings: { agentDefaultArgs: { claude: '--settings corp' } } },
|
||||
{
|
||||
label: 'default environment',
|
||||
settings: { agentDefaultEnv: { claude: { ANTHROPIC_BASE_URL: 'https://corp.invalid' } } }
|
||||
}
|
||||
])('does not reject against a probe that omits the launch $label', async ({ settings }) => {
|
||||
const { runtime, discover, resolveHostKey } = validationRuntime(settings)
|
||||
|
||||
const plan = await prepareLocalWorkerStart({
|
||||
params: localParams({ model: 'gateway-only-model' }),
|
||||
createsWorktree: false,
|
||||
runtime
|
||||
})
|
||||
|
||||
expect(plan.launch.preferences).toEqual({ model: 'gateway-only-model' })
|
||||
expect(resolveHostKey).not.toHaveBeenCalled()
|
||||
expect(discover).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('still probes through a configured command override, which discovery applies', async () => {
|
||||
const { runtime, discover, resolveHostKey } = validationRuntime({
|
||||
agentCmdOverrides: { claude: 'corp-claude' }
|
||||
})
|
||||
|
||||
await prepareLocalWorkerStart({
|
||||
params: localParams({}),
|
||||
createsWorktree: false,
|
||||
runtime
|
||||
})
|
||||
|
||||
expect(resolveHostKey).toHaveBeenCalledWith('id:wt_coordinator')
|
||||
expect(discover).toHaveBeenCalledWith('id:wt_coordinator', 'claude', {
|
||||
agentCmdOverrides: { claude: 'corp-claude' }
|
||||
})
|
||||
})
|
||||
|
||||
it('probes the remote worktree a federated attachment names', async () => {
|
||||
const { runtime, resolveHostKey } = validationRuntime()
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
discoveredModelsReplaceSeed,
|
||||
getAgentSessionOptionCatalog
|
||||
} from '../../../../../../shared/agent-session-option-catalog'
|
||||
import { hasExplicitTuiLaunchCustomization } from '../../../../../../shared/tui-agent-launch-customization'
|
||||
import type { FederationAttachStartInput } from '../federation/federation-start-schema'
|
||||
import { resolveDispatchCallerWorktreeId } from '../../orchestration-caller-workspace'
|
||||
import {
|
||||
@@ -30,6 +31,27 @@ type WorkerStartAgentPlan = {
|
||||
|
||||
const COORDINATOR_HOSTED_PLACEMENTS = new Set(['current', 'new-child', 'new-top-level'])
|
||||
|
||||
function modelProbeMissesLaunchCustomization(
|
||||
settings: ReturnType<OrcaRuntimeService['getClientSettings']>,
|
||||
agent: TuiAgent
|
||||
): boolean {
|
||||
const { agentDefaultArgs, agentDefaultEnv } = settings
|
||||
// Command overrides are part of model discovery. Terminal-only args and env are not, so their
|
||||
// catalog cannot safely reject a value the actual launch may accept.
|
||||
return hasExplicitTuiLaunchCustomization({ agentDefaultArgs, agentDefaultEnv }, agent)
|
||||
}
|
||||
|
||||
function readWorkerLaunchSettings(
|
||||
runtime: OrcaRuntimeService
|
||||
): ReturnType<OrcaRuntimeService['getClientSettings']> | null {
|
||||
try {
|
||||
return runtime.getClientSettings()
|
||||
} catch {
|
||||
// Missing settings cannot prove that discovery matches the eventual launch command.
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
function canResolveWorkerLaunchModelAuthority(
|
||||
agent: string | undefined,
|
||||
model: string | undefined
|
||||
@@ -221,6 +243,10 @@ async function resolveWorkerStartAgent(args: {
|
||||
if (!catalog || !discoveredModelsReplaceSeed(agent, catalog)) {
|
||||
return { agent, launch }
|
||||
}
|
||||
const settings = readWorkerLaunchSettings(args.runtime)
|
||||
if (!settings || modelProbeMissesLaunchCustomization(settings, agent)) {
|
||||
return { agent, launch }
|
||||
}
|
||||
return {
|
||||
agent,
|
||||
launch: resolveWorkerLaunchPreferences({
|
||||
@@ -230,6 +256,7 @@ async function resolveWorkerStartAgent(args: {
|
||||
authority: await resolveWorkerLaunchModelAuthority({
|
||||
catalog,
|
||||
agent,
|
||||
agentCommandOverride: settings.agentCmdOverrides?.[agent],
|
||||
runtime: args.runtime,
|
||||
worktreeSelector: args.worktreeSelector
|
||||
})
|
||||
|
||||
@@ -128,6 +128,8 @@ const CLAUDE_FAST_MODE: CatalogOption = {
|
||||
|
||||
export const CLAUDE_SESSION_OPTION_CATALOG: AgentSessionOptionCatalog = {
|
||||
supportsWorkerLaunchPreferences: true,
|
||||
// Why: list_models publishes `default`, but picker discovery drops that mirror row.
|
||||
launchModelAliases: ['default'],
|
||||
// Why: these ids are Claude CLI aliases that resolve to the newest model of
|
||||
// each family on the host's CLI (`opus` is Opus 5 on current CLIs, older
|
||||
// Opus on older CLIs), so pinned version labels lie on part of the fleet.
|
||||
|
||||
@@ -58,6 +58,8 @@ export type CatalogModel = {
|
||||
export type AgentSessionOptionCatalog = {
|
||||
models: CatalogModel[]
|
||||
modelApply: CatalogOptionApply
|
||||
/** Stable launch values intentionally omitted from picker membership. */
|
||||
launchModelAliases?: readonly string[]
|
||||
/** Opts this agent into structured per-worker launch overrides. */
|
||||
supportsWorkerLaunchPreferences?: true
|
||||
/** Launch-safe options for opaque model ids that are absent from the static catalog. */
|
||||
|
||||
Reference in New Issue
Block a user