fix(fleet): enforce the job's concrete engine in routing
The engine picker only constrained the UI; the router never gated on the chosen engine, so a job pinned to e.g. engine:codex could be claimed by a factory that doesn't run codex (the runner's resolve_engine honors an explicit engine with no availability check, so it would then fail at execution time). Add a pure engineEligible(job, caps) hard gate to the section-7 scheduler filter (and preemption): a concrete-engine job runs only on a factory advertising the matching engine:<e> cap. Gated only against engine-aware factories (those that advertise any engine:* token); engine-agnostic/legacy factories stay eligible, mirroring the picker's "engine set unknown => offer all" fallback. explainJob now surfaces the mismatch reason. No DB migration; behavior-preserving for legacy. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
parent
d318e0fa2a
commit
f0a30b8356
@ -27,6 +27,7 @@ import {
|
||||
selectPreemptionVictim,
|
||||
scoreCandidate,
|
||||
capabilitiesSubset,
|
||||
engineEligible,
|
||||
type RunningJobView,
|
||||
type ScoreBreakdown,
|
||||
type SchedulerContext,
|
||||
@ -1392,6 +1393,9 @@ export async function explainJob(jobId: string, productId: string): Promise<JobE
|
||||
if (!capabilitiesSubset(job.capabilities ?? [], f.capabilities)) {
|
||||
reasons.push('missing required capabilities');
|
||||
}
|
||||
if (!engineEligible(job, f.capabilities)) {
|
||||
reasons.push(`engine '${job.engine}' not advertised by factory`);
|
||||
}
|
||||
if (!depsSatisfied) reasons.push(`unmet deps: ${unmet.join(', ')}`);
|
||||
return {
|
||||
factoryId: f.factoryId,
|
||||
|
||||
@ -9,6 +9,7 @@ import {
|
||||
scoreCandidate,
|
||||
selectJob,
|
||||
capabilitiesSubset,
|
||||
engineEligible,
|
||||
type SchedulerContext,
|
||||
type SchedulerFactory,
|
||||
} from './scheduler.js';
|
||||
@ -81,6 +82,51 @@ describe('scheduler §7 — capability hard filter', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('scheduler §7 — concrete-engine routing gate', () => {
|
||||
it('engineEligible: no concrete engine ⇒ never constrained', () => {
|
||||
expect(engineEligible(job({ id: 'j' }), ['engine:devin'])).toBe(true);
|
||||
expect(engineEligible(job({ id: 'j' }), [])).toBe(true);
|
||||
});
|
||||
|
||||
it('engineEligible: engine-aware factory must advertise the matching engine', () => {
|
||||
const codexJob = job({ id: 'codex', engine: 'codex' });
|
||||
expect(engineEligible(codexJob, ['engine:devin', 'engine:claude'])).toBe(false);
|
||||
expect(engineEligible(codexJob, ['engine:devin', 'engine:codex'])).toBe(true);
|
||||
});
|
||||
|
||||
it('engineEligible: engine-agnostic/legacy factory (no engine:* caps) stays eligible', () => {
|
||||
const codexJob = job({ id: 'codex', engine: 'codex' });
|
||||
expect(engineEligible(codexJob, ['os:mac', 'has:git'])).toBe(true);
|
||||
expect(engineEligible(codexJob, [])).toBe(true);
|
||||
});
|
||||
|
||||
it('selectJob: a codex job is not claimed by a factory advertising only devin/claude', () => {
|
||||
const codexJob = job({ id: 'codex', engine: 'codex' });
|
||||
const devinOnly = fac({ capabilities: ['engine:devin', 'engine:claude'] });
|
||||
expect(selectJob([codexJob], devinOnly, ctx())).toBeNull();
|
||||
});
|
||||
|
||||
it('selectJob: a codex job IS claimed by a factory advertising codex', () => {
|
||||
const codexJob = job({ id: 'codex', engine: 'codex' });
|
||||
const codexFac = fac({ capabilities: ['engine:devin', 'engine:codex'] });
|
||||
expect(selectJob([codexJob], codexFac, ctx())?.id).toBe('codex');
|
||||
});
|
||||
|
||||
it('selectJob: routes a codex job to the codex factory, not the devin-only peer', () => {
|
||||
const codexJob = job({ id: 'codex', engine: 'codex' });
|
||||
const devinOnly = fac({ capabilities: ['engine:devin'] });
|
||||
const codexFac = fac({ capabilities: ['engine:codex'] });
|
||||
expect(selectJob([codexJob], devinOnly, ctx())).toBeNull();
|
||||
expect(selectJob([codexJob], codexFac, ctx())?.id).toBe('codex');
|
||||
});
|
||||
|
||||
it('selectJob: engine-agnostic legacy factory still claims an engine-pinned job', () => {
|
||||
const codexJob = job({ id: 'codex', engine: 'codex', capabilities: ['os:mac'] });
|
||||
const legacy = fac({ capabilities: ['os:mac', 'has:git'] }); // advertises no engine:* caps
|
||||
expect(selectJob([codexJob], legacy, ctx())?.id).toBe('codex');
|
||||
});
|
||||
});
|
||||
|
||||
describe('scheduler §7 — priority + age tie-breaks (all else equal)', () => {
|
||||
it('priority dominates when scores tie', () => {
|
||||
const low = job({ id: 'low', priority: 'low' });
|
||||
|
||||
@ -133,6 +133,29 @@ export function capabilitiesSubset(required: string[], available: string[]): boo
|
||||
return required.every(token => set.has(token));
|
||||
}
|
||||
|
||||
/** True when the factory advertises any concrete `engine:*` capability — i.e. it is
|
||||
* "engine-aware" and its advertised engine set is authoritative. A factory that
|
||||
* advertises none is treated as engine-agnostic/legacy (no engine gating). */
|
||||
function advertisesAnyEngine(available: string[]): boolean {
|
||||
return available.some(cap => cap.startsWith('engine:'));
|
||||
}
|
||||
|
||||
/**
|
||||
* Engine routing gate (hard): a job pinned to a concrete `engine` may only run on
|
||||
* a factory that advertises the matching `engine:<e>` capability — but only when
|
||||
* that factory is engine-aware (advertises at least one `engine:*` token). A
|
||||
* factory advertising no engine caps is engine-agnostic/legacy and stays eligible
|
||||
* (behavior-preserving — mirrors the UI picker's "engine set unknown ⇒ offer all"
|
||||
* fallback). A job WITHOUT a concrete `engine` is never constrained here: its
|
||||
* abstract `engineClass`/`prefersEngine` resolve on the runner, which does its own
|
||||
* availability check + fallback (`resolve_engine`).
|
||||
*/
|
||||
export function engineEligible(job: FleetJobDoc, available: string[]): boolean {
|
||||
if (!job.engine) return true;
|
||||
if (!advertisesAnyEngine(available)) return true;
|
||||
return available.includes(`engine:${job.engine}`);
|
||||
}
|
||||
|
||||
function overlaps(a: readonly string[], b: readonly string[]): boolean {
|
||||
if (a.length === 0 || b.length === 0) return false;
|
||||
const set = new Set(b);
|
||||
@ -312,8 +335,9 @@ export function selectPreemptionVictim(
|
||||
// Only critical jobs may preempt
|
||||
if (candidate.priorityOrder !== 0) return null; // 0 = critical
|
||||
|
||||
// Candidate must be runnable on this factory (capability subset)
|
||||
// Candidate must be runnable on this factory (capability subset + engine gate)
|
||||
if (!capabilitiesSubset(candidate.capabilities ?? [], factory.capabilities)) return null;
|
||||
if (!engineEligible(candidate, factory.capabilities)) return null;
|
||||
|
||||
// Find strictly-lower-priority running jobs
|
||||
const evictable = runningJobs.filter(r => r.priorityOrder > candidate.priorityOrder);
|
||||
@ -364,6 +388,7 @@ export function selectJob(
|
||||
job =>
|
||||
(job.stage === 'queued' || job.stage === 'blocked') &&
|
||||
capabilitiesSubset(job.capabilities ?? [], factory.capabilities) &&
|
||||
engineEligible(job, factory.capabilities) &&
|
||||
depsSatisfied(job)
|
||||
);
|
||||
if (eligible.length === 0) return null;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user