fix(row 332 R2): proxy first check gates the route the final check picks (Vikunja #774)
Route selection moves into one helper, pickRoute(proxyOf), that both checks call: the primary preflight, and the fallback with its own preflight only when the primary's fails, with the same messages. The final check passes proxyFor and is otherwise unchanged. The first check of a proxy launch passes a stand-in session proxy (marker key PENDING_ADMISSION_KEY and the configured loopback base URL), which reaches only the preflights, then runs gate() with a null session proxy on the predicted route only. The round 1 union over both routes is gone. The managed fixture takes extra (more harnesses switched on), real (harnesses loaded unfaked) and hostEnv. Seven tests in proxy-launch-order.test.mjs.
This commit is contained in:
jason.woltje committed
1 parent
a605e44d1e
commit
b8f01a189f
3 files changed
+176
-45
No files matched your search
@@ -185,6 +185,9 @@ const LINE_MAX = 4096;
|
||||
const THREAD_TICK_MS = 2000;
|
||||
// Row 194: the broker's LIFECYCLE_REQUEST_MS. A regenerate's old run must be gone by then.
|
||||
const LIFECYCLE_REQUEST_MS = 60000;
|
||||
// #774 R2: the session proxy key the first check of a proxy launch gives the harness preflights in
|
||||
// place of the key P2 hasn't admitted yet. A fixed marker, never a credential; it never leaves check().
|
||||
export const PENDING_ADMISSION_KEY = "pending-admission";
|
||||
|
||||
// A refusal the host decided; `code` goes back to the caller and to the broker.
|
||||
class Refusal extends Error {
|
||||
@@ -401,16 +404,18 @@ export function createLauncher({
|
||||
let routeReason = null;
|
||||
// A launch is checked twice. Without system.proxy both are full checks: the first before the
|
||||
// authorization, workspace and checkout work, the final one in startLocked before newRun. With
|
||||
// system.proxy the first check skips only what needs the key P2 admits in startLocked: the
|
||||
// harness preflight and the match of the session proxy key against launch.account.name. It
|
||||
// still runs the account declaration (credential "proxy" only), the sandbox, capacity, the
|
||||
// limit checks and admission, so a launch that can never start costs no authorization record,
|
||||
// checkout or key (#774 F1). The final check is always full, on the admitted key. Never reject
|
||||
// proxy-only login using upstream host env.
|
||||
// system.proxy the first check comes before P2 admits a key in startLocked, so it predicts the
|
||||
// route: it runs the harness preflights on a stand-in key and picks the route as the final check
|
||||
// does (#774 R2). The prediction holds because a harness preflight tests only that a proxy key
|
||||
// is present, never its value. On the predicted route it runs the account declaration
|
||||
// (credential "proxy" only), the sandbox, capacity, the limit checks and admission, so a launch
|
||||
// that can never start costs no authorization record, checkout or key (#774 F1). It skips only
|
||||
// the match of the session proxy key against launch.account.name. The final check is always
|
||||
// full, on the admitted key, and it picks the route again: if a harness binary appears or goes
|
||||
// between the two checks, its pick decides. Never reject proxy-only login using upstream host env.
|
||||
const early = Boolean(system.proxy) && !proxyReady;
|
||||
// The checks that follow the route's preflight. The parameters shadow the outer route on
|
||||
// purpose: the first check of a proxy launch gates a route it hasn't chosen. sessionProxy is
|
||||
// null there, and the account must then name credential "proxy".
|
||||
// The checks that follow the route's preflight, on the route the caller picked. In the first
|
||||
// check of a proxy launch sessionProxy is null, and the account must then name credential "proxy".
|
||||
const gate = async (model, harness, sessionProxy) => {
|
||||
const proxied = early || Boolean(sessionProxy);
|
||||
// Row 126 (#1620): only a launch on a harness whose module counts the account spends the named
|
||||
@@ -455,22 +460,6 @@ export function createLauncher({
|
||||
}
|
||||
return account;
|
||||
};
|
||||
if (early) {
|
||||
// The final check takes the fallback only when the primary's preflight fails, which this check
|
||||
// can't know yet, so it refuses only when neither route would pass, with the primary's reason.
|
||||
try {
|
||||
await gate(primary, harness, null);
|
||||
} catch (e) {
|
||||
if (!(e instanceof Refusal) || !fallback) throw e;
|
||||
try {
|
||||
await gate(fallback, await harnessOf(fallback), null);
|
||||
} catch (also) {
|
||||
throw also instanceof Refusal ? e : also;
|
||||
}
|
||||
}
|
||||
admission(admissionGeneration);
|
||||
return { resolved, model, account: null, harness, route, routeReason, admissionGeneration };
|
||||
}
|
||||
const proxyFor = async (selected) => {
|
||||
if (!system.proxy && selected.provider !== "proxy") return null;
|
||||
const context = system.proxy ? proxyContext : typeof testProxy === "function" ? await testProxy({ business: b.id, instance }) : testProxy;
|
||||
@@ -481,21 +470,41 @@ export function createLauncher({
|
||||
// Copy only the agreed seam; never keep a store or management handle.
|
||||
return { baseUrl: context.baseUrl, key: context.key };
|
||||
};
|
||||
let sessionProxy = await proxyFor(primary);
|
||||
const failed = call(primary.harness, "preflight", () => harness.preflight({ env: sessionProxy ? sessionEnv(env, "preflight", sessionProxy) : env, proxy: sessionProxy }));
|
||||
if (failed) {
|
||||
if (!fallback) throw new Refusal(failed.code, failed.message);
|
||||
// The fallback only on a failed primary preflight, and only at launch (row 65). Any refusal of
|
||||
// the fallback names the primary's failure too, so the reason it was tried stays visible.
|
||||
const second = await harnessOf(fallback).catch((e) => {
|
||||
if (e instanceof Refusal) e.message += ` (primary ${primary.harness}: ${failed.message})`;
|
||||
throw e;
|
||||
});
|
||||
sessionProxy = await proxyFor(fallback);
|
||||
const also = call(fallback.harness, "preflight", () => second.preflight({ env: sessionProxy ? sessionEnv(env, "preflight", sessionProxy) : env, proxy: sessionProxy }));
|
||||
if (also) throw new Refusal(also.code, `fallback ${fallback.harness}: ${also.message} (primary ${primary.harness}: ${failed.message})`);
|
||||
[model, harness, route, routeReason] = [fallback, second, "fallback", failed.code];
|
||||
// The route both checks pick (#774 R2): the primary when its preflight passes, else the fallback
|
||||
// through its own preflight. proxyOf gives the session proxy for a route; the preflights see it,
|
||||
// and the helper returns the last one they saw.
|
||||
const pickRoute = async (proxyOf) => {
|
||||
let sessionProxy = await proxyOf(primary);
|
||||
const failed = call(primary.harness, "preflight", () => harness.preflight({ env: sessionProxy ? sessionEnv(env, "preflight", sessionProxy) : env, proxy: sessionProxy }));
|
||||
if (failed) {
|
||||
if (!fallback) throw new Refusal(failed.code, failed.message);
|
||||
// The fallback only on a failed primary preflight, and only at launch (row 65). Any refusal of
|
||||
// the fallback names the primary's failure too, so the reason it was tried stays visible.
|
||||
const second = await harnessOf(fallback).catch((e) => {
|
||||
if (e instanceof Refusal) e.message += ` (primary ${primary.harness}: ${failed.message})`;
|
||||
throw e;
|
||||
});
|
||||
sessionProxy = await proxyOf(fallback);
|
||||
const also = call(fallback.harness, "preflight", () => second.preflight({ env: sessionProxy ? sessionEnv(env, "preflight", sessionProxy) : env, proxy: sessionProxy }));
|
||||
if (also) throw new Refusal(also.code, `fallback ${fallback.harness}: ${also.message} (primary ${primary.harness}: ${failed.message})`);
|
||||
[model, harness, route, routeReason] = [fallback, second, "fallback", failed.code];
|
||||
}
|
||||
return sessionProxy;
|
||||
};
|
||||
if (early) {
|
||||
// The first check predicts the final check's route. A harness preflight tests only that a proxy
|
||||
// key is present, so a stand-in key gets the answer the admitted key will get, and the route
|
||||
// picked here is the one the final check picks unless a harness binary appears or goes in
|
||||
// between; then the final check's own pick decides. The stand-in reaches only the preflights:
|
||||
// pickRoute's answer is dropped, gate() gets null, and nothing records or returns it. The gate
|
||||
// then runs on the predicted route only, with the codes, messages and limits the final check uses.
|
||||
const standIn = { baseUrl: Number.isInteger(system.proxy.port) ? `http://127.0.0.1:${system.proxy.port}` : "http://127.0.0.1:0", key: PENDING_ADMISSION_KEY };
|
||||
await pickRoute(() => standIn);
|
||||
await gate(model, harness, null);
|
||||
admission(admissionGeneration);
|
||||
return { resolved, model, account: null, harness, route, routeReason, admissionGeneration };
|
||||
}
|
||||
const sessionProxy = await pickRoute(proxyFor);
|
||||
const account = await gate(model, harness, sessionProxy);
|
||||
admission(admissionGeneration);
|
||||
return { resolved, model, account, harness, route, routeReason, admissionGeneration, sessionProxy };
|
||||
|
||||
@@ -17,13 +17,16 @@ export const pause = ms => new Promise(r=>setTimeout(r,ms));
|
||||
const id = value => createHash('sha256').update(value).digest('hex').slice(0,12);
|
||||
// #774: bwrap overrides the launcher's sandbox binary; admit rewrites what P2 admits, so a test can
|
||||
// reach the final check's key match. trace.ops names every trusted host operation in order.
|
||||
export async function managedLaunchFixture(t,{proxy=true,launches=true,refuseProbe=false,harness='pi',bwrap=null,admit=null}={}){
|
||||
// #774 R2: extra names more harnesses to switch on and load (a fallback route's); real names the
|
||||
// harnesses loaded as the real module, so their own preflight runs; hostEnv adds launcher env
|
||||
// such as MOSAIC_CLAUDE_BIN.
|
||||
export async function managedLaunchFixture(t,{proxy=true,launches=true,refuseProbe=false,harness='pi',bwrap=null,admit=null,extra=[],real=[],hostEnv={}}={}){
|
||||
const root=fs.mkdtempSync(join(tmpdir(),'ml653-')),closers=[],history=new Set(['fx653-manage']),proxyReceipts=[],logs=[];
|
||||
let server,bus,launcher,stderr;const trace={preflightAttempts:0,successfulPreflights:0,enableBeforeSpawn:[],bindings:[],stalls:[],ops:[]};
|
||||
const checkHistory=entries=>{const text=JSON.stringify(entries);for(const value of history)if(text.includes(value))throw Error('fixture credential leaked');};
|
||||
t.after(async()=>{const failures=[];for(const close of [...closers].reverse()){try{await close();}catch{failures.push('owned-close');}}try{server?.closeAllConnections();if(server?.listening)await new Promise((resolve,reject)=>server.close(e=>e?reject(e):resolve()));}catch{failures.push('owned-server-close');}try{checkHistory({logs,proxyReceipts,trace});}catch{failures.push('value-free-receipts');}if(stderr)process.stderr.write=stderr;try{fs.rmSync(root,{recursive:true,force:true});}catch{failures.push('owned-root-close');}if(failures.length)throw new AggregateError(failures.map(code=>Error(code)),'owned managed fixture cleanup failed');});
|
||||
const f=fixture(root,'acme',doc=>{doc.plugins={...doc.plugins,harness:[harness]};for(const role of Object.values(doc.roles))role.vars={...role.vars,harness};doc.roles.chief.vars.model='claude-opus-5-5';return doc;});
|
||||
const env={PATH:process.env.PATH,HOME:process.env.HOME,TMPDIR:process.env.TMPDIR,MOSAIC_CONFIG:f.config,MOSAIC_ROLES_DIR:f.roles};
|
||||
const f=fixture(root,'acme',doc=>{doc.plugins={...doc.plugins,harness:[harness,...extra]};for(const role of Object.values(doc.roles))role.vars={...role.vars,harness};doc.roles.chief.vars.model='claude-opus-5-5';return doc;});
|
||||
const env={PATH:process.env.PATH,HOME:process.env.HOME,TMPDIR:process.env.TMPDIR,MOSAIC_CONFIG:f.config,MOSAIC_ROLES_DIR:f.roles,...hostEnv};
|
||||
// The nonproxy positive fixture supplies its own fake pi login. Managed
|
||||
// proxy fixtures deliberately hold no upstream credential at all.
|
||||
if(!proxy)env.ZAI_API_KEY='fx653-pi';
|
||||
@@ -84,7 +87,7 @@ export async function managedLaunchFixture(t,{proxy=true,launches=true,refusePro
|
||||
return actualOp(m);
|
||||
},
|
||||
bindLaunch:async record=>{const result=bus.bindLaunch(record);trace.bindings.push({run:record.run,pid:record.pid,startTime:record.startTime,modelKeyId:record.modelKeyId});return result;}};
|
||||
const makeLoader=await fakeHarnesses({after:close=>closers.push(close)},{[harness]:adapter});
|
||||
const makeLoader=await fakeHarnesses({after:close=>closers.push(close)},Object.fromEntries([harness,...extra].map(name=>[name,real.includes(name)?null:adapter])));
|
||||
launcher=createLauncher({host,system,env,launches,makeLoader,...(bwrap?{bwrap}:{}),sessionDefaults:{pollInterval:50},harnessVersion:'0.85.1',log:line=>logs.push(line)});
|
||||
// The existing trusted host hook owns all launcher-created children. No registry
|
||||
// PID is signalled by fixture teardown; normal bounded launcher close is used.
|
||||
|
||||
@@ -1,11 +1,14 @@
|
||||
// Vikunja #774 F1 (row 332 review): with system.proxy set, a launch that can never start refuses in
|
||||
// the first check, before authorizeLaunch, the workspace and checkouts, and the P2 modelPreflight
|
||||
// that may issue a key. Only the harness preflight and the session proxy key match wait for the
|
||||
// admitted key, in the final check. Real launcher, bus runtime and runner (managed P2 fixture).
|
||||
// that may issue a key. The first check predicts the route from the harness preflights on a
|
||||
// stand-in key (R2); only the session proxy key match waits for the admitted key, in the final
|
||||
// check. Real launcher, bus runtime and runner (managed P2 fixture).
|
||||
import test from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import fs from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { DatabaseSync } from 'node:sqlite';
|
||||
import { PENDING_ADMISSION_KEY } from '../src/launcher.mjs';
|
||||
import { sandboxProblem } from '../../seat/src/sandbox.mjs';
|
||||
import { workspaceDir } from '../../seat/src/session.mjs';
|
||||
import { managedLaunchFixture } from './model-managed-launch-fixture.mjs';
|
||||
@@ -122,3 +125,119 @@ test('#774 F1 proxy launch of an agent with no key yet passes the first check un
|
||||
assert.equal(f.trace.ops.filter(op => op === 'mayLaunch').length, 2, 'mayLaunch in the first and the final check');
|
||||
f.valueFree();
|
||||
});
|
||||
|
||||
// #774 R2: the first check gates the route the final check picks. The coder's primary is the real
|
||||
// claude-code module on claude-opus-5-5 (provider anthropic); MOSAIC_CLAUDE_BIN decides whether its
|
||||
// preflight passes. The stated fallback is pi on glm-5.3-flash (provider zai).
|
||||
const NO_CLAUDE = '/nonexistent-774/claude';
|
||||
const NO_CLAUDE_WHY = `MOSAIC_CLAUDE_BIN ${NO_CLAUDE} isn't an executable file at an absolute path`;
|
||||
const routed = (t, bin) => managedLaunchFixture(t, { harness: 'claude-code', extra: ['pi'], real: ['claude-code'], hostEnv: { MOSAIC_CLAUDE_BIN: bin } });
|
||||
const withFallback = (doc, harness = 'pi', model = 'glm-5.3-flash') => {
|
||||
claudeCoder(doc);
|
||||
doc.roles.coder.vars = { ...doc.roles.coder.vars, 'fallback.harness': harness, 'fallback.model': model, 'fallback.thinking': 'high' };
|
||||
};
|
||||
const full = (doc, provider, model) => { doc.launch.limits.models = { ...doc.launch.limits.models, [provider]: { ...doc.launch.limits.models?.[provider], [model]: 0 } }; };
|
||||
const launched = (f, run) => f.launchLog().find(e => e.event === 'launch' && e.run === run);
|
||||
|
||||
test('#774 R2 a healthy primary at capacity zero refuses capacity-full in the first check, though the fallback has room', { skip, timeout: 60000 }, async t => {
|
||||
const f = await routed(t, process.execPath);
|
||||
editBusiness(f, doc => { withFallback(doc); full(doc, 'anthropic', 'claude-opus-5-5'); });
|
||||
const before = f.trace.ops.length;
|
||||
assert.deepEqual(await f.launcher.fromSession(chief(f), 'coder'), { ok: false, error: 'capacity-full' });
|
||||
assert.deepEqual(refusal(f, 'capacity-full').map(e => e.limit), [{ scope: 'model', limit: 0, running: 0, provider: 'anthropic', model: 'claude-opus-5-5' }]);
|
||||
refusedFirst(f, before, 0);
|
||||
assert.equal(f.runDirectories().length, 0);
|
||||
f.valueFree();
|
||||
});
|
||||
|
||||
test('#774 R2 a failed primary preflight with a stated fallback launches on the fallback, route fallback, reason harness-binary-missing', { skip, timeout: 60000 }, async t => {
|
||||
const f = await routed(t, NO_CLAUDE);
|
||||
editBusiness(f, doc => withFallback(doc));
|
||||
const answer = await f.launcher.fromSession(chief(f), 'coder');
|
||||
assert.equal(answer.ok, true, JSON.stringify(answer));
|
||||
assert.deepEqual(answer.result, { run: answer.result.run, instance: 'coder', harness: 'pi', model: 'glm-5.3-flash' });
|
||||
assert.ok(f.sessions().find(s => s.run === answer.result.run)?.live, 'the fallback launch is live');
|
||||
const event = launched(f, answer.result.run);
|
||||
assert.deepEqual([event?.harness, event?.route, event?.routeReason, event?.provider, event?.model], ['pi', 'fallback', 'harness-binary-missing', 'zai', 'glm-5.3-flash']);
|
||||
assert.equal(f.trace.successfulPreflights, 1);
|
||||
assert.equal(f.trace.ops.filter(op => op === 'authorizeLaunch').length, 1);
|
||||
f.valueFree();
|
||||
});
|
||||
|
||||
test('#774 R2 a failed primary preflight at capacity zero still launches on the fallback: the first check gates the predicted route', { skip, timeout: 60000 }, async t => {
|
||||
const f = await routed(t, NO_CLAUDE);
|
||||
editBusiness(f, doc => { withFallback(doc); full(doc, 'anthropic', 'claude-opus-5-5'); });
|
||||
const answer = await f.launcher.fromSession(chief(f), 'coder');
|
||||
assert.equal(answer.ok, true, JSON.stringify(answer));
|
||||
assert.ok(f.sessions().find(s => s.run === answer.result.run)?.live, 'the fallback launch is live');
|
||||
const event = launched(f, answer.result.run);
|
||||
assert.deepEqual([event?.harness, event?.route, event?.routeReason], ['pi', 'fallback', 'harness-binary-missing']);
|
||||
assert.equal(refusal(f, 'capacity-full').length, 0);
|
||||
f.valueFree();
|
||||
});
|
||||
|
||||
test('#774 R2 a failed primary preflight and a fallback at capacity zero refuse capacity-full for the fallback in the first check', { skip, timeout: 60000 }, async t => {
|
||||
const f = await routed(t, NO_CLAUDE);
|
||||
editBusiness(f, doc => { withFallback(doc); full(doc, 'zai', 'glm-5.3-flash'); });
|
||||
const before = f.trace.ops.length;
|
||||
assert.deepEqual(await f.launcher.fromSession(chief(f), 'coder'), { ok: false, error: 'capacity-full' });
|
||||
assert.deepEqual(refusal(f, 'capacity-full').map(e => e.limit), [{ scope: 'model', limit: 0, running: 0, provider: 'zai', model: 'glm-5.3-flash' }]);
|
||||
refusedFirst(f, before, 0);
|
||||
assert.equal(f.runDirectories().length, 0);
|
||||
f.valueFree();
|
||||
});
|
||||
|
||||
test('#774 R2 when both preflights fail the first check refuses with the final check\'s combined code and message', { skip, timeout: 60000 }, async t => {
|
||||
const f = await routed(t, NO_CLAUDE);
|
||||
editBusiness(f, doc => withFallback(doc, 'claude-code', 'claude-sonnet-5-5'));
|
||||
const before = f.trace.ops.length;
|
||||
assert.deepEqual(await f.launcher.fromSession(chief(f), 'coder'), { ok: false, error: 'harness-binary-missing' });
|
||||
assert.deepEqual(refusal(f, 'harness-binary-missing').map(e => e.why), [`fallback claude-code: ${NO_CLAUDE_WHY} (primary claude-code: ${NO_CLAUDE_WHY})`]);
|
||||
refusedFirst(f, before, 0);
|
||||
assert.equal(f.runDirectories().length, 0);
|
||||
f.valueFree();
|
||||
});
|
||||
|
||||
test('#774 R2 a failed primary preflight with no fallback refuses harness-binary-missing in the first check', { skip, timeout: 60000 }, async t => {
|
||||
const f = await routed(t, NO_CLAUDE);
|
||||
editBusiness(f, claudeCoder);
|
||||
const before = f.trace.ops.length;
|
||||
assert.deepEqual(await f.launcher.fromSession(chief(f), 'coder'), { ok: false, error: 'harness-binary-missing' });
|
||||
assert.deepEqual(refusal(f, 'harness-binary-missing').map(e => e.why), [NO_CLAUDE_WHY]);
|
||||
refusedFirst(f, before, 0);
|
||||
assert.equal(f.runDirectories().length, 0);
|
||||
f.valueFree();
|
||||
});
|
||||
|
||||
// Both preflights of the first check saw the stand-in key; none of it reaches the evidence.
|
||||
test('#774 R2 the stand-in key never reaches the launch log, the authorization record, session.json or the data root', { skip, timeout: 60000 }, async t => {
|
||||
assert.equal(typeof PENDING_ADMISSION_KEY === 'string' && PENDING_ADMISSION_KEY.length > 0, true);
|
||||
const f = await routed(t, NO_CLAUDE);
|
||||
editBusiness(f, doc => withFallback(doc));
|
||||
const answer = await f.launcher.fromSession(chief(f), 'coder');
|
||||
assert.equal(answer.ok, true, JSON.stringify(answer));
|
||||
assert.ok(f.sessions().find(s => s.run === answer.result.run)?.live, 'the fallback launch is live');
|
||||
const absent = (what, value) => assert.equal(String(value).includes(PENDING_ADMISSION_KEY), false, `the stand-in key reached ${what}`);
|
||||
const log = f.launchLog();
|
||||
assert.ok(launched(f, answer.result.run), 'the launch log holds the launch');
|
||||
absent('the launch log', JSON.stringify(log));
|
||||
const db = new DatabaseSync(join(f.f.dataRoot, 'bus', 'bus.sqlite'), { readOnly: true });
|
||||
let events;
|
||||
try { events = db.prepare('SELECT kind, subject, body FROM events ORDER BY seq').all(); } finally { db.close(); }
|
||||
assert.equal(events.filter(e => e.kind === 'action.allowed' && JSON.parse(e.body).action === 'role.launch').length, 1, 'one authorization record for the launch');
|
||||
absent('the bus events', JSON.stringify(events));
|
||||
const session = join(f.runDirectories().find(d => d.endsWith(`/${answer.result.run}`)), 'session.json');
|
||||
assert.ok(fs.existsSync(session), 'the run has a session.json');
|
||||
absent('session.json', fs.readFileSync(session, 'utf8'));
|
||||
absent('the host log', JSON.stringify(f.logs));
|
||||
absent('the trace', JSON.stringify(f.trace));
|
||||
// Every regular file under the data root, the bus database and its WAL included.
|
||||
const files = fs.readdirSync(f.f.dataRoot, { recursive: true, withFileTypes: true }).filter(e => e.isFile()).map(e => join(e.parentPath, e.name));
|
||||
assert.ok(files.length > 0);
|
||||
for (const file of files) {
|
||||
let bytes;
|
||||
try { bytes = fs.readFileSync(file); } catch (e) { if (e.code === 'ENOENT') continue; throw e; }
|
||||
absent(file, bytes);
|
||||
}
|
||||
f.valueFree();
|
||||
});
|
||||
Reference in new issue
Block a user