From b222de303f4df890bdc39ac9b48c7cf8355bf0e8 Mon Sep 17 00:00:00 2001 From: "open-swe[bot]" Date: Sat, 8 Aug 2026 10:38:06 -0400 Subject: [PATCH 1/2] feat(quorum): validate the roster and decide the quorum gate in preflight (P3 + P5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracted fresh from the long-open #293, the same way P2 (#392) and P4 (#395) were — that PR is 5 weeks stale against files that have changed substantially. P5 — validateProviders(). A malformed slot does not announce itself; it fails at dispatch, mid-quorum, as an opaque UNAVAIL. Both faults this repo has actually shipped were visible in the config the whole time: - neither `cli` nor `mainTool` -> spawn() gets null, whole quorum offline (#197) - args_template with no {prompt} -> CLI runs with flags and no question Also errors on duplicate names (silently last-wins downstream), missing/unknown type, and http slots with no baseUrl or auth. Warns on non-positive timeout fields and third-party-routed slots with no deep_probe. Non-fatal by design: it reports on every --all and never blocks the preflight, because a config linter that can wedge the fleet is worse than the faults it catches. P3 — computeQuorumGate(). "available < max_quorum_size -> BLOCK unless --force-quorum" was prose in commands/nf/quorum.md for an LLM to interpret. A gate that depends on a model reading a paragraph correctly is not a gate. It is now a field: {available_count, quorum_met, degraded, blocked, waiver_required, waiver_used, gate_reason}. Two properties the prose kept losing, now pinned by tests: - degraded != blocked. Below max_quorum_size but above min_live_voters still dispatches; only below the floor blocks. - a --force-quorum waiver unblocks but is RECORDED, and does not fabricate quorum_met — the panel still reports what it actually is. 11 tests; the waiver, the degraded/blocked distinction, and the spawn-null check are each mutation-proven. Existing preflight suites unaffected. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + bin/preflight-gate-validator.test.cjs | 115 ++++++++++++++++++++++++ bin/quorum-preflight.cjs | 120 +++++++++++++++++++++++++- package.json | 2 +- 4 files changed, 235 insertions(+), 3 deletions(-) create mode 100644 bin/preflight-gate-validator.test.cjs diff --git a/CHANGELOG.md b/CHANGELOG.md index f8ebb6dc33..8ee33a71a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] ### Added +- feat(quorum): **preflight now validates the roster and decides the quorum gate (P3 + P5 of #293, extracted fresh).** `validateProviders()` reports config faults that would otherwise surface mid-quorum as an opaque UNAVAIL — the two this repo has actually shipped are a slot with neither `cli` nor `mainTool` (spawn receives `null`, taking the whole quorum offline, #197) and an `args_template` with no `{prompt}` placeholder (the CLI runs with flags and no question). Also errors on duplicate names (silently last-wins everywhere downstream), missing/unknown `type`, and http slots without a base URL or any auth; warns on non-positive timeout fields and on third-party-routed slots with no `deep_probe`. Non-fatal by design: it reports on every `--all` and never blocks the preflight. `computeQuorumGate()` turns *"available < max_quorum_size → BLOCK unless `--force-quorum`"* from prose in `commands/nf/quorum.md` into a machine field — `{available_count, quorum_met, degraded, blocked, waiver_required, waiver_used, gate_reason}` — because a gate that depends on a model reading a paragraph correctly is not a gate. It keeps **degraded** and **blocked** distinct (below `max_quorum_size` but above `min_live_voters` still dispatches), and a `--force-quorum` waiver unblocks while being *recorded* rather than silently making the panel look healthy. 11 tests, each of the three core behaviours mutation-proven. - feat(release): **`Align dist-tags` — a manually dispatchable workflow that repairs `@next` drift without a one-time password.** `publish.yml` aligns `@next` automatically after every publish since #391, but that cannot help a release published *before* that fix, or an alignment that failed transiently. Doing it by hand needs `npm dist-tag add`, which on a 2FA-protected account fails with `EOTP` and demands an authenticator code — so it cannot be scripted, and in practice it simply does not get done (0.44.3 shipped with `next` still on 0.44.2). The `NPM_TOKEN` secret is 2FA-exempt, which is what automation tokens are for, so CI can do unattended what an interactive human cannot. Scope is deliberately narrow: it moves a dist-tag and cannot publish. It refuses a prerelease target (a prerelease holding `@next` would silently install for everyone under the alias policy), and — unlike `publish.yml`'s verify step, which must never fail a job whose package is already out — this one **fails** if the tag did not move, because a green manual run that changed nothing is the exact outcome worth preventing. Each `run:` block was executed against a stubbed `npm view` across all four branches (aligned / not-moved / unreadable / prerelease) rather than reviewed by eye. - feat(quorum): **timeout records now say which config produced the window (I5).** `resolveTtfbTimeout` / `resolveInterChunkCeiling` each compute a `source` (`per-slot` / `default` / `idle-budget` / `legacy-stall`) and `recordTelemetry` was dropping both it and the resolved values, so a TIMEOUT in `quorum-rounds-*.jsonl` could be seen but never attributed — the number alone never answers "did *my* config do this?", since today's per-slot value can be tomorrow's default. Records now carry `ttfb_timeout_ms`, `ttfb_source`, `inter_chunk_ceiling_ms` and `inter_chunk_source`, always present (null when unknown) so the schema stays stable for analysis. Passed as a trailing options object rather than a 15th positional parameter. 4 tests drive the **real child** and read the actual log, so the fields are proven to survive the whole dispatch path; mutation-proven. Accepted as improvement I5 by the 2026-08-07 quorum. diff --git a/bin/preflight-gate-validator.test.cjs b/bin/preflight-gate-validator.test.cjs new file mode 100644 index 0000000000..4c19c138d9 --- /dev/null +++ b/bin/preflight-gate-validator.test.cjs @@ -0,0 +1,115 @@ +#!/usr/bin/env node +'use strict'; +// bin/preflight-gate-validator.test.cjs — P3 + P5 of the long-open #293, extracted fresh. +// +// P5 validateProviders: a malformed slot does not announce itself. It fails at dispatch, +// mid-quorum, as an opaque UNAVAIL. This repo has shipped that twice — a missing +// args_template crashed every slot on `.map` (ARGS-TEMPLATE-01), and a slot with neither +// `cli` nor `mainTool` handed null to spawn() and took the whole quorum offline (#197). +// Both were visible in the config the whole time. +// +// P3 computeQuorumGate: "available < max_quorum_size -> BLOCK unless --force-quorum" was +// prose in quorum.md for an LLM to interpret. A gate that depends on a model reading a +// paragraph correctly is not a gate. + +const { test } = require('node:test'); +const assert = require('node:assert'); +const { validateProviders, computeQuorumGate } = require('./quorum-preflight.cjs'); + +// ── P5 ─────────────────────────────────────────────────────────────────────── + +test('PGV-1: a well-formed roster is clean', () => { + const r = validateProviders([ + { name: 'codex-1', type: 'subprocess', mainTool: 'codex', args_template: ['exec', '{prompt}'] }, + { name: 'api-1', type: 'http', baseUrl: 'https://x/v1', apiKeyEnv: 'X_KEY' }, + ]); + assert.deepStrictEqual(r.errors, []); + assert.strictEqual(r.valid, true); +}); + +test('PGV-2: catches the two failures that actually took quorums down', () => { + // spawn(null) — #197. + const noCli = validateProviders([{ name: 'a', type: 'subprocess' }]); + assert.match(noCli.errors.join(' '), /neither 'cli' nor 'mainTool'/); + // A template that drops the prompt — the CLI runs with flags and no question. + const noPrompt = validateProviders([ + { name: 'b', type: 'subprocess', mainTool: 'x', args_template: ['exec'] }, + ]); + assert.match(noPrompt.errors.join(' '), /no \{prompt\} placeholder/); +}); + +test('PGV-3: duplicate names are an error, not last-wins', () => { + const r = validateProviders([ + { name: 'dup', type: 'subprocess', mainTool: 'x', args_template: ['{prompt}'] }, + { name: 'dup', type: 'subprocess', mainTool: 'y', args_template: ['{prompt}'] }, + ]); + assert.match(r.errors.join(' '), /duplicate name/); + assert.strictEqual(r.valid, false); +}); + +test('PGV-4: http slots need a base URL and some auth', () => { + assert.match(validateProviders([{ name: 'h', type: 'http', apiKeyEnv: 'K' }]).errors.join(' '), /missing 'baseUrl'/); + assert.match(validateProviders([{ name: 'h', type: 'http', baseUrl: 'https://x' }]).errors.join(' '), /no 'apiKeyEnv'/); + // An inline token in env counts as auth. + assert.deepStrictEqual( + validateProviders([{ name: 'h', type: 'http', baseUrl: 'https://x', env: { ANTHROPIC_AUTH_TOKEN: 't' } }]).errors, []); +}); + +test('PGV-5: garbage timeouts WARN rather than error — they are ignored, not fatal', () => { + const r = validateProviders([ + { name: 'w', type: 'subprocess', mainTool: 'x', args_template: ['{prompt}'], ttfb_timeout_ms: true, idle_timeout_ms: -1 }, + ]); + assert.deepStrictEqual(r.errors, [], 'a bad timeout still dispatches, so it must not be an error'); + assert.strictEqual(r.warnings.length, 2); + assert.match(r.warnings.join(' '), /ttfb_timeout_ms/); +}); + +test('PGV-6: never throws on hostile input', () => { + for (const bad of [null, undefined, 'string', 42, [null], [{}], [{ name: 'x' }]]) { + assert.doesNotThrow(() => validateProviders(bad), `threw on ${JSON.stringify(bad)}`); + } +}); + +// ── P3 ─────────────────────────────────────────────────────────────────────── + +test('PGV-7: full roster is neither degraded nor blocked', () => { + const g = computeQuorumGate({ availableCount: 7, maxQuorumSize: 7, minLiveVoters: 2 }); + assert.strictEqual(g.quorum_met, true); + assert.strictEqual(g.degraded, false); + assert.strictEqual(g.blocked, false); + assert.strictEqual(g.waiver_required, false); +}); + +test('PGV-8: degraded and blocked are DIFFERENT states', () => { + // The distinction prose kept losing: below max but above the floor still runs. + const degraded = computeQuorumGate({ availableCount: 3, maxQuorumSize: 7, minLiveVoters: 2 }); + assert.strictEqual(degraded.degraded, true); + assert.strictEqual(degraded.blocked, false, 'above the floor must still dispatch'); + assert.strictEqual(degraded.waiver_required, false); + + const blocked = computeQuorumGate({ availableCount: 1, maxQuorumSize: 7, minLiveVoters: 2 }); + assert.strictEqual(blocked.blocked, true); + assert.strictEqual(blocked.waiver_required, true); +}); + +test('PGV-9: --force-quorum waives a block and records that it did', () => { + const g = computeQuorumGate({ availableCount: 1, maxQuorumSize: 7, minLiveVoters: 2, forceQuorum: true }); + assert.strictEqual(g.blocked, false, 'an explicit waiver unblocks'); + assert.strictEqual(g.waiver_used, true, 'and the waiver must be recorded, not silent'); + assert.strictEqual(g.waiver_required, true); +}); + +test('PGV-10: force-quorum does not fabricate a healthy gate', () => { + // The failure mode worth preventing: a waiver that makes the panel look fine. + const g = computeQuorumGate({ availableCount: 1, maxQuorumSize: 7, minLiveVoters: 2, forceQuorum: true }); + assert.strictEqual(g.quorum_met, false); + assert.match(g.gate_reason, /below the floor/); +}); + +test('PGV-11: garbage inputs fall back to safe defaults instead of NaN', () => { + const g = computeQuorumGate({ availableCount: undefined, maxQuorumSize: 'x', minLiveVoters: 0 }); + assert.strictEqual(g.available_count, 0); + assert.strictEqual(g.max_quorum_size, 3); + assert.strictEqual(g.min_live_voters, 2); + assert.strictEqual(g.blocked, true, 'zero available must block, not NaN-compare to false'); +}); diff --git a/bin/quorum-preflight.cjs b/bin/quorum-preflight.cjs index 17b5f7aeb0..b900e24889 100644 --- a/bin/quorum-preflight.cjs +++ b/bin/quorum-preflight.cjs @@ -519,6 +519,105 @@ function ensureServices(providers) { } } +// ─── P5: providers.json schema validation ─────────────────────────────────── +// A malformed slot does not announce itself — it fails at dispatch, mid-quorum, as an +// opaque UNAVAIL. This repo has shipped that failure repeatedly: a missing +// `args_template` crashed every slot on `.map` (ARGS-TEMPLATE-01), and a slot whose +// `cli`/`mainTool` were both empty handed `null` to spawn() and took the whole quorum +// offline (#197). Both are visible in the config before anything is dispatched. +// +// Errors are things that WILL fail at dispatch. Warnings are things that degrade +// diagnosis but still run. Validation never throws and never blocks — it reports. +function validateProviders(providers) { + const errors = []; + const warnings = []; + const list = Array.isArray(providers) ? providers : []; + + const seen = new Map(); + for (const [i, p] of list.entries()) { + const where = (p && p.name) ? `slot '${p.name}'` : `slot #${i}`; + if (!p || typeof p !== 'object') { errors.push(`${where}: entry is not an object`); continue; } + if (!p.name || typeof p.name !== 'string') { errors.push(`${where}: missing a string 'name'`); continue; } + + // Duplicate names are silently last-wins everywhere downstream, so one slot's + // config quietly becomes another's. + if (seen.has(p.name)) errors.push(`slot '${p.name}': duplicate name (also at #${seen.get(p.name)})`); + else seen.set(p.name, i); + + if (!p.type) { errors.push(`slot '${p.name}': missing 'type' (expected 'subprocess' or 'http')`); continue; } + + if (p.type === 'subprocess') { + // resolveSpawnTarget falls back cli -> mainTool; with neither, spawn() gets null. + if (!p.cli && !p.mainTool) errors.push(`slot '${p.name}': subprocess slot has neither 'cli' nor 'mainTool' — spawn would receive null`); + // args_template may be absent IF the family has a canonical default (#275). + if (p.args_template !== undefined && !Array.isArray(p.args_template)) { + errors.push(`slot '${p.name}': 'args_template' must be an array`); + } else if (Array.isArray(p.args_template) && + !p.args_template.some(a => typeof a === 'string' && a.includes('{prompt}'))) { + errors.push(`slot '${p.name}': 'args_template' has no {prompt} placeholder — the prompt would be silently dropped`); + } + } else if (p.type === 'http') { + if (!p.baseUrl) errors.push(`slot '${p.name}': http slot missing 'baseUrl'`); + if (!p.apiKeyEnv && !(p.env && (p.env.ANTHROPIC_AUTH_TOKEN || p.env.ANTHROPIC_API_KEY))) { + errors.push(`slot '${p.name}': http slot has no 'apiKeyEnv' and no auth token in 'env'`); + } + } else { + errors.push(`slot '${p.name}': unknown type '${p.type}'`); + } + + // Warnings — degraded diagnosis, still dispatchable. + for (const field of ['idle_timeout_ms', 'hard_timeout_ms', 'quorum_timeout_ms', 'ttfb_timeout_ms', 'inter_chunk_ceiling_ms']) { + const v = p[field]; + if (v === undefined) continue; + if (typeof v !== 'number' || !(v > 0) || !Number.isFinite(v)) { + warnings.push(`slot '${p.name}': '${field}' is not a positive number (${JSON.stringify(v)}) — it will be ignored`); + } + } + if (p.type === 'subprocess' && p.env && p.env.ANTHROPIC_BASE_URL && !p.deep_probe) { + warnings.push(`slot '${p.name}': routed to a third-party base URL with no 'deep_probe' — a quota/auth-dead slot cannot be told from a healthy one before dispatch`); + } + } + + return { valid: errors.length === 0, errors, warnings }; +} + +// ─── P3: the degraded-panel gate as a machine field ───────────────────────── +// "available < max_quorum_size -> BLOCK unless --force-quorum" was prose in +// commands/nf/quorum.md for an LLM to interpret. A gate that depends on a model +// reading a paragraph correctly is not a gate. This computes it, so the orchestrator +// reads a decision instead of making one. +// +// The invariant it encodes: when `blocked` is true, no dispatch may proceed without a +// recorded waiver. `min_live_voters` is the floor BELOW max_quorum_size — a roster can +// satisfy the floor while still being degraded, and those are different states. +function computeQuorumGate({ availableCount, maxQuorumSize, minLiveVoters = 2, forceQuorum = false }) { + const available = Number.isFinite(availableCount) ? availableCount : 0; + const max = Number.isFinite(maxQuorumSize) && maxQuorumSize >= 1 ? maxQuorumSize : 3; + const floor = Number.isFinite(minLiveVoters) && minLiveVoters >= 1 ? minLiveVoters : 2; + + const quorum_met = available >= max; + const meets_floor = available >= floor; + const degraded = !quorum_met && meets_floor; + const wouldBlock = !meets_floor; + + let gate_reason; + if (quorum_met) gate_reason = `${available} available >= max_quorum_size ${max}`; + else if (wouldBlock) gate_reason = `${available} available < min_live_voters ${floor} — below the floor`; + else gate_reason = `${available} available < max_quorum_size ${max} but >= min_live_voters ${floor} — degraded`; + + return { + available_count: available, + max_quorum_size: max, + min_live_voters: floor, + quorum_met, + degraded, + blocked: wouldBlock && !forceQuorum, + waiver_required: wouldBlock, + waiver_used: wouldBlock && forceQuorum, + gate_reason, + }; +} + // ─── Main ─────────────────────────────────────────────────────────────────── async function main() { const mode = process.argv[2] || '--all'; @@ -548,7 +647,13 @@ async function main() { const team = buildTeam(providers, active); const maxSize = cfg.max_quorum_size ?? 3; - const output = { quorum_active: active, max_quorum_size: maxSize, team }; + // P5: non-fatal diagnostics — surfaced on every --all so a malformed slot is seen + // before it fails opaquely mid-quorum, never by blocking the preflight itself. + const validation = validateProviders(providers); + for (const e of validation.errors) process.stderr.write(`[quorum-preflight] CONFIG ERROR: ${e}\n`); + for (const w of validation.warnings) process.stderr.write(`[quorum-preflight] config warning: ${w}\n`); + + const output = { quorum_active: active, max_quorum_size: maxSize, team, validation }; if (PROBE) { // Filter providers to only active ones @@ -599,6 +704,17 @@ async function main() { } } + // P3: emit the gate as a decided field rather than leaving the orchestrator to + // re-derive "available < max_quorum_size -> BLOCK unless --force-quorum" from + // prose. `blocked: true` means no dispatch without a recorded waiver. + output.gate = computeQuorumGate({ + availableCount: output.available_slots.length, + maxQuorumSize: maxSize, + minLiveVoters: (cfg.quorum && cfg.quorum.min_live_voters) || 2, + forceQuorum: process.argv.includes('--force-quorum'), + }); + process.stderr.write(`[quorum-preflight] gate: ${output.gate.blocked ? 'BLOCKED' : output.gate.degraded ? 'DEGRADED' : 'OK'} — ${output.gate.gate_reason}\n`); + // NOTE: nf-prompt.js also tiers slots independently via auth_type in its // quorum injection logic. This sort covers the quorum.md direct-read path // (workflows that consume preflight JSON output directly). @@ -646,4 +762,4 @@ if (require.main === module) { main().catch(e => { console.error(e); process.exit(1); }); } -module.exports = { dedupBySlotIdentity, probeHealth, findProviders }; +module.exports = { dedupBySlotIdentity, probeHealth, findProviders, validateProviders, computeQuorumGate }; diff --git a/package.json b/package.json index a68e760f31..c2925fe16a 100644 --- a/package.json +++ b/package.json @@ -114,7 +114,7 @@ "secrets:history": "bash scripts/secret-audit.sh", "test": "npm run test:ci && npm run test:tui && npm run test:formal", "test:changed": "node bin/test-changed.cjs", - "test:ci": "node scripts/lint-isolation.js && node scripts/verify-hooks-sync.cjs && node --test hooks/nf-precompact.test.js hooks/nf-context-monitor.test.js hooks/nf-session-start.test.js bin/conformance-schema.test.cjs bin/resolve-cli.test.cjs bin/secrets.test.cjs bin/verify-quorum-health.test.cjs hooks/nf-stop.test.js hooks/config-loader.test.js hooks/config-loader-validate-adversarial.test.cjs hooks/config-loader-load-adversarial.test.cjs hooks/config-loader-adversarial2.test.cjs core/bin/nf-tools.test.cjs hooks/nf-circuit-breaker.test.js hooks/nf-prompt.test.js bin/update-scoreboard.test.cjs bin/quota-reset-cooldown.test.cjs bin/update-scoreboard-recording-adversarial.test.cjs bin/update-scoreboard-availability-adversarial.test.cjs bin/update-scoreboard-adversarial2.test.cjs hooks/nf-statusline.test.js bin/review-mcp-logs.test.cjs bin/migrate-to-slots.test.cjs bin/validate-traces.test.cjs bin/write-check-result.test.cjs bin/check-results-exit.test.cjs bin/check-trace-redaction.test.cjs bin/check-trace-schema-drift.test.cjs bin/nForma.test.cjs bin/set-secret.test.cjs bin/issue-classifier.test.cjs bin/generate-tla-cfg.test.cjs bin/ccr-secure-config.test.cjs bin/nf-quorum-slot-worker-improvements.test.cjs bin/quorum-improvements-signal.test.cjs bin/quorum-checkpoint.test.cjs bin/claude-md-references.test.cjs hooks/nf-spec-regen.test.js bin/propose-debug-invariants.test.cjs bin/proximity-embed-lazy-load.test.cjs bin/aggregate-requirements.test.cjs bin/validate-invariant.test.cjs bin/validate-memory.test.cjs bin/validate-requirements-haiku.test.cjs bin/call-quorum-slot-retry.test.cjs bin/provider-mapping.test.cjs bin/provider-concurrency.test.cjs bin/token-dashboard.test.cjs bin/batch7-guards.test.cjs bin/execution-progress.test.cjs bin/memory-store.test.cjs hooks/nf-session-end.test.js bin/skill-extractor.test.cjs bin/learning-extractor.test.cjs bin/continuous-verify.test.cjs bin/context-retriever.test.cjs bin/context-stack.test.cjs hooks/nf-destructive-git-guard.test.js bin/worktree-merge.test.cjs bin/call-quorum-slot-latency.test.cjs bin/nforma-cli.test.cjs bin/solve-trend.test.cjs bin/dedup-changelog.test.cjs bin/changelog-sections.test.cjs bin/oscillation-detector.test.cjs bin/gate-stability.test.cjs bin/predictive-power.test.cjs bin/convergence-report.test.cjs bin/escalation-classifier.test.cjs test/formal-scope-scan-semantic.test.cjs bin/repowise/escape-xml.test.cjs bin/repowise/pack-file.test.cjs bin/repowise/context-packer.test.cjs bin/repowise/hotspot.test.cjs bin/repowise/resolve-hotspot-risk.test.cjs bin/repowise/cochange.test.cjs bin/repowise/inject-cochange-debug.test.cjs bin/repowise/skeleton.test.cjs bin/repowise/budget-compressor.test.cjs test/providers-path-consolidation.test.cjs test/quorum-consensus-gate-cluster.test.cjs test/quorum-preflight-maintool-healthy.test.cjs test/quorum-preflight-probe.test.cjs test/quorum-preflight-dedup.test.cjs test/quorum-preflight-roster-adversarial.test.cjs test/quorum-preflight-roster-adversarial2.test.cjs test/quorum-preflight-probe-adversarial.test.cjs test/quorum-preflight-binary-cache.test.cjs test/resolve-providers-order.test.cjs test/proximity-resolve-guards.test.cjs test/observe-polyrepo-guards.test.cjs bin/no-side-effects-on-require.test.cjs bin/no-shipped-user-config.test.cjs bin/install-providers-precedence.test.cjs bin/alloy-exec.test.cjs test/install-mcp-nfbin.test.cjs bin/semaphore-concurrency.test.cjs bin/quorum-dispatch-argv.test.cjs bin/quorum-slot-dispatch.test.cjs bin/quorum-content-verdict-integrity.test.cjs bin/quorum-slot-authenticity.test.cjs bin/call-quorum-slot-cooldown-path.test.cjs hooks/nf-prompt-cache-budget.test.js bin/coderlm-adapter-filter.test.cjs bin/coderlm-skill-blocks.test.cjs bin/solve-commit-artifacts.test.cjs bin/solve-debt-bridge.test.cjs test/baseline-drift.test.cjs bin/cli-robustness.test.cjs bin/mcp-skill-embedded.test.cjs bin/skill-eval-args.test.cjs bin/observe-handler-upstream.test.cjs bin/skill-mcp-tool-names.test.cjs bin/skill-command-correctness.test.cjs bin/mcp-update-classify.test.cjs bin/formal-test-sync-safety.test.cjs bin/skill-papercut-sweep.test.cjs bin/roadmap-health-parser.test.cjs bin/mcp-setup-env-order.test.cjs bin/sync-baseline-dry-run.test.cjs bin/skill-cli-sweep.test.cjs bin/validate-requirements-staleness.test.cjs bin/audit-allowlist-precision.test.cjs bin/skill-eval-lint.test.cjs bin/machine-path-lint.test.cjs bin/release-guards.test.cjs bin/hooks-dist-completeness.test.cjs bin/quorum-resume.test.cjs bin/autonomous-grind.test.cjs bin/goal-writer-blocks.test.cjs bin/skill-mcp-lint.test.cjs bin/skill-invocation-lint.test.cjs bin/skill-state-guards.test.cjs bin/skill-state-parse-lint.test.cjs bin/map-codebase-secret-scan.test.cjs bin/verify-work-falsepass.test.cjs bin/solve-tui-itemkey.test.cjs bin/provider-arg-templates.test.cjs test/unified-mcp-server-slot-health.test.cjs bin/call-quorum-slot-stall-timeout.test.cjs bin/telemetry-window-source.test.cjs bin/mcp-repair-statusline-refresh.test.cjs bin/observe-config.test.cjs bin/observe-debt-writer.test.cjs bin/observe-handler-deps.test.cjs bin/observe-handler-grafana.test.cjs bin/observe-handler-logstash.test.cjs bin/observe-handler-prometheus.test.cjs bin/observe-handler-session-insights.test.cjs bin/observe-handlers.test.cjs bin/observe-registry.test.cjs bin/observe-render.test.cjs bin/observe-solve-pipe.test.cjs bin/observe-tooling.test.cjs bin/observed-fsm.test.cjs bin/observe-handler-internal.test.cjs bin/observe-pipeline.test.cjs bin/delegate-session.test.cjs bin/delegate-session-store.test.cjs bin/delegate-auditor.test.cjs bin/delegate-loop.test.cjs bin/coding-task-router.test.cjs", + "test:ci": "node scripts/lint-isolation.js && node scripts/verify-hooks-sync.cjs && node --test hooks/nf-precompact.test.js hooks/nf-context-monitor.test.js hooks/nf-session-start.test.js bin/conformance-schema.test.cjs bin/resolve-cli.test.cjs bin/secrets.test.cjs bin/verify-quorum-health.test.cjs hooks/nf-stop.test.js hooks/config-loader.test.js hooks/config-loader-validate-adversarial.test.cjs hooks/config-loader-load-adversarial.test.cjs hooks/config-loader-adversarial2.test.cjs core/bin/nf-tools.test.cjs hooks/nf-circuit-breaker.test.js hooks/nf-prompt.test.js bin/update-scoreboard.test.cjs bin/quota-reset-cooldown.test.cjs bin/update-scoreboard-recording-adversarial.test.cjs bin/update-scoreboard-availability-adversarial.test.cjs bin/update-scoreboard-adversarial2.test.cjs hooks/nf-statusline.test.js bin/review-mcp-logs.test.cjs bin/migrate-to-slots.test.cjs bin/validate-traces.test.cjs bin/write-check-result.test.cjs bin/check-results-exit.test.cjs bin/check-trace-redaction.test.cjs bin/check-trace-schema-drift.test.cjs bin/nForma.test.cjs bin/set-secret.test.cjs bin/issue-classifier.test.cjs bin/generate-tla-cfg.test.cjs bin/ccr-secure-config.test.cjs bin/nf-quorum-slot-worker-improvements.test.cjs bin/quorum-improvements-signal.test.cjs bin/quorum-checkpoint.test.cjs bin/claude-md-references.test.cjs hooks/nf-spec-regen.test.js bin/propose-debug-invariants.test.cjs bin/proximity-embed-lazy-load.test.cjs bin/aggregate-requirements.test.cjs bin/validate-invariant.test.cjs bin/validate-memory.test.cjs bin/validate-requirements-haiku.test.cjs bin/call-quorum-slot-retry.test.cjs bin/provider-mapping.test.cjs bin/provider-concurrency.test.cjs bin/token-dashboard.test.cjs bin/batch7-guards.test.cjs bin/execution-progress.test.cjs bin/memory-store.test.cjs hooks/nf-session-end.test.js bin/skill-extractor.test.cjs bin/learning-extractor.test.cjs bin/continuous-verify.test.cjs bin/context-retriever.test.cjs bin/context-stack.test.cjs hooks/nf-destructive-git-guard.test.js bin/worktree-merge.test.cjs bin/call-quorum-slot-latency.test.cjs bin/nforma-cli.test.cjs bin/solve-trend.test.cjs bin/dedup-changelog.test.cjs bin/changelog-sections.test.cjs bin/oscillation-detector.test.cjs bin/gate-stability.test.cjs bin/predictive-power.test.cjs bin/convergence-report.test.cjs bin/escalation-classifier.test.cjs test/formal-scope-scan-semantic.test.cjs bin/repowise/escape-xml.test.cjs bin/repowise/pack-file.test.cjs bin/repowise/context-packer.test.cjs bin/repowise/hotspot.test.cjs bin/repowise/resolve-hotspot-risk.test.cjs bin/repowise/cochange.test.cjs bin/repowise/inject-cochange-debug.test.cjs bin/repowise/skeleton.test.cjs bin/repowise/budget-compressor.test.cjs test/providers-path-consolidation.test.cjs test/quorum-consensus-gate-cluster.test.cjs test/quorum-preflight-maintool-healthy.test.cjs test/quorum-preflight-probe.test.cjs test/quorum-preflight-dedup.test.cjs bin/preflight-gate-validator.test.cjs test/quorum-preflight-roster-adversarial.test.cjs test/quorum-preflight-roster-adversarial2.test.cjs test/quorum-preflight-probe-adversarial.test.cjs test/quorum-preflight-binary-cache.test.cjs test/resolve-providers-order.test.cjs test/proximity-resolve-guards.test.cjs test/observe-polyrepo-guards.test.cjs bin/no-side-effects-on-require.test.cjs bin/no-shipped-user-config.test.cjs bin/install-providers-precedence.test.cjs bin/alloy-exec.test.cjs test/install-mcp-nfbin.test.cjs bin/semaphore-concurrency.test.cjs bin/quorum-dispatch-argv.test.cjs bin/quorum-slot-dispatch.test.cjs bin/quorum-content-verdict-integrity.test.cjs bin/quorum-slot-authenticity.test.cjs bin/call-quorum-slot-cooldown-path.test.cjs hooks/nf-prompt-cache-budget.test.js bin/coderlm-adapter-filter.test.cjs bin/coderlm-skill-blocks.test.cjs bin/solve-commit-artifacts.test.cjs bin/solve-debt-bridge.test.cjs test/baseline-drift.test.cjs bin/cli-robustness.test.cjs bin/mcp-skill-embedded.test.cjs bin/skill-eval-args.test.cjs bin/observe-handler-upstream.test.cjs bin/skill-mcp-tool-names.test.cjs bin/skill-command-correctness.test.cjs bin/mcp-update-classify.test.cjs bin/formal-test-sync-safety.test.cjs bin/skill-papercut-sweep.test.cjs bin/roadmap-health-parser.test.cjs bin/mcp-setup-env-order.test.cjs bin/sync-baseline-dry-run.test.cjs bin/skill-cli-sweep.test.cjs bin/validate-requirements-staleness.test.cjs bin/audit-allowlist-precision.test.cjs bin/skill-eval-lint.test.cjs bin/machine-path-lint.test.cjs bin/release-guards.test.cjs bin/hooks-dist-completeness.test.cjs bin/quorum-resume.test.cjs bin/autonomous-grind.test.cjs bin/goal-writer-blocks.test.cjs bin/skill-mcp-lint.test.cjs bin/skill-invocation-lint.test.cjs bin/skill-state-guards.test.cjs bin/skill-state-parse-lint.test.cjs bin/map-codebase-secret-scan.test.cjs bin/verify-work-falsepass.test.cjs bin/solve-tui-itemkey.test.cjs bin/provider-arg-templates.test.cjs test/unified-mcp-server-slot-health.test.cjs bin/call-quorum-slot-stall-timeout.test.cjs bin/telemetry-window-source.test.cjs bin/mcp-repair-statusline-refresh.test.cjs bin/observe-config.test.cjs bin/observe-debt-writer.test.cjs bin/observe-handler-deps.test.cjs bin/observe-handler-grafana.test.cjs bin/observe-handler-logstash.test.cjs bin/observe-handler-prometheus.test.cjs bin/observe-handler-session-insights.test.cjs bin/observe-handlers.test.cjs bin/observe-registry.test.cjs bin/observe-render.test.cjs bin/observe-solve-pipe.test.cjs bin/observe-tooling.test.cjs bin/observed-fsm.test.cjs bin/observe-handler-internal.test.cjs bin/observe-pipeline.test.cjs bin/delegate-session.test.cjs bin/delegate-session-store.test.cjs bin/delegate-auditor.test.cjs bin/delegate-loop.test.cjs bin/coding-task-router.test.cjs", "embed:build": "node bin/proximity-embed.mjs --cache-only", "test:tui": "NF_TEST_MODE=1 node scripts/run-tui-tests.cjs", "test:install": "node --test test/install-virgin.test.cjs", From c1c37d31194d7dd5a142a223c9993539218431ad Mon Sep 17 00:00:00 2001 From: "open-swe[bot]" Date: Sat, 8 Aug 2026 10:51:01 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix(quorum):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20validate=20the=20raw=20roster,=20gate=20after=20dedup,=20qui?= =?UTF-8?q?et=20the=20secrets=20scan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings, all valid. Two were the same shape: functions correct in isolation, wrong where they were wired. - The validator received findProviders()' output, which strips null/non-object/ nameless entries so the rest of preflight degrades gracefully. It therefore never saw the entries it exists to report: a corrupt `[null]` roster came back valid:true with no diagnostic. It now validates the RAW providers.json. My unit test proved the function HANDLES those inputs; nothing proved it ever received them, which is the gap that matters. - The gate counted output.available_slots BEFORE dedupBySlotIdentity() removes duplicate (model, display_provider) identities from that same array. It could therefore report a roster meeting the voter floor while the actually-dispatchable list did not — a gate passing on slots that will never be dispatched. Moved after dedup. - detect-secrets flagged `apiKeyEnv:` / `ANTHROPIC_AUTH_TOKEN:` in the fixtures as Secret Keyword. They are field NAMES in test data, not credentials; marked with the standard allowlist pragma rather than mutating the committed baseline. 2 wiring tests added that spawn the real preflight against a corrupt roster and assert both that the fault is REPORTED and that preflight still produces its team output (report, never block). Mutation-proven: pointing the validator back at the filtered roster fails PGV-12. Co-Authored-By: Claude Opus 5 (1M context) --- bin/preflight-gate-validator.test.cjs | 47 +++++++++++++++++++++++++-- bin/quorum-preflight.cjs | 35 +++++++++++++------- 2 files changed, 68 insertions(+), 14 deletions(-) diff --git a/bin/preflight-gate-validator.test.cjs b/bin/preflight-gate-validator.test.cjs index 4c19c138d9..162c59de6d 100644 --- a/bin/preflight-gate-validator.test.cjs +++ b/bin/preflight-gate-validator.test.cjs @@ -21,7 +21,7 @@ const { validateProviders, computeQuorumGate } = require('./quorum-preflight.cjs test('PGV-1: a well-formed roster is clean', () => { const r = validateProviders([ { name: 'codex-1', type: 'subprocess', mainTool: 'codex', args_template: ['exec', '{prompt}'] }, - { name: 'api-1', type: 'http', baseUrl: 'https://x/v1', apiKeyEnv: 'X_KEY' }, + { name: 'api-1', type: 'http', baseUrl: 'https://x/v1', apiKeyEnv: 'X_KEY' }, // pragma: allowlist secret ]); assert.deepStrictEqual(r.errors, []); assert.strictEqual(r.valid, true); @@ -48,11 +48,11 @@ test('PGV-3: duplicate names are an error, not last-wins', () => { }); test('PGV-4: http slots need a base URL and some auth', () => { - assert.match(validateProviders([{ name: 'h', type: 'http', apiKeyEnv: 'K' }]).errors.join(' '), /missing 'baseUrl'/); + assert.match(validateProviders([{ name: 'h', type: 'http', apiKeyEnv: 'K' }]).errors.join(' '), /missing 'baseUrl'/); // pragma: allowlist secret assert.match(validateProviders([{ name: 'h', type: 'http', baseUrl: 'https://x' }]).errors.join(' '), /no 'apiKeyEnv'/); // An inline token in env counts as auth. assert.deepStrictEqual( - validateProviders([{ name: 'h', type: 'http', baseUrl: 'https://x', env: { ANTHROPIC_AUTH_TOKEN: 't' } }]).errors, []); + validateProviders([{ name: 'h', type: 'http', baseUrl: 'https://x', env: { ANTHROPIC_AUTH_TOKEN: 't' } }]).errors, []); // pragma: allowlist secret }); test('PGV-5: garbage timeouts WARN rather than error — they are ignored, not fatal', () => { @@ -113,3 +113,44 @@ test('PGV-11: garbage inputs fall back to safe defaults instead of NaN', () => { assert.strictEqual(g.min_live_voters, 2); assert.strictEqual(g.blocked, true, 'zero available must block, not NaN-compare to false'); }); + +// ── Wiring: the two orderings that made these functions look correct in isolation +// while being wrong in the pipeline. Both were review findings on the first version. + +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); + +const TMP = []; +process.on('exit', () => { for (const d of TMP) { try { fs.rmSync(d, { recursive: true, force: true }); } catch (_) {} } }); + +function runPreflight(providers, argv = []) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'nf-pf-')); + TMP.push(dir); + const pj = path.join(dir, 'providers.json'); + fs.writeFileSync(pj, JSON.stringify({ providers }), 'utf8'); + const res = spawnSync(process.execPath, [ + path.join(__dirname, 'quorum-preflight.cjs'), '--all', '--no-probe', ...argv, + ], { cwd: dir, encoding: 'utf8', timeout: 60000, + env: { ...process.env, UNIFIED_PROVIDERS_CONFIG: pj } }); + let json = null; + try { json = JSON.parse((res.stdout || '').split('\n').filter(Boolean).pop()); } catch (_) {} + return { json, stderr: res.stderr || '' }; +} + +test('PGV-12: a corrupt roster entry reaches the validator, not just the filter', () => { + // findProviders() strips null/nameless entries so the rest of preflight degrades + // gracefully — which meant the validator never saw them and reported valid:true on + // a roster it had never actually inspected. + const { json, stderr } = runPreflight([null, { name: 'ok', type: 'subprocess', mainTool: 'x', args_template: ['{prompt}'] }]); + assert.ok(json, 'preflight produced no JSON'); + assert.strictEqual(json.validation.valid, false, 'a [null] entry must be reported, not silently filtered'); + assert.match(stderr, /CONFIG ERROR/); +}); + +test('PGV-13: a corrupt entry does not wedge the preflight (still fail-open)', () => { + // The validator reports; it must never block. The rest of the output must survive. + const { json } = runPreflight([null, { name: 'ok', type: 'subprocess', mainTool: 'x', args_template: ['{prompt}'] }]); + assert.ok(json.team, 'team output must still be produced alongside the diagnostic'); +}); diff --git a/bin/quorum-preflight.cjs b/bin/quorum-preflight.cjs index b900e24889..e509afe77f 100644 --- a/bin/quorum-preflight.cjs +++ b/bin/quorum-preflight.cjs @@ -649,7 +649,14 @@ async function main() { // P5: non-fatal diagnostics — surfaced on every --all so a malformed slot is seen // before it fails opaquely mid-quorum, never by blocking the preflight itself. - const validation = validateProviders(providers); + // Validate the RAW roster, not findProviders()' output: that function strips + // null/non-object/nameless entries so the rest of preflight degrades gracefully, + // which means the validator would never see the entries it exists to report. A + // corrupt `[null]` roster reported valid:true and said nothing. + const rawProviders = (() => { + try { return loadProviders({ baseDir: __dirname }) || []; } catch (_) { return providers; } + })(); + const validation = validateProviders(rawProviders); for (const e of validation.errors) process.stderr.write(`[quorum-preflight] CONFIG ERROR: ${e}\n`); for (const w of validation.warnings) process.stderr.write(`[quorum-preflight] config warning: ${w}\n`); @@ -704,16 +711,6 @@ async function main() { } } - // P3: emit the gate as a decided field rather than leaving the orchestrator to - // re-derive "available < max_quorum_size -> BLOCK unless --force-quorum" from - // prose. `blocked: true` means no dispatch without a recorded waiver. - output.gate = computeQuorumGate({ - availableCount: output.available_slots.length, - maxQuorumSize: maxSize, - minLiveVoters: (cfg.quorum && cfg.quorum.min_live_voters) || 2, - forceQuorum: process.argv.includes('--force-quorum'), - }); - process.stderr.write(`[quorum-preflight] gate: ${output.gate.blocked ? 'BLOCKED' : output.gate.degraded ? 'DEGRADED' : 'OK'} — ${output.gate.gate_reason}\n`); // NOTE: nf-prompt.js also tiers slots independently via auth_type in its // quorum injection logic. This sort covers the quorum.md direct-read path @@ -740,6 +737,22 @@ async function main() { output.available_slots = deduped; output.deduped_slots = dedupedOut; + // P3: emit the gate as a decided field rather than leaving the orchestrator to + // re-derive "available < max_quorum_size -> BLOCK unless --force-quorum" from + // prose. `blocked: true` means no dispatch without a recorded waiver. + // + // Computed AFTER dedup deliberately: dedup removes duplicate (model, provider) + // identities from available_slots, so counting before it would report a roster + // that meets the voter floor while the actually-dispatchable list does not — + // a gate that passes on slots that will never be dispatched. + output.gate = computeQuorumGate({ + availableCount: output.available_slots.length, + maxQuorumSize: maxSize, + minLiveVoters: (cfg.quorum && cfg.quorum.min_live_voters) || 2, + forceQuorum: process.argv.includes('--force-quorum'), + }); + process.stderr.write(`[quorum-preflight] gate: ${output.gate.blocked ? 'BLOCKED' : output.gate.degraded ? 'DEGRADED' : 'OK'} — ${output.gate.gate_reason}\n`); + // Add transparency fields output.primary_slots = output.available_slots.filter(s => typeMap.get(s) !== 'http'); output.backup_slots = output.available_slots.filter(s => typeMap.get(s) === 'http').concat(dedupedOut);