From f0a30b8356ccf80ec0d906ac44eeb9c97f842dbf Mon Sep 17 00:00:00 2001 From: saravanakumardb1 Date: Mon, 1 Jun 2026 10:59:57 -0700 Subject: [PATCH] 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: 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> --- .../src/modules/fleet/coordinator.ts | 4 ++ .../src/modules/fleet/scheduler.test.ts | 46 +++++++++++++++++++ .../src/modules/fleet/scheduler.ts | 27 ++++++++++- 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/services/platform-service/src/modules/fleet/coordinator.ts b/services/platform-service/src/modules/fleet/coordinator.ts index c33aaf3f..4de03feb 100644 --- a/services/platform-service/src/modules/fleet/coordinator.ts +++ b/services/platform-service/src/modules/fleet/coordinator.ts @@ -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 { }); }); +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' }); diff --git a/services/platform-service/src/modules/fleet/scheduler.ts b/services/platform-service/src/modules/fleet/scheduler.ts index 265cf255..703de200 100644 --- a/services/platform-service/src/modules/fleet/scheduler.ts +++ b/services/platform-service/src/modules/fleet/scheduler.ts @@ -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:` 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;