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
6 changes: 3 additions & 3 deletions .secrets.baseline

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 14 additions & 1 deletion bin/call-quorum-slot.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const path = require('path');
const os = require('os');
const { resolveCli, resolveSpawnTarget } = require('./resolve-cli.cjs');
const { loadProviders } = require('./resolve-providers.cjs');
const { resolveArgsTemplate } = require('./provider-arg-templates.cjs');
const { acquireSlot, releaseSlot, providerKeyFromUrl } = require('./provider-concurrency.cjs');
const { resolveEnvPlaceholders, findUnresolvedPlaceholders, isPlaceholder, extractPlaceholderVar, namespacedSecretKey } = require('./resolve-env.cjs');
const { warnUnknownDispatchFlags } = require('./quorum-dispatch-argv.cjs'); // #202: parent→child argv contract
Expand Down Expand Up @@ -420,7 +421,19 @@ function buildSpawnArgs(provider, prompt, allowedToolsFlag) {
// flag (and a stderr warning in runSubprocess) until CCR can accept the prompt
// over stdin/temp-file without shell re-interpretation.
const promptMutated = Boolean(isCcr) && safePrompt !== prompt;
args = provider.args_template.map(a => (a === '{prompt}' ? safePrompt : a));
// ARGS-TEMPLATE-01: providers.json entries from the auto-detect install path can
// lack args_template; `.map` on undefined crashed every slot opaquely. Fall back
// to the canonical per-family template (by mainTool), and fail LOUD with an
// actionable message if the family is unknown — never the bare TypeError.
const argsTemplate = resolveArgsTemplate(provider);
if (!Array.isArray(argsTemplate)) {
throw new Error(
`provider ${provider.name || '(unnamed)'} has no args_template and family ` +
`'${provider.mainTool || '?'}' has no canonical default — regenerate providers.json ` +
`(run install or /nf:mcp-setup), or add an args_template to this slot.`
);
}
args = argsTemplate.map(a => (a === '{prompt}' ? safePrompt : a));
// Only use stdin for slots that explicitly require it (none currently do)
useStdinPrompt = false;

Expand Down
62 changes: 49 additions & 13 deletions bin/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,8 @@ function ensureMcpSlotsFromProviders() {
// Step 1: Sync slots from ~/.claude.json (fan-out creates MCP entries here)
// Only include slots whose prefix is a known coding agent CLI.
const KNOWN_CLI_PREFIXES = ['claude', 'codex', 'gemini', 'opencode', 'copilot', 'kilo', 'cursor', 'windsurf', 'antigravity', 'augment', 'trae', 'cline'];
const { resolveCli } = require('./resolve-cli.cjs');
const { argsTemplateFor } = require('./provider-arg-templates.cjs');
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
const claudeJsonPath = path.join(os.homedir(), '.claude.json');
const claudeJson = JSON.parse(fs.readFileSync(claudeJsonPath, 'utf8'));
const mcpSlots = Object.keys(claudeJson.mcpServers || {}).filter(name =>
Expand All @@ -632,6 +634,21 @@ function ensureMcpSlotsFromProviders() {
display_type: mainTool + '-cli',
display_provider: mainTool.charAt(0).toUpperCase() + mainTool.slice(1),
};
// ARGS-TEMPLATE-01: emit the canonical per-family invocation template so
// call-quorum-slot / unified-mcp-server don't crash on a missing field.
// Only for families with a KNOWN template — KNOWN_CLI_PREFIXES also admits
// IDE runtimes (kilo/cursor/windsurf/augment/trae/cline) that are NOT quorum
// CLIs; writing a guessed `-p {prompt}` for those would be wrong. Leave the
// field absent for unknown families so the consumer fail-loud contract applies.
const _tmpl = argsTemplateFor(mainTool);
if (_tmpl) base.args_template = _tmpl;
// antigravity's family name ≠ its binary (`agy`); record the resolved binary
// so spawn targets `agy`, never a non-existent `antigravity`. Persist it even
// when resolution falls back to bare `agy` (better an explicit ENOENT on `agy`
// than silently spawning the wrong mainTool).
if (mainTool === 'antigravity' && !base.cli) {
base.cli = resolveCli('agy');
}
const existing = existingByName.get(slotName);
// Overlay existing on top of base (preserves Daintree metadata, env, etc.)
// BUT force `name` and `mainTool` from the base (slot-name-derived). Otherwise
Expand All @@ -641,23 +658,35 @@ function ensureMcpSlotsFromProviders() {
? { ...base, ...existing, name: base.name, mainTool: base.mainTool }
: base);
}
// Step 2: Also add PATH-detected CLIs that aren't already in ~/.claude.json
const { resolveCli } = require('./resolve-cli.cjs');
const KNOWN_CLIS = ['claude', 'codex', 'gemini', 'opencode', 'copilot'];
// Step 2: Also add PATH-detected CLIs that aren't already in ~/.claude.json.
// Each family's binary name usually matches the family, except antigravity
// (family `antigravity` ships the `agy` binary) — so resolve by binary but
// record the family name + its canonical args_template.
const KNOWN_CLIS = [
{ family: 'claude', bin: 'claude' },
{ family: 'codex', bin: 'codex' },
{ family: 'gemini', bin: 'gemini' },
{ family: 'opencode', bin: 'opencode' },
{ family: 'copilot', bin: 'copilot' },
{ family: 'antigravity', bin: 'agy' },
];
const existingNames = new Set(mcpSlots);
for (const name of KNOWN_CLIS) {
if (existingNames.has(name + '-1')) continue; // Already covered by step 1
const resolved = resolveCli(name);
if (resolved !== name) {
for (const { family, bin } of KNOWN_CLIS) {
if (existingNames.has(family + '-1')) continue; // Already covered by step 1
const resolved = resolveCli(bin);
if (resolved !== bin) {
merged.providers.push({
name: name + '-1',
name: family + '-1',
provider: 'auto-detected',
type: 'subprocess',
description: `Auto-detected ${name} on PATH`,
mainTool: name,
description: `Auto-detected ${family} on PATH`,
mainTool: family,
cli: resolved,
display_type: name + '-cli',
display_provider: name.charAt(0).toUpperCase() + name.slice(1),
display_type: family + '-cli',
display_provider: family.charAt(0).toUpperCase() + family.slice(1),
// All Step-2 families have a canonical template; argsTemplateFor is
// authoritative (no guessed fallback).
args_template: argsTemplateFor(family),
});
}
}
Expand Down Expand Up @@ -3181,12 +3210,19 @@ function install(isGlobal, runtime = 'claude') {
// ── MIGRATION: remove old nf-* hook entries ─────────────────────────
// Old installs registered hooks as nf-prompt.js, nf-stop.js, nf-circuit-breaker.js.
// Remove them so the nf-* replacements below can register cleanly.
// These map an event to OLD/renamed hook names whose stale registrations must
// be stripped. The matching CURRENT hooks (nf-prompt, nf-stop, …) are
// re-registered AFTER this migration, so removing them here is harmless.
// nf-session-start is the EXCEPTION: it is registered ABOVE (before this
// migration) and never re-added after, so listing it here removed the live
// hook on every install — silently disabling SessionStart (incl. the
// post-compaction continuation injection). It is NOT an old name; do not list it.
const OLD_HOOK_MAP = {
UserPromptSubmit: 'nf-prompt',
Stop: 'nf-stop',
PreToolUse: 'nf-circuit-breaker',
PostToolUse: ['nf-spec-regen', 'nf-context-monitor', 'nf-context-monitor'],
SessionStart: ['nf-check-update', 'nf-session-start'],
SessionStart: ['nf-check-update'],
};
for (const [event, oldNames] of Object.entries(OLD_HOOK_MAP)) {
if (settings.hooks[event]) {
Expand Down
15 changes: 13 additions & 2 deletions bin/probe-quorum-slots.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const os = require('os');
const { resolveSpawnTarget } = require('./resolve-cli.cjs');
const { resolveEnvPlaceholders } = require('./resolve-env.cjs');
const { loadProviders } = require('./resolve-providers.cjs');
const { resolveArgsTemplate } = require('./provider-arg-templates.cjs');

// ─── Find providers.json ─────────────────────────────────────────────────────
// Delegates to the single source of truth in resolve-providers.cjs (issue #197).
Expand All @@ -51,8 +52,18 @@ function probeSlot(provider, timeoutMs, spawnCwd) {
return new Promise((resolve) => {
const start = Date.now();

// Replace {prompt} with the minimal probe string
const args = provider.args_template.map(a => (a === '{prompt}' ? 'OK' : a));
// Replace {prompt} with the minimal probe string. Use the canonical per-family
// template when the entry lacks args_template; if NEITHER resolves (unknown
// family), report an explicit config error rather than probing with empty argv
// (which would dispatch no prompt and read as a false success/usage banner).
const argsTemplate = resolveArgsTemplate(provider);
if (!Array.isArray(argsTemplate)) {
return resolve({
slot: provider.name, healthy: false, latencyMs: Date.now() - start,
error: `no args_template and family '${provider.mainTool || '?'}' has no canonical default — fix providers.json`,
});
}
const args = argsTemplate.map(a => (a === '{prompt}' ? 'OK' : a));
// Resolve ${VAR} placeholders in provider.env (issue parity with
// call-quorum-slot.cjs:462 / unified-mcp-server.mjs) — a secret-placeholder
// slot was previously probed with an UNRESOLVED env and false-failed.
Expand Down
63 changes: 63 additions & 0 deletions bin/provider-arg-templates.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'use strict';

/**
* provider-arg-templates.cjs — canonical per-family CLI args_template map.
*
* Single source of truth for how each quorum CLI family is invoked non-interactively
* (the `{prompt}` placeholder is substituted with the actual prompt at dispatch time).
*
* WHY THIS EXISTS: providers.json entries are supposed to carry an `args_template`
* field, but the install-time auto-detection path (bin/install.js provider sync)
* historically wrote entries WITHOUT it. Every consumer then did
* `provider.args_template.map(...)` unguarded — so a missing field crashed quorum
* dispatch with an opaque "Cannot read properties of undefined (reading 'map')"
* for ALL slots (call-quorum-slot.cjs, unified-mcp-server.mjs, probe-quorum-slots,
* provider-status). This map lets both the generator emit a correct default and the
* consumers fall back / fail loud instead of crashing.
*
* The values below are the canonical templates asserted by the guided-install path
* and dispatch tests (test/install-guided-providers.test.cjs, test/shell-escape-01.test.cjs)
* — they are recovered from those invariants, not guessed.
*/

const FAMILY_ARGS_TEMPLATE = {
// claude CLI: print mode, skip interactive permission prompts
claude: ['-p', '{prompt}', '--dangerously-skip-permissions'],
// codex CLI: non-interactive `exec` subcommand
codex: ['exec', '{prompt}'],
// gemini CLI: print mode (default model)
gemini: ['-p', '{prompt}'],
// opencode CLI: `run` with quiet logs so stdout is just the response
opencode: ['run', '--print-logs', '--log-level', 'ERROR', '{prompt}'],
// copilot CLI: print mode, auto-approve tools, no color, suppress spinner
copilot: ['-p', '{prompt}', '--allow-all-tools', '--no-color', '-s'],
// antigravity (`agy` binary): print mode (verified against agy v1.0.8 `-p`/--print)
antigravity: ['-p', '{prompt}'],
};

/**
* Return a fresh copy of the canonical args_template for a family, or null if the
* family is unknown. Callers should treat null as "cannot dispatch — fail loud".
* @param {string} mainTool - family name (e.g. 'gemini', 'codex', 'antigravity')
* @returns {string[]|null}
*/
function argsTemplateFor(mainTool) {
const t = FAMILY_ARGS_TEMPLATE[mainTool];
return t ? t.slice() : null;
}

/**
* Resolve the args_template to use for a provider: its own explicit field wins,
* otherwise the canonical family default by mainTool. Returns null when neither
* is available (unknown family + no explicit template) so callers can fail loud
* with a clear message instead of crashing on `.map` of undefined.
* @param {object} provider - a providers.json entry
* @returns {string[]|null}
*/
function resolveArgsTemplate(provider) {
if (provider && Array.isArray(provider.args_template)) return provider.args_template;
if (provider && provider.mainTool) return argsTemplateFor(provider.mainTool);
return null;
}

module.exports = { FAMILY_ARGS_TEMPLATE, argsTemplateFor, resolveArgsTemplate };
86 changes: 86 additions & 0 deletions bin/provider-arg-templates.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
'use strict';

// ARGS-TEMPLATE-01 regression suite.
//
// The install-time auto-detect path wrote providers.json entries WITHOUT an
// args_template field; every consumer then did `provider.args_template.map(...)`
// unguarded, so quorum dispatch crashed for ALL slots with an opaque
// "Cannot read properties of undefined (reading 'map')". This suite locks in:
// 1. the canonical per-family template map,
// 2. the explicit-field-wins / family-default / null resolution order,
// 3. that call-quorum-slot's buildSpawnArgs falls back to the family default
// and fails LOUD (clear message) on an unknown family — never the bare crash.

const { describe, it } = require('node:test');
const assert = require('node:assert/strict');

const { FAMILY_ARGS_TEMPLATE, argsTemplateFor, resolveArgsTemplate } = require('./provider-arg-templates.cjs');
const { buildSpawnArgs } = require('./call-quorum-slot.cjs');

describe('provider-arg-templates: canonical map', () => {
it('has a template for every quorum CLI family incl. antigravity', () => {
for (const fam of ['claude', 'codex', 'gemini', 'opencode', 'copilot', 'antigravity']) {
assert.ok(Array.isArray(FAMILY_ARGS_TEMPLATE[fam]), `${fam} must have a template`);
assert.ok(FAMILY_ARGS_TEMPLATE[fam].includes('{prompt}'), `${fam} template must carry {prompt}`);
}
});

it('codex uses exec; antigravity uses -p print mode', () => {
assert.deepEqual(argsTemplateFor('codex'), ['exec', '{prompt}']);
assert.deepEqual(argsTemplateFor('antigravity'), ['-p', '{prompt}']);
});

it('argsTemplateFor returns a fresh copy (no shared mutation) and null for unknown', () => {
const a = argsTemplateFor('gemini');
a.push('MUTATED');
assert.ok(!argsTemplateFor('gemini').includes('MUTATED'), 'must return a defensive copy');
assert.equal(argsTemplateFor('nope'), null);
});

it('IDE runtimes (kilo/cursor/windsurf/augment/trae/cline) have NO canonical template', () => {
// install.js admits these as KNOWN_CLI_PREFIXES but they are not quorum CLIs;
// they must NOT get a guessed `-p {prompt}` template (CodeRabbit #275).
for (const ide of ['kilo', 'cursor', 'windsurf', 'augment', 'trae', 'cline']) {
assert.equal(argsTemplateFor(ide), null, `${ide} must have no canonical template`);
}
});
});

describe('resolveArgsTemplate: resolution order', () => {
it('explicit args_template on the provider wins', () => {
const p = { mainTool: 'gemini', args_template: ['--custom', '{prompt}'] };
assert.deepEqual(resolveArgsTemplate(p), ['--custom', '{prompt}']);
});
it('falls back to the family default by mainTool', () => {
assert.deepEqual(resolveArgsTemplate({ mainTool: 'codex' }), ['exec', '{prompt}']);
});
it('returns null when neither explicit nor a known family is present', () => {
assert.equal(resolveArgsTemplate({ mainTool: 'mystery' }), null);
assert.equal(resolveArgsTemplate({}), null);
});
});

describe('buildSpawnArgs: no crash on a provider missing args_template', () => {
it('a slot with no args_template falls back to the family template (the crash regression)', () => {
const { args } = buildSpawnArgs({ name: 'gemini-1', mainTool: 'gemini' }, 'HELLO');
assert.deepEqual(args, ['-p', 'HELLO']);
});

it('antigravity slot (no args_template) dispatches via -p', () => {
const { args } = buildSpawnArgs({ name: 'antigravity-1', mainTool: 'antigravity', cli: '/x/agy' }, 'HI');
assert.deepEqual(args, ['-p', 'HI']);
});

it('an explicit args_template is still honored', () => {
const { args } = buildSpawnArgs({ name: 'codex-1', mainTool: 'codex', args_template: ['exec', '{prompt}'] }, 'Q');
assert.deepEqual(args, ['exec', 'Q']);
});

it('an unknown family with no template fails LOUD, not with an opaque TypeError', () => {
assert.throws(
() => buildSpawnArgs({ name: 'weird-1', mainTool: 'weird' }, 'x'),
(e) => /no args_template/.test(e.message) && !/undefined \(reading 'map'\)/.test(e.message),
'must throw a clear, actionable error'
);
});
});
13 changes: 12 additions & 1 deletion bin/provider-status.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const path = require('path');
const os = require('os');
const { resolveSpawnTarget } = require('./resolve-cli.cjs');
const { loadProviders: loadProvidersConfig } = require('./resolve-providers.cjs');
const { resolveArgsTemplate } = require('./provider-arg-templates.cjs');

// ─── Error pattern classification ────────────────────────────────────────────

Expand Down Expand Up @@ -184,7 +185,17 @@ async function probeProvider(provider, quick) {

// Deep probe: send actual prompt, check for PROBE_OK in output
const probe = provider.deep_probe ?? { prompt: 'respond with: PROBE_OK', expect: 'PROBE_OK', timeout_ms: 45000 };
const args = provider.args_template.map(a => (a === '{prompt}' ? probe.prompt : a));
// Resolve the args_template (explicit field → canonical family default). A null
// here means an unknown family with no template — report MISCONFIGURED explicitly
// rather than probing with empty argv (which would mask the config error).
const argsTemplate = resolveArgsTemplate(provider);
if (!Array.isArray(argsTemplate)) {
return {
slot: name, status: 'MISCONFIGURED', version: null, latencyMs: 0,
detail: `no args_template and family '${provider.mainTool || '?'}' has no canonical default — fix providers.json`,
};
}
const args = argsTemplate.map(a => (a === '{prompt}' ? probe.prompt : a));
const result = await runCommand(spawnTarget, args, probe.timeout_ms, provider.env);
const combined = result.output;

Expand Down
Loading
Loading