Skip to content
Open
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
14 changes: 6 additions & 8 deletions Cargo.lock

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

2 changes: 0 additions & 2 deletions crates/jcode-app-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ serde_json = { version = "1", features = ["raw_value"] }
# CLI

# File operations
glob = "0.3"
ignore = "0.4"
similar = "2" # diffing for edits

# Utilities
Expand Down
21 changes: 19 additions & 2 deletions crates/jcode-app-core/src/server/comm_session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,27 @@ async fn resolve_coordinator_spawn_identity(
}
Err(error) => {
crate::logging::warn(&format!(
"Swarm spawn: failed to load persisted coordinator session {} for model inheritance: {} (spawned agent will use server defaults)",
"Swarm spawn: failed to load persisted coordinator session {} for model inheritance: {} (falling back to config default model)",
req_session_id, error
));
CoordinatorSpawnIdentity::default()
// Fall back to the config's default model instead of returning
// all-None, which would cause spawned agents to silently use a
// hardcoded/random provider default instead of the user's
// configured default model (issue #981).
let config = crate::config::config();
let default_model = config.provider.default_model.clone();
let default_provider = config.provider.default_provider.clone();
let provider_key = default_provider.as_deref().map(str::to_string).or_else(|| {
default_model
.as_deref()
.and_then(|m| provider_key_for_spawn_model(Some(m), None))
});
CoordinatorSpawnIdentity {
model: default_model,
provider_key,
route_api_method: None,
is_canary: false,
}
}
}
}
Expand Down
35 changes: 35 additions & 0 deletions crates/jcode-app-core/src/server/comm_session_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,41 @@ async fn session_agent_models_update_and_spawn_while_coordinator_busy() {
}
}

#[tokio::test]
async fn coordinator_identity_falls_back_to_config_default_when_session_load_fails() {
let _guard = crate::storage::lock_test_env();
let previous_home = std::env::var_os("JCODE_HOME");
let temp_home = tempfile::TempDir::new().expect("temp home");
crate::env::set_var("JCODE_HOME", temp_home.path());
// Pin a known config default so the assertion checks the real fallback
// identity instead of passing trivially when both fields are None.
std::fs::write(
temp_home.path().join("config.toml"),
"[provider]\ndefault_model = \"gpt-5.4-luna\"\n",
)
.expect("write config");
crate::config::invalidate_config_cache();

// No agent registered and no persisted session on disk: the load_startup_stub
// path fails. The fallback must use the config default model instead of
// returning all-None (which would silently use a hardcoded provider default).
let sessions = Arc::new(RwLock::new(HashMap::new()));
let identity = resolve_coordinator_spawn_identity("nonexistent", &sessions).await;

assert_eq!(
identity.model.as_deref(),
Some("gpt-5.4-luna"),
"error fallback must resolve the config default model, not silently return all-None"
);

if let Some(previous) = previous_home {
crate::env::set_var("JCODE_HOME", previous);
} else {
crate::env::remove_var("JCODE_HOME");
}
crate::config::invalidate_config_cache();
}

#[tokio::test]
async fn spawn_bootstraps_coordinator_when_swarm_has_none() {
let swarm_members = Arc::new(RwLock::new(HashMap::new()));
Expand Down
41 changes: 41 additions & 0 deletions crates/jcode-app-core/src/server/provider_control_busy_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,45 @@ impl Drop for IsolatedRuntimeDir {
}
}

// `derive_session_provider_key` folds ambient runtime env (JCODE_RUNTIME_PROVIDER,
// JCODE_ACTIVE_PROVIDER, JCODE_OPENROUTER_CACHE_NAMESPACE) into a new session's
// provider_key, and session-model restore then turns `test-model-a` into
// `openai:test-model-a`. Tests asserting raw model strings must run without
// whatever provider env the surrounding process happened to export.
struct IsolatedProviderEnv {
prev: Vec<(&'static str, Option<std::ffi::OsString>)>,
}

impl IsolatedProviderEnv {
fn new() -> Self {
let prev = [
"JCODE_RUNTIME_PROVIDER",
"JCODE_ACTIVE_PROVIDER",
"JCODE_OPENROUTER_CACHE_NAMESPACE",
]
.iter()
.map(|&key| {
let prev = std::env::var_os(key);
if prev.is_some() {
crate::env::remove_var(key);
}
(key, prev)
})
.collect();
Self { prev }
}
}

impl Drop for IsolatedProviderEnv {
fn drop(&mut self) {
for (key, prev) in self.prev.drain(..) {
if let Some(prev) = prev {
crate::env::set_var(key, prev);
}
}
}
}

#[derive(Default)]
struct TestEffortProvider {
model: StdMutex<Option<String>>,
Expand Down Expand Up @@ -180,6 +219,7 @@ async fn set_reasoning_effort_does_not_wait_for_busy_agent_lock() {
async fn automatic_failover_revalidates_after_waiting_for_agent_lock() {
let _guard = crate::storage::lock_test_env();
let _runtime = IsolatedRuntimeDir::new();
let _provider_env = IsolatedProviderEnv::new();
let (provider, agent, tx, mut rx) = test_agent("session_rejected_auto_route").await;
let selection = crate::provider::RouteSelection {
model: "gpt-6.1-sol".to_string(),
Expand Down Expand Up @@ -269,6 +309,7 @@ async fn automatic_failover_source_respects_current_api_pool_membership() {
async fn set_model_does_not_wait_for_busy_agent_lock() {
let _guard = crate::storage::lock_test_env();
let _runtime = IsolatedRuntimeDir::new();
let _provider_env = IsolatedProviderEnv::new();

let (provider, agent, client_event_tx, mut client_event_rx) =
test_agent("session_busy_set_model").await;
Expand Down
2 changes: 1 addition & 1 deletion crates/jcode-app-core/src/server/turn_failover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ pub(super) async fn process_message_with_server_failover(
enabled: bool,
) -> anyhow::Result<()> {
let reminder = system_reminder.clone();
let mut result = super::client_lifecycle::process_message_streaming_mpsc(
let result = super::client_lifecycle::process_message_streaming_mpsc(
Arc::clone(&agent),
content,
images,
Expand Down
4 changes: 4 additions & 0 deletions crates/jcode-app-core/src/tool/browser_jev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ impl JevTransport {
}
}

/// Builds the browser-decision request body with the default browser model.
/// Only in-crate tests call this directly; production paths pass their own
/// model label through [`request_body_for_model`].
#[cfg(test)]
fn request_body(request: &DecisionRequest) -> Result<Value> {
request_body_for_model(request, &crate::jev::browser_jev_model_label())
}
Expand Down
39 changes: 33 additions & 6 deletions crates/jcode-app-core/src/tool/browser_session_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,18 @@ impl Drop for BrowserFixture {
async fn browser_session_reuses_daemon_across_five_tool_calls() {
let fixture = BrowserFixture::new();
let ctx = fixture.context("reuse");
// The first call pays daemon startup (a scripted 0.2s sleep plus Python
// interpreter spawn, which parallel test load can stretch well past a
// second). The reuse property is about the warm calls after it: they must
// stay cheap socket round trips, not new daemon starts. starts().len()
// below catches a restart deterministically; the clock bounds a hang.
let mut responses = vec![
firefox_run_bridge_command("getActiveTab", json!({}), Some("chrome"), &ctx)
.await
.unwrap(),
];
let started = Instant::now();
let mut responses = Vec::new();
for _ in 0..5 {
for _ in 0..4 {
responses.push(
firefox_run_bridge_command("getActiveTab", json!({}), Some("chrome"), &ctx)
.await
Expand All @@ -131,7 +140,16 @@ async fn browser_session_reuses_daemon_across_five_tool_calls() {
"all calls must reach the same daemon"
);
assert_eq!(fixture.starts().len(), 1);
assert!(started.elapsed() < Duration::from_secs(5));
// The reuse guarantee above is structural (one daemon start). The clock
// only bounds a hang: four warm socket round trips can each pay ~1s of
// process-spawn jitter in a full-suite run, so a tight bound turns load
// into false failures. 20s still fails long before the test runner's own
// timeout if a call genuinely deadlocks.
assert!(
started.elapsed() < Duration::from_secs(20),
"reused-daemon calls took {:?}",
started.elapsed()
);
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
Expand Down Expand Up @@ -176,7 +194,6 @@ async fn browser_session_stalled_startup_keeps_healthy_calls_and_workers_respons
let _ = release_rx.recv_timeout(Duration::from_secs(3));
std::fs::write(release_path, "ready").unwrap();
});
let started = Instant::now();
let barrier = std::sync::Arc::new(tokio::sync::Barrier::new(5));
let mut browser_calls = Vec::new();
let mut bash_calls = Vec::new();
Expand All @@ -197,14 +214,20 @@ async fn browser_session_stalled_startup_keeps_healthy_calls_and_workers_respons
}));
}
barrier.wait().await;
let poll_started = Instant::now();
while !fixture
.starts()
.iter()
.any(|(name, _)| name.contains("blocked"))
&& started.elapsed() < Duration::from_secs(2)
&& poll_started.elapsed() < Duration::from_secs(2)
{
tokio::time::sleep(Duration::from_millis(10)).await;
}
// Measure only the window the assertion is about: while two sessions sit
// blocked at daemon startup, a healthy call and an unrelated task must
// still make progress. Daemon-start observation time above is scheduling
// noise under parallel load, not a stall, so it stays outside the window.
let started = Instant::now();
let healthy = tokio::spawn(async move {
firefox_run_bridge_command("getActiveTab", json!({}), Some("chrome"), &healthy_ctx).await
});
Expand All @@ -223,8 +246,12 @@ async fn browser_session_stalled_startup_keeps_healthy_calls_and_workers_respons
call.await.unwrap().unwrap();
}
healthy_result.unwrap();
// The bound must stay below the 3s watchdog release: a healthy call
// actually blocked behind the stalled daemon startups waits for that
// release, while a warm call under parallel load only pays process-spawn
// jitter (observed at ~1.1s in a full-suite run).
assert!(
elapsed < Duration::from_secs(1),
elapsed < Duration::from_millis(2500),
"healthy calls and unrelated tasks stalled for {elapsed:?}"
);
assert_eq!(fixture.starts().len(), 3, "one daemon per session/browser");
Expand Down
2 changes: 1 addition & 1 deletion crates/jcode-app-core/src/tool/communicate_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use anyhow::Result;
use async_trait::async_trait;
use futures::StreamExt;
use serde_json::json;
use std::collections::{HashMap, HashSet};
use std::collections::HashMap;
use std::path::Path;
use std::sync::Arc;
use std::time::Duration;
Expand Down
7 changes: 6 additions & 1 deletion crates/jcode-app-core/src/tool/compile_remote/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,12 @@ mod tests {
use std::os::unix::ffi::OsStrExt;
let repo = repo().await;
let name = std::ffi::OsStr::from_bytes(b"non-utf8-\xff");
std::fs::write(repo.path().join(name), b"contents").unwrap();
if std::fs::write(repo.path().join(name), b"contents").is_err() {
// APFS refuses to create files whose names are not valid UTF-8, so
// git can never list one here. The UTF-8 rejection itself stays
// covered on filesystems that allow arbitrary bytes (ext4 on CI).
return;
}
assert!(
snapshot(repo.path())
.await
Expand Down
8 changes: 8 additions & 0 deletions crates/jcode-app-core/src/tool/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,9 @@ async fn tool_descriptions_stay_under_token_cap() {
// applet carries the whole view-node vocabulary inline, since the model has
// no other way to learn which node types and props the host renders.
// codemode documents its sandbox API; compile_remote a credit/consent notice.
// macos_computer_use is macOS-gated and keeps its live-machine safety policy.
// bash keeps the repository-search guidance (fixes #1804); the three pinned
// phrases alone exceed the cap, so shortening cannot satisfy it.
const EXEMPT: &[&str] = &[
"integration_tools",
"swarm",
Expand All @@ -727,6 +730,8 @@ async fn tool_descriptions_stay_under_token_cap() {
"applet",
"codemode",
"compile_remote",
"macos_computer_use",
"bash",
];

let provider: Arc<dyn Provider> = Arc::new(MockProvider);
Expand Down Expand Up @@ -790,13 +795,16 @@ async fn tool_parameter_descriptions_stay_under_token_cap() {
// applet placement lists every shorthand so the model can pick a surface
// without a round trip. desktop_selfdev action carries a safety warning:
// killing the harness bridge by hand strands the calling session.
// macos_computer_use action/element document its live-machine safety surface.
const EXEMPT: &[(&str, &str)] = &[
(
"todo",
"$.properties.goals.items.properties.feedback_loop_relevance",
),
("applet", "$.properties.placement"),
("desktop_selfdev", "$.properties.action"),
("macos_computer_use", "$.properties.action"),
("macos_computer_use", "$.properties.element"),
];

let provider: Arc<dyn Provider> = Arc::new(MockProvider);
Expand Down
5 changes: 3 additions & 2 deletions crates/jcode-base/src/auth/muse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ pub enum PollOutcome {
SlowDown { retry_after: u64 },
/// Approved. The minted credentials are not persisted by polling, and the caller
/// must call [`save_credentials`] explicitly after its own cancellation checks.
Approved(MuseCredentials),
/// Boxed: this variant is ~240 bytes and dwarfs the rest of the enum.
Approved(Box<MuseCredentials>),
/// The user denied the request.
Denied,
/// The device code expired before approval.
Expand Down Expand Up @@ -553,7 +554,7 @@ pub async fn poll_login_by_code(
if status.is_success() {
if let Some(token) = parsed.access_token.filter(|token| !token.trim().is_empty()) {
let credentials = mint_credentials(client, &token).await?;
return Ok(PollOutcome::Approved(credentials));
return Ok(PollOutcome::Approved(Box::new(credentials)));
}
if parsed.error.as_deref().is_none() {
anyhow::bail!("Muse sign-in response carried no usable token");
Expand Down
1 change: 1 addition & 0 deletions crates/jcode-base/src/auth/muse_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ async fn poll_approval_mints_without_persisting() {
assert!(!credentials_path().unwrap().exists());
match poll_login(&client, &challenge).await.expect("poll") {
PollOutcome::Approved(credentials) => {
let credentials = *credentials;
assert_eq!(credentials.api_key, "LLM_TEST_KEY_1");
assert_eq!(credentials.base_url, EXPECTED_API_BASE_URL);
save_credentials(&credentials).expect("explicit save");
Expand Down
Loading
Loading