mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): surface codex config-parse failures and -32603 internal errors clearly (#1745)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
co-authored by
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
parent
e8df6ba694
commit
ec37625522
@@ -103,6 +103,14 @@ pub(crate) enum RequirementPayload {
|
||||
/// missing and the probe was skipped.
|
||||
availability: AcpAvailabilityStatus,
|
||||
},
|
||||
/// The CLI is installed but its config file could not be parsed.
|
||||
/// Informational only — no in-app action can fix an external config file.
|
||||
CliConfigInvalid {
|
||||
probe_args: Vec<String>,
|
||||
setup_copy: String,
|
||||
/// One-line stderr excerpt identifying the parse error.
|
||||
diagnostic: String,
|
||||
},
|
||||
}
|
||||
|
||||
impl RequirementPayload {
|
||||
@@ -149,6 +157,18 @@ impl RequirementPayload {
|
||||
format!("install {} (open Doctor in Settings to diagnose)", harness)
|
||||
}
|
||||
},
|
||||
RequirementPayload::CliConfigInvalid {
|
||||
probe_args,
|
||||
diagnostic,
|
||||
..
|
||||
} => {
|
||||
let cli = probe_args.first().map(String::as_str).unwrap_or("the CLI");
|
||||
let config_file = format!("~/.{}/config.toml", cli);
|
||||
format!(
|
||||
"{} is invalid: {} — fix the config and restart the agent",
|
||||
config_file, diagnostic
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -213,10 +233,32 @@ impl SetupPayload {
|
||||
.map(|r| format!("- {}", r.instruction()))
|
||||
.collect();
|
||||
|
||||
let all_external = self
|
||||
.requirements
|
||||
.iter()
|
||||
.all(|r| matches!(r, RequirementPayload::CliConfigInvalid { .. }));
|
||||
let any_external = self
|
||||
.requirements
|
||||
.iter()
|
||||
.any(|r| matches!(r, RequirementPayload::CliConfigInvalid { .. }));
|
||||
|
||||
let footer = if all_external {
|
||||
// All requirements are external config files — Edit Agent cannot
|
||||
// help. Don't send the user there.
|
||||
"Fix the config file(s) and restart the agent.".to_string()
|
||||
} else if any_external {
|
||||
// Mixed: some Buzz-managed fields, some external config.
|
||||
"Open Edit Agent in the Buzz app for the Buzz-managed fields; fix the external CLI config files manually and restart the agent.".to_string()
|
||||
} else {
|
||||
// All Buzz-managed — original footer unchanged.
|
||||
"Open Edit Agent in the Buzz app to set these.".to_string()
|
||||
};
|
||||
|
||||
format!(
|
||||
"**{}** needs configuration before it can respond:\n{}\n\nOpen Edit Agent in the Buzz app to set these.",
|
||||
"**{}** needs configuration before it can respond:\n{}\n\n{}",
|
||||
self.agent_name,
|
||||
steps.join("\n"),
|
||||
footer,
|
||||
)
|
||||
};
|
||||
|
||||
@@ -678,6 +720,85 @@ mod tests {
|
||||
assert!(body.contains("needs configuration"));
|
||||
}
|
||||
|
||||
// ── config-invalid footer tests ────────────────────────────────────────────
|
||||
|
||||
fn make_cli_config_invalid(cli: &str, diagnostic: &str) -> RequirementPayload {
|
||||
RequirementPayload::CliConfigInvalid {
|
||||
probe_args: vec![cli.to_string()],
|
||||
setup_copy: String::new(),
|
||||
diagnostic: diagnostic.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nudge_body_all_config_invalid_omits_edit_agent_footer() {
|
||||
// An all-CliConfigInvalid requirements list must NOT send users to
|
||||
// Edit Agent (which cannot fix an external config file).
|
||||
let payload = SetupPayload {
|
||||
agent_name: "Codex".to_string(),
|
||||
agent_pubkey: "test".to_string(),
|
||||
requirements: vec![make_cli_config_invalid(
|
||||
"codex",
|
||||
"unknown variant `ultra` for field `model_reasoning_effort`",
|
||||
)],
|
||||
};
|
||||
let body = payload.nudge_body();
|
||||
assert!(
|
||||
!body.contains("Open Edit Agent"),
|
||||
"all-config-invalid nudge must not mention Open Edit Agent; got: {body:?}"
|
||||
);
|
||||
assert!(
|
||||
body.contains("config.toml"),
|
||||
"nudge must name the config file; got: {body:?}"
|
||||
);
|
||||
assert!(
|
||||
body.contains("restart the agent"),
|
||||
"nudge must include restart guidance; got: {body:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nudge_body_mixed_requirements_uses_split_footer() {
|
||||
// Mixed list: one Buzz-managed env key + one external config invalid.
|
||||
// Footer must address both sides.
|
||||
let payload = SetupPayload {
|
||||
agent_name: "Codex".to_string(),
|
||||
agent_pubkey: "test".to_string(),
|
||||
requirements: vec![
|
||||
RequirementPayload::EnvKey {
|
||||
key: "SOME_API_KEY".to_string(),
|
||||
},
|
||||
make_cli_config_invalid("codex", "unknown variant `gpt-5.6-sol`"),
|
||||
],
|
||||
};
|
||||
let body = payload.nudge_body();
|
||||
assert!(
|
||||
body.contains("Open Edit Agent"),
|
||||
"mixed nudge must mention Open Edit Agent for managed fields; got: {body:?}"
|
||||
);
|
||||
assert!(
|
||||
body.contains("fix the external CLI config"),
|
||||
"mixed nudge must mention fixing the external config; got: {body:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn nudge_body_all_buzz_managed_retains_original_footer() {
|
||||
// Pure Buzz-managed requirements → original "Open Edit Agent" footer unchanged.
|
||||
let payload = SetupPayload {
|
||||
agent_name: "Fizz".to_string(),
|
||||
agent_pubkey: "test".to_string(),
|
||||
requirements: vec![RequirementPayload::EnvKey {
|
||||
key: "ANTHROPIC_API_KEY".to_string(),
|
||||
}],
|
||||
};
|
||||
let body = payload.nudge_body();
|
||||
assert!(
|
||||
body.contains("Open Edit Agent in the Buzz app to set these."),
|
||||
"all-managed nudge must use the original Edit Agent footer; got: {body:?}"
|
||||
);
|
||||
}
|
||||
|
||||
// ── sentinel block tests ───────────────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -143,7 +143,10 @@ const overrides = new Map([
|
||||
// +1 rebase merge: GlobalAgentConfig import added alongside AcpAvailabilityStatus.
|
||||
// +2 rebase onto #1667: behavioral quad fields in PersonaRecord/ManagedAgentRecord.
|
||||
// +3 rebase onto main (#1568 + #1613): identity-import-keyring + augmented-PATH probes.
|
||||
["src-tauri/src/managed_agents/readiness.rs", 1565],
|
||||
// +18: CliConfigInvalid requirement surface for config-parse probe classification —
|
||||
// new Requirement variant + updated cli_login_requirements + 3 new probe-layer tests.
|
||||
// Load-bearing UX fix (bad config → clear diagnostic, not "run codex login").
|
||||
["src-tauri/src/managed_agents/readiness.rs", 1583],
|
||||
// applyWorkspace reposDir parameter plus the validateReposDir binding,
|
||||
// threaded through Tauri invokes for configurable repos_dir, plus the
|
||||
// harness-persona-sync `harnessOverride` create-input bit — load-bearing
|
||||
|
||||
@@ -177,6 +177,19 @@ pub enum Requirement {
|
||||
/// and route to Doctor with accurate context.
|
||||
availability: AcpAvailabilityStatus,
|
||||
},
|
||||
/// The CLI is installed but its config file could not be parsed.
|
||||
/// This is an informational surface only — there is no in-app destination
|
||||
/// that can repair an external config file; the user must edit it manually.
|
||||
CliConfigInvalid {
|
||||
/// Arguments used in the probe (e.g. `["codex", "login", "status"]`);
|
||||
/// `probe_args[0]` is the CLI name (e.g. `"codex"`).
|
||||
probe_args: Vec<String>,
|
||||
/// Human-readable hint shown when no structured copy is available.
|
||||
setup_copy: String,
|
||||
/// A one-line excerpt from the CLI's stderr (the parse-error line).
|
||||
/// Shown verbatim in the nudge so the user can identify the problem.
|
||||
diagnostic: String,
|
||||
},
|
||||
}
|
||||
|
||||
// ── AgentReadiness ────────────────────────────────────────────────────────────
|
||||
@@ -505,20 +518,25 @@ fn cli_login_requirements(
|
||||
};
|
||||
|
||||
let augmented_path = cli_probe::augmented_path();
|
||||
let logged_in = cli_probe::login_probe_succeeds(
|
||||
&binary_path,
|
||||
probe_args,
|
||||
augmented_path.as_deref(),
|
||||
);
|
||||
let outcome =
|
||||
cli_probe::login_probe(&binary_path, probe_args, augmented_path.as_deref());
|
||||
|
||||
if logged_in {
|
||||
vec![]
|
||||
} else {
|
||||
vec![Requirement::CliLogin {
|
||||
probe_args: probe_args.iter().map(|s| s.to_string()).collect(),
|
||||
setup_copy: setup_copy.to_string(),
|
||||
availability: AcpAvailabilityStatus::Available,
|
||||
}]
|
||||
match outcome {
|
||||
cli_probe::ProbeOutcome::LoggedIn => vec![],
|
||||
cli_probe::ProbeOutcome::LoggedOut => {
|
||||
vec![Requirement::CliLogin {
|
||||
probe_args: probe_args.iter().map(|s| s.to_string()).collect(),
|
||||
setup_copy: setup_copy.to_string(),
|
||||
availability: AcpAvailabilityStatus::Available,
|
||||
}]
|
||||
}
|
||||
cli_probe::ProbeOutcome::ConfigInvalid { stderr_excerpt } => {
|
||||
vec![Requirement::CliConfigInvalid {
|
||||
probe_args: probe_args.iter().map(|s| s.to_string()).collect(),
|
||||
setup_copy: setup_copy.to_string(),
|
||||
diagnostic: stderr_excerpt,
|
||||
}]
|
||||
}
|
||||
}
|
||||
}
|
||||
// Tooling is not fully installed — emit CliLogin with the precise
|
||||
|
||||
@@ -12,28 +12,74 @@ pub(super) fn augmented_path() -> Option<String> {
|
||||
)
|
||||
}
|
||||
|
||||
pub(super) fn login_probe_succeeds(
|
||||
/// Outcome of a CLI login-status probe.
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub(super) enum ProbeOutcome {
|
||||
/// The CLI reported a successful login (exit 0).
|
||||
LoggedIn,
|
||||
/// The CLI exited non-zero without a config-parse signal — treat as
|
||||
/// "not authenticated."
|
||||
LoggedOut,
|
||||
/// The CLI exited non-zero and its stderr contains a config-parse error
|
||||
/// (e.g. from `~/.codex/config.toml`). The user needs to fix their
|
||||
/// config, not re-run login.
|
||||
ConfigInvalid {
|
||||
/// A trimmed excerpt of the stderr message to surface in the nudge.
|
||||
stderr_excerpt: String,
|
||||
},
|
||||
}
|
||||
|
||||
/// Signals emitted to stderr by codex (and related CLI tools) when they
|
||||
/// fail to parse their config file. We check these to distinguish a
|
||||
/// config-parse failure from a genuine "not authenticated" exit.
|
||||
///
|
||||
/// The real codex error reads:
|
||||
/// `Error loading configuration: .../.codex/config.toml:... unknown variant ...`
|
||||
/// So we require BOTH "error loading configuration" AND "unknown variant" to be
|
||||
/// present, avoiding false positives from unrelated errors that mention only
|
||||
/// one term.
|
||||
const CONFIG_PARSE_SIGNALS: &[&str] = &["error loading configuration", "unknown variant"];
|
||||
|
||||
/// Run the probe at the resolved absolute path so the GUI-PATH gap is
|
||||
/// bypassed. Injects the same augmented PATH used for launched agents so
|
||||
/// script shims with `/usr/bin/env <interpreter>` shebangs can find runtimes
|
||||
/// such as node/python when the app was launched with a bare GUI PATH.
|
||||
pub(super) fn login_probe(
|
||||
binary_path: &Path,
|
||||
probe_args: &[&str],
|
||||
augmented_path: Option<&str>,
|
||||
) -> bool {
|
||||
// Run the probe at the resolved absolute path so the GUI-PATH gap is
|
||||
// bypassed. Inject the same augmented PATH used for launched agents so
|
||||
// script shims with `/usr/bin/env <interpreter>` shebangs can find runtimes
|
||||
// such as node/python when the app was launched with a bare GUI PATH.
|
||||
) -> ProbeOutcome {
|
||||
let mut command = std::process::Command::new(binary_path);
|
||||
command.args(&probe_args[1..]);
|
||||
if let Some(path) = augmented_path {
|
||||
command.env("PATH", path);
|
||||
}
|
||||
command
|
||||
.output()
|
||||
.map(|o| o.status.success())
|
||||
.unwrap_or(false)
|
||||
|
||||
match command.output() {
|
||||
Ok(o) if o.status.success() => ProbeOutcome::LoggedIn,
|
||||
Ok(o) => {
|
||||
let stderr = String::from_utf8_lossy(&o.stderr);
|
||||
let stderr_lower = stderr.to_lowercase();
|
||||
if CONFIG_PARSE_SIGNALS
|
||||
.iter()
|
||||
.all(|sig| stderr_lower.contains(sig))
|
||||
{
|
||||
let excerpt = stderr.trim().lines().next().unwrap_or("").to_string();
|
||||
ProbeOutcome::ConfigInvalid {
|
||||
stderr_excerpt: excerpt,
|
||||
}
|
||||
} else {
|
||||
ProbeOutcome::LoggedOut
|
||||
}
|
||||
}
|
||||
Err(_) => ProbeOutcome::LoggedOut,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{ProbeOutcome, CONFIG_PARSE_SIGNALS};
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn login_probe_uses_augmented_path_for_env_shebang_interpreter() {
|
||||
@@ -83,12 +129,13 @@ mod tests {
|
||||
let augmented_path =
|
||||
std::env::join_paths([interpreter_dir.as_path()]).expect("join augmented PATH");
|
||||
let augmented_path = augmented_path.to_string_lossy().into_owned();
|
||||
assert!(
|
||||
super::login_probe_succeeds(
|
||||
assert_eq!(
|
||||
super::login_probe(
|
||||
&script_path,
|
||||
&["fake-codex", "login", "status"],
|
||||
Some(&augmented_path),
|
||||
),
|
||||
ProbeOutcome::LoggedIn,
|
||||
"the injected augmented PATH should allow /usr/bin/env to find the interpreter"
|
||||
);
|
||||
assert!(
|
||||
@@ -96,4 +143,86 @@ mod tests {
|
||||
"the fake node from the injected PATH should have run"
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn login_probe_config_invalid_on_stderr_signal() {
|
||||
use std::fs;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let temp = tempfile::tempdir().expect("temp dir");
|
||||
let bin_dir = temp.path().join("bin");
|
||||
fs::create_dir_all(&bin_dir).expect("bin dir");
|
||||
|
||||
// Script that exits 1 and writes a codex-style config-parse error to stderr.
|
||||
let script_path = bin_dir.join("fake-codex-bad-config");
|
||||
fs::write(
|
||||
&script_path,
|
||||
"#!/bin/sh\necho 'Error loading configuration: /home/user/.codex/config.toml: unknown variant `ultra`, expected one of none/minimal/low/medium/high/xhigh' >&2\nexit 1\n",
|
||||
)
|
||||
.expect("write script");
|
||||
fs::set_permissions(&script_path, fs::Permissions::from_mode(0o755)).expect("chmod script");
|
||||
|
||||
let outcome = super::login_probe(
|
||||
&script_path,
|
||||
&["fake-codex-bad-config", "login", "status"],
|
||||
None,
|
||||
);
|
||||
assert!(
|
||||
matches!(outcome, ProbeOutcome::ConfigInvalid { .. }),
|
||||
"stderr with 'unknown variant' should produce ConfigInvalid; got {:?}",
|
||||
outcome
|
||||
);
|
||||
if let ProbeOutcome::ConfigInvalid { stderr_excerpt } = outcome {
|
||||
assert!(
|
||||
stderr_excerpt.contains("unknown variant")
|
||||
|| stderr_excerpt.contains("Error loading"),
|
||||
"stderr_excerpt should contain the parse error: {stderr_excerpt}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn login_probe_logged_out_on_nonzero_without_config_signal() {
|
||||
use std::fs;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let temp = tempfile::tempdir().expect("temp dir");
|
||||
let bin_dir = temp.path().join("bin");
|
||||
fs::create_dir_all(&bin_dir).expect("bin dir");
|
||||
|
||||
// Script that exits 1 with a generic "not logged in" message.
|
||||
let script_path = bin_dir.join("fake-codex-logged-out");
|
||||
fs::write(
|
||||
&script_path,
|
||||
"#!/bin/sh\necho 'not authenticated' >&2\nexit 1\n",
|
||||
)
|
||||
.expect("write script");
|
||||
fs::set_permissions(&script_path, fs::Permissions::from_mode(0o755)).expect("chmod script");
|
||||
|
||||
let outcome = super::login_probe(
|
||||
&script_path,
|
||||
&["fake-codex-logged-out", "login", "status"],
|
||||
None,
|
||||
);
|
||||
assert_eq!(
|
||||
outcome,
|
||||
ProbeOutcome::LoggedOut,
|
||||
"non-config stderr should produce LoggedOut"
|
||||
);
|
||||
}
|
||||
|
||||
/// Verify that every string in CONFIG_PARSE_SIGNALS is lowercased so the
|
||||
/// case-insensitive match works correctly.
|
||||
#[test]
|
||||
fn config_parse_signals_are_lowercase() {
|
||||
for sig in CONFIG_PARSE_SIGNALS {
|
||||
assert_eq!(
|
||||
*sig,
|
||||
sig.to_lowercase(),
|
||||
"CONFIG_PARSE_SIGNAL must be lowercase for case-insensitive matching: {sig}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1615,6 +1615,16 @@ pub fn spawn_agent_child(
|
||||
"setup_copy": setup_copy,
|
||||
"availability": availability,
|
||||
}),
|
||||
Requirement::CliConfigInvalid {
|
||||
probe_args,
|
||||
setup_copy,
|
||||
diagnostic,
|
||||
} => serde_json::json!({
|
||||
"surface": "cli_config_invalid",
|
||||
"probe_args": probe_args,
|
||||
"setup_copy": setup_copy,
|
||||
"diagnostic": diagnostic,
|
||||
}),
|
||||
})
|
||||
.collect();
|
||||
let payload = serde_json::json!({
|
||||
|
||||
@@ -4,6 +4,7 @@ import test from "node:test";
|
||||
import {
|
||||
friendlyAgentLastError,
|
||||
friendlyTurnErrorCopy,
|
||||
CLI_ACP_INTERNAL_ERROR_COPY,
|
||||
MODEL_NOT_FOUND_COPY,
|
||||
RELAY_MESH_DENIED_COPY,
|
||||
} from "./friendlyAgentLastError.ts";
|
||||
@@ -213,3 +214,105 @@ test("friendlyTurnErrorCopy: garbage string code coerces to NaN → string path"
|
||||
RELAY_MESH_DENIED_COPY,
|
||||
);
|
||||
});
|
||||
|
||||
// --- -32603 internal error (Fix #2: CLI-ACP unsupported model hint) ---
|
||||
|
||||
test("code -32603 bare 'Internal error' → cli-acp internal error hint (severity: generic)", () => {
|
||||
const result = friendlyAgentLastError("Internal error", -32603);
|
||||
assert.deepEqual(result, {
|
||||
severity: "generic",
|
||||
copy: CLI_ACP_INTERNAL_ERROR_COPY,
|
||||
});
|
||||
});
|
||||
|
||||
test("code -32603 bare Internal error (wrapped) → cli-acp internal error hint", () => {
|
||||
// The ACP wrapper form "Agent reported error (code -32603): Internal error"
|
||||
// is treated as bare — the remainder after stripping the prefix is
|
||||
// "Internal error", which maps to the hint.
|
||||
const result = friendlyAgentLastError(
|
||||
"Agent reported error (code -32603): Internal error",
|
||||
-32603,
|
||||
);
|
||||
assert.deepEqual(result, {
|
||||
severity: "generic",
|
||||
copy: CLI_ACP_INTERNAL_ERROR_COPY,
|
||||
});
|
||||
});
|
||||
|
||||
test("code -32603 with specific message → original message preserved, NOT hint", () => {
|
||||
// If the adapter provides detail beyond "Internal error", preserve it —
|
||||
// don't bury actionable information with a broad codex-specific hint.
|
||||
const result = friendlyAgentLastError(
|
||||
"Internal error: model gpt-5.6-sol rejected by adapter",
|
||||
-32603,
|
||||
);
|
||||
assert.deepEqual(result, {
|
||||
severity: "generic",
|
||||
copy: "Internal error: model gpt-5.6-sol rejected by adapter",
|
||||
});
|
||||
});
|
||||
|
||||
test("embedded code -32603 recovered from message when code param is null", () => {
|
||||
const result = friendlyAgentLastError(
|
||||
"Agent reported error (code -32603): Internal error",
|
||||
null,
|
||||
);
|
||||
assert.deepEqual(result, {
|
||||
severity: "generic",
|
||||
copy: CLI_ACP_INTERNAL_ERROR_COPY,
|
||||
});
|
||||
});
|
||||
|
||||
test("embedded code -32603 with specific detail → original message preserved", () => {
|
||||
// Embedded code path also preserves specific detail.
|
||||
const result = friendlyAgentLastError(
|
||||
"Agent reported error (code -32603): model not in registry",
|
||||
null,
|
||||
);
|
||||
assert.deepEqual(result, {
|
||||
severity: "generic",
|
||||
copy: "model not in registry",
|
||||
});
|
||||
});
|
||||
|
||||
test("code -32603 structured param + wrapped specific detail → clean message, NOT wrapper", () => {
|
||||
// Real path: storage.rs stores message = full wrapped line, code = parsed finite.
|
||||
// The transport wrapper must be stripped; only the adapter detail reaches the UI.
|
||||
const result = friendlyAgentLastError(
|
||||
"Agent reported error (code -32603): model not in registry",
|
||||
-32603,
|
||||
);
|
||||
assert.deepEqual(result, {
|
||||
severity: "generic",
|
||||
copy: "model not in registry",
|
||||
});
|
||||
});
|
||||
|
||||
test("friendlyTurnErrorCopy: -32603 structured param + wrapped specific detail → clean message", () => {
|
||||
// friendlyTurnErrorCopy is the transcript path (agentSessionTranscript.ts).
|
||||
assert.equal(
|
||||
friendlyTurnErrorCopy(
|
||||
"Agent reported error (code -32603): model not in registry",
|
||||
-32603,
|
||||
),
|
||||
"model not in registry",
|
||||
);
|
||||
});
|
||||
|
||||
test("friendlyTurnErrorCopy: code -32603 bare Internal error → cli-acp internal error hint", () => {
|
||||
assert.equal(
|
||||
friendlyTurnErrorCopy("Internal error", -32603),
|
||||
CLI_ACP_INTERNAL_ERROR_COPY,
|
||||
);
|
||||
});
|
||||
|
||||
test("-32603 does not affect -32001/-32002 classification (regression)", () => {
|
||||
assert.deepEqual(friendlyAgentLastError("any", -32001), {
|
||||
severity: "denied",
|
||||
copy: RELAY_MESH_DENIED_COPY,
|
||||
});
|
||||
assert.deepEqual(friendlyAgentLastError("any", -32002), {
|
||||
severity: "denied",
|
||||
copy: MODEL_NOT_FOUND_COPY,
|
||||
});
|
||||
});
|
||||
|
||||
@@ -42,11 +42,23 @@ export const RELAY_MESH_DENIED_COPY =
|
||||
export const MODEL_NOT_FOUND_COPY =
|
||||
"The configured model is not available — open agent settings and select a different one from the dropdown.";
|
||||
|
||||
const EMBEDDED_CODE_RE = /^Agent reported error \(code (-?\d+)\): /;
|
||||
export const CLI_ACP_INTERNAL_ERROR_COPY =
|
||||
"The agent's harness reported an internal error. For Codex agents this can mean the configured model isn't supported by your installed codex-acp — check the model in `~/.codex/config.toml` or upgrade the adapter (`brew upgrade codex-acp`).";
|
||||
|
||||
function recoverEmbeddedCode(trimmed: string): number | null {
|
||||
const EMBEDDED_CODE_RE = /^Agent reported error \(code (-?\d+)\): /;
|
||||
/** Bare form of the standard JSON-RPC -32603 message (after stripping the ACP wrapper prefix). */
|
||||
const BARE_INTERNAL_ERROR = "Internal error";
|
||||
|
||||
function recoverEmbeddedCode(trimmed: string): {
|
||||
code: number;
|
||||
remainder: string;
|
||||
} | null {
|
||||
const match = EMBEDDED_CODE_RE.exec(trimmed);
|
||||
return match ? Number(match[1]) : null;
|
||||
if (!match) return null;
|
||||
return {
|
||||
code: Number(match[1]),
|
||||
remainder: trimmed.slice(match[0].length),
|
||||
};
|
||||
}
|
||||
|
||||
export function friendlyAgentLastError(
|
||||
@@ -59,15 +71,33 @@ export function friendlyAgentLastError(
|
||||
|
||||
// Structured code first; a code embedded in the message string is the
|
||||
// same signal recovered from a record that lost the field.
|
||||
const embedded = recoverEmbeddedCode(trimmed);
|
||||
const effectiveCode = Number.isFinite(code)
|
||||
? (code as number)
|
||||
: recoverEmbeddedCode(trimmed);
|
||||
: (embedded?.code ?? null);
|
||||
if (effectiveCode != null) {
|
||||
switch (effectiveCode) {
|
||||
case -32001:
|
||||
return { severity: "denied", copy: RELAY_MESH_DENIED_COPY };
|
||||
case -32002:
|
||||
return { severity: "denied", copy: MODEL_NOT_FOUND_COPY };
|
||||
case -32603: {
|
||||
// Standard JSON-RPC "Internal error" — emitted by external harnesses
|
||||
// (e.g. codex-acp) when the configured model is unsupported. Only
|
||||
// substitute the hint when the message is the bare "Internal error"
|
||||
// form; if the adapter included specific detail, preserve it so we
|
||||
// don't bury actionable information with a broad codex-specific hint.
|
||||
//
|
||||
// "Bare" means the remainder after stripping the ACP wrapper prefix
|
||||
// (if present) is exactly "Internal error". This covers both the raw
|
||||
// form ("Internal error") and the ACP-wrapped form
|
||||
// ("Agent reported error (code -32603): Internal error").
|
||||
const remainder = embedded?.remainder ?? trimmed;
|
||||
if (remainder === BARE_INTERNAL_ERROR) {
|
||||
return { severity: "generic", copy: CLI_ACP_INTERNAL_ERROR_COPY };
|
||||
}
|
||||
return { severity: "generic", copy: remainder };
|
||||
}
|
||||
}
|
||||
// A structured code we don't recognize is authoritative — don't let
|
||||
// string patterns cross-classify it.
|
||||
|
||||
@@ -36,6 +36,18 @@ export type ConfigNudgeRequirement =
|
||||
* - "not_installed" → neither adapter nor CLI found
|
||||
*/
|
||||
availability: AcpAvailabilityStatus;
|
||||
}
|
||||
| {
|
||||
/**
|
||||
* The CLI is installed but its config file could not be parsed.
|
||||
* Informational only — no in-app action can repair an external config
|
||||
* file. Rendered without a Doctor or Edit Agent CTA.
|
||||
*/
|
||||
surface: "cli_config_invalid";
|
||||
probe_args: string[];
|
||||
setup_copy: string;
|
||||
/** One-line stderr excerpt from the CLI's parse error. */
|
||||
diagnostic: string;
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -131,6 +143,13 @@ function isConfigNudgeRequirement(v: unknown): v is ConfigNudgeRequirement {
|
||||
r.availability === "cli_missing" ||
|
||||
r.availability === "not_installed")
|
||||
);
|
||||
case "cli_config_invalid":
|
||||
return (
|
||||
Array.isArray(r.probe_args) &&
|
||||
r.probe_args.every((a) => typeof a === "string") &&
|
||||
typeof r.setup_copy === "string" &&
|
||||
typeof r.diagnostic === "string"
|
||||
);
|
||||
default:
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -33,6 +33,8 @@ function requirementKey(
|
||||
return `normalized_field:${req.field}:${index}`;
|
||||
case "cli_login":
|
||||
return `cli_login:${req.probe_args.join(",")}:${index}`;
|
||||
case "cli_config_invalid":
|
||||
return `cli_config_invalid:${req.probe_args.join(",")}:${index}`;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -62,6 +64,17 @@ function isAuthOnly(reqs: ConfigNudgePayload["requirements"]): boolean {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true when every requirement is a `cli_config_invalid` surface.
|
||||
* Config-invalid cards are purely informational — the user must edit an
|
||||
* external file; there is no in-app destination that can fix it.
|
||||
*/
|
||||
function isAllConfigInvalid(reqs: ConfigNudgePayload["requirements"]): boolean {
|
||||
return (
|
||||
reqs.length > 0 && reqs.every((r) => r.surface === "cli_config_invalid")
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Per-state human-readable copy for a cli_login requirement.
|
||||
* Uses the probe_args[0] as a best-effort harness name.
|
||||
@@ -164,6 +177,10 @@ export function ConfigNudgeCard({
|
||||
|
||||
const allCliLogin = isAllCliLogin(nudge.requirements);
|
||||
const authOnly = isAuthOnly(nudge.requirements);
|
||||
const allConfigInvalid = isAllConfigInvalid(nudge.requirements);
|
||||
// Any card that is purely informational (auth-only or all-config-invalid)
|
||||
// has no clickable destination — treat them the same for affordance/routing.
|
||||
const informationalOnly = authOnly || allConfigInvalid;
|
||||
|
||||
const openDoctor = () => {
|
||||
if (!onOpenSettings) {
|
||||
@@ -212,9 +229,9 @@ export function ConfigNudgeCard({
|
||||
<Attachment
|
||||
className={cn(
|
||||
"max-w-[min(100%,32rem)] shrink-0 shadow-none",
|
||||
// Affordance: cursor-pointer + subtle hover lift — omitted for auth-only
|
||||
// cards which are purely informational (no click destination).
|
||||
!authOnly && "cursor-pointer hover:shadow-sm",
|
||||
// Affordance: cursor-pointer + subtle hover lift — omitted for
|
||||
// informational-only cards which have no click destination.
|
||||
!informationalOnly && "cursor-pointer hover:shadow-sm",
|
||||
className,
|
||||
)}
|
||||
orientation="horizontal"
|
||||
@@ -240,15 +257,15 @@ export function ConfigNudgeCard({
|
||||
</div>
|
||||
</AttachmentContent>
|
||||
{/* (A) Install-state all-cli_login card only — single card-level CTA
|
||||
confirming the action at rest. Auth-only cards are informational
|
||||
(no CTA); mixed cards render per-row CTAs instead. */}
|
||||
{allCliLogin && !authOnly && (
|
||||
confirming the action at rest. Informational-only cards have no CTA;
|
||||
mixed cards render per-row CTAs instead. */}
|
||||
{allCliLogin && !informationalOnly && (
|
||||
<AttachmentActions className="items-end self-end">
|
||||
<span className="text-xs text-muted-foreground">Open Doctor →</span>
|
||||
</AttachmentActions>
|
||||
)}
|
||||
{/* Auth-only cards are purely informational — no trigger, no routing. */}
|
||||
{!authOnly && (
|
||||
{/* Informational-only cards are purely informational — no trigger, no routing. */}
|
||||
{!informationalOnly && (
|
||||
<AttachmentTrigger
|
||||
aria-label={
|
||||
allCliLogin
|
||||
@@ -345,5 +362,23 @@ function RequirementRow({
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
case "cli_config_invalid": {
|
||||
// Config-invalid rows are purely informational — the user must edit an
|
||||
// external file. No Doctor CTA (Doctor can't repair ~/.codex/config.toml)
|
||||
// and no Edit Agent CTA (the field isn't managed by Buzz).
|
||||
const cli = requirement.probe_args[0] ?? "the CLI";
|
||||
const configFile = `~/.${cli}/config.toml`;
|
||||
return (
|
||||
<div className="flex items-center gap-2 text-xs leading-4 text-muted-foreground">
|
||||
<span className="flex-1 [overflow-wrap:anywhere]">
|
||||
{configFile} is invalid:{" "}
|
||||
<code className="rounded bg-muted px-1 py-0.5 font-mono text-xs text-foreground">
|
||||
{requirement.diagnostic}
|
||||
</code>{" "}
|
||||
— fix the config and restart the agent
|
||||
</span>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user