Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions server/services/agentProviderResolution.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,30 @@ async function resolvePublicReviewAgentProvider(task, posture) {
providerId: provider.id,
});
}
// A model pin only survives when it was chosen FOR this provider; otherwise
// fall back to the provider's own default rather than handing one vendor's
// model id to another (the failure mode documented in the ordinary path).
// A model pin only survives when it was chosen FOR this provider AND that
// provider still offers it; otherwise fall back to the provider's own default
// rather than handing one vendor's model id to another (the failure mode
// documented in the ordinary path). Matching the provider alone is not
// enough: a stage pin outlives edits to that provider's own `models` list,
// and a retired id reaches the CLI as a model it cannot serve, so the stage
// spawns, produces no output, and is retried. The ordinary path guards this
// at its list check; `cliProviderRun.js#resolveCliProviderAndModel` does too.
const modelSelection = await selectModelForTask(task, provider);
const pinnedModel = task.metadata?.model;
const selectedModel = pinnedModel && task.metadata?.provider === provider.id
const pinnedForThisProvider = Boolean(pinnedModel) && task.metadata?.provider === provider.id;
// A provider that enumerates NO models is a pass-through (any id is its
// caller's to choose), so only a non-empty list can invalidate a pin.
const offeredModels = Array.isArray(provider.models) ? provider.models : [];
const pinIsOffered = offeredModels.length === 0 || offeredModels.includes(pinnedModel);
if (pinnedForThisProvider && !pinIsOffered) {
emitLog('warn', `Public-review stage model "${pinnedModel}" is not offered by provider "${provider.id}" — using its default instead`, {
taskId: task.id,
requestedModel: pinnedModel,
providerId: provider.id,
validModels: offeredModels,
});
}
const selectedModel = pinnedForThisProvider && pinIsOffered
? pinnedModel
: (modelSelection.model || provider.defaultModel || null);
emitLog('info', `Public-review stage (${posture}) resolved to provider ${provider.id}${selectedModel ? ` model ${selectedModel}` : ''}`, {
Expand Down
24 changes: 24 additions & 0 deletions server/services/agentProviderResolution.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,30 @@ describe('resolveAgentProviderAndModel — public-review stages', () => {
.resolves.toMatchObject({ provider: { id: 'codex-cli' }, selectedModel: 'm-default' });
});

// A stage pin outlives edits to the provider's own model list: the live
// pr-reviewer gate sat pinned to an id its provider no longer offered, so
// every run spawned a CLI that could not serve the model, produced no
// output, and was retried — matching the provider is not enough on its own.
it('drops a model pin the matching provider no longer offers', async () => {
const CURATED = { id: 'grok-cli', type: 'cli', command: 'grok', models: ['grok-4'], defaultModel: 'grok-4' };
getAllProviders.mockResolvedValue({ providers: [CURATED], activeProvider: null });

// Still listed → honored.
await expect(resolveAgentProviderAndModel(gateTask({ provider: 'grok-cli', model: 'grok-4' })))
.resolves.toMatchObject({ provider: { id: 'grok-cli' }, selectedModel: 'grok-4' });

// Retired from the list → the provider's own selection wins instead.
await expect(resolveAgentProviderAndModel(gateTask({ provider: 'grok-cli', model: 'grok-3-retired' })))
.resolves.toMatchObject({ provider: { id: 'grok-cli' }, selectedModel: 'm-default' });
});

// A provider that enumerates no models is a pass-through, so the pin stands.
it('honors a model pin on a provider that enumerates no models', async () => {
getAllProviders.mockResolvedValue({ providers: [GROK], activeProvider: null });
await expect(resolveAgentProviderAndModel(gateTask({ provider: 'grok-cli', model: 'anything-goes' })))
.resolves.toMatchObject({ provider: { id: 'grok-cli' }, selectedModel: 'anything-goes' });
});

it('blocks PERMANENTLY when no enabled provider can enforce the posture', async () => {
getAllProviders.mockResolvedValue({ providers: [OPENCODE], activeProvider: { id: 'opencode' } });
const r = await resolveAgentProviderAndModel(gateTask());
Expand Down