Skip to content

feat(tui): Ctrl+S in /model picker selects the swarm model; coordinator identity falls back to config default (closes #981) - #1706

Open
alecuba16 wants to merge 1 commit into
1jehuang:masterfrom
alecuba16:fix/swarm-model-picker
Open

alecuba16 wants to merge 1 commit into
1jehuang:masterfrom
alecuba16:fix/swarm-model-picker

Conversation

@alecuba16

Copy link
Copy Markdown
Contributor

Closes #981.

Ctrl+S inside the /model picker opens the swarm model sub-picker, so the swarm model is discoverable from the main model flow instead of only via the agents.swarm_model config. With no override saved, spawned agents inherit the current session model.

Also fixes the coordinator identity root cause from #981: when the persisted session cannot be loaded and resolve_coordinator_spawn_identity has no model, spawn identity now falls back to the config default model instead of silently resolving to a provider default.

Tests: comm_session_tests covers the identity fallback (37/37 pass), TUI tests cover Ctrl+S opening the swarm sub-picker from the model picker and not firing outside it, and provider-init tests are isolated from user env vars so they pass on machines with provider profiles configured. Single commit rebased onto current master.

- Ctrl+S in the /model picker opens the swarm model sub-picker so users
  choose which model spawned swarm agents use without leaving the flow.
- Coordinator spawn identity falls back to the config default model when
  the persisted session cannot be loaded instead of silently using
  provider defaults (issue 1jehuang#981).
- tests: Ctrl+S hotkey coverage; isolate provider init tests from user
  env vars; fix stale assertions (Jcode Subscription, Validate further).
@greptile-apps

greptile-apps Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 3/5

[Medium risk] Adds swarm model picker and fixes coordinator fallback logic.

Not safe to merge until the two swarm-picker failures are fixed. The test concerns are non-blocking.

Findings

  1. P1 Delayed Load Changes Picker ▶
  2. P1 Remote Override Saves Locally ▶
  3. P2 Fallback Test Misses Regression ▶
  4. P2 Tests Discard Environment Values ▶
Fix with agent prompt
### Issue 1
crates/jcode-tui/src/tui/app/inline_interactive.rs:3538
If the model catalog takes more than two seconds to load, Ctrl+S opens the swarm picker, but the completed load replaces it with the ordinary model picker. Pressing Enter can then change the session model instead of saving a swarm-model override. Preserve the swarm selection target when the catalog refreshes before merging.

### Issue 2
crates/jcode-tui/src/tui/app/inline_interactive.rs:3538
If a non-SSH remote client and server use separate Jcode homes, selecting a swarm model saves the override in the client's configuration while worker spawning reads the server's configuration. The UI confirms a save, but workers can keep using the server's setting or inherit the coordinator model. Ensure the selection reaches the server before merging.

### Issue 3
crates/jcode-app-core/src/server/comm_session_tests.rs:831-834
The assertion passes when both the model and provider key are `None`. The test therefore misses the all-None fallback regression it is intended to catch, allowing that regression to pass unnoticed. Set a known default in the test and assert the resulting identity.

### Issue 4
src/cli/provider_init_tests.rs:305-315
The Jcode test clears provider-profile and OpenRouter variables without restoring their previous values; the Ollama test also clears three profile variables absent from its restore list. If those values were set before either test, later tests in the same process can see them missing, making results depend on test order.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR adds Ctrl+S in the runtime model picker so a user can choose a swarm-worker model without changing the session model. It also changes the coordinator’s fallback identity and adjusts provider setup tests. Two picker failures must be fixed before merging: a delayed catalog refresh can turn the swarm picker into a session-model picker, and a remote client can confirm an override that the server cannot read. The fallback test misses an all-None regression, and two provider tests can leave pre-existing environment values cleared.

Reviews (1) · Last reviewed commit: "feat(tui): add Ctrl+S shortcut in /model..."

.map(picker_is_runtime_model_picker)
.unwrap_or(false)
{
self.open_agent_model_picker(crate::tui::AgentModelTarget::Swarm);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Delayed Load Changes Picker

If the model catalog takes more than two seconds to load, Ctrl+S opens the swarm picker, but the completed load replaces it with the ordinary model picker. Pressing Enter can then change the session model instead of saving a swarm-model override. Preserve the swarm selection target when the catalog refreshes before merging.

Knowledge Base Used: Terminal user interface

Artifacts

Focused slow-catalog picker test source

  • The authored Rust test drives Ctrl+S, delayed catalog completion, and Enter against the in-process picker, exposing the wrong selection target.

Parent-versus-PR test runner

  • The authored shell script runs the same test against parent and PR picker code in an untracked checkout copy, recording each command and exit code.

Parent-version picker test output

  • The executed parent-version test shows Ctrl+S did not open a swarm picker; the run exited 0.

PR-version delayed-refresh picker test output

  • The executed PR-version test shows Ctrl+S opened the swarm picker, the refresh replaced it, and Enter changed the session model; the run exited 0.

Separate-home config probe source

  • The authored source calls Jcode’s config APIs to save a client override and read server config, showing the operations used in the probe.

Separate-home config probe command

  • The authored shell command compiles and runs the Rust probe with isolated Jcode homes, showing how the comparison was executed.

Config probe output with shared and separate homes

  • The executed probe saved a swarm override and read server config in both home arrangements; only the shared-home server saw the override.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-tui/src/tui/app/inline_interactive.rs
Line: 3538

Comment:
**Delayed Load Changes Picker**

If the model catalog takes more than two seconds to load, Ctrl+S opens the swarm picker, but the completed load replaces it with the ordinary model picker. Pressing Enter can then change the session model instead of saving a swarm-model override. Preserve the swarm selection target when the catalog refreshes before merging.

**Knowledge Base Used:** [Terminal user interface](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/terminal-user-interface.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

.map(picker_is_runtime_model_picker)
.unwrap_or(false)
{
self.open_agent_model_picker(crate::tui::AgentModelTarget::Swarm);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Remote Override Saves Locally

If a non-SSH remote client and server use separate Jcode homes, selecting a swarm model saves the override in the client's configuration while worker spawning reads the server's configuration. The UI confirms a save, but workers can keep using the server's setting or inherit the coordinator model. Ensure the selection reaches the server before merging.

Knowledge Base Used:

Artifacts

Focused slow-catalog picker test source

  • The authored Rust test drives Ctrl+S, delayed catalog completion, and Enter against the in-process picker, exposing the wrong selection target.

Parent-versus-PR test runner

  • The authored shell script runs the same test against parent and PR picker code in an untracked checkout copy, recording each command and exit code.

Parent-version picker test output

  • The executed parent-version test shows Ctrl+S did not open a swarm picker; the run exited 0.

PR-version delayed-refresh picker test output

  • The executed PR-version test shows Ctrl+S opened the swarm picker, the refresh replaced it, and Enter changed the session model; the run exited 0.

Separate-home config probe source

  • The authored source calls Jcode’s config APIs to save a client override and read server config, showing the operations used in the probe.

Separate-home config probe command

  • The authored shell command compiles and runs the Rust probe with isolated Jcode homes, showing how the comparison was executed.

Config probe output with shared and separate homes

  • The executed probe saved a swarm override and read server config in both home arrangements; only the shared-home server saw the override.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-tui/src/tui/app/inline_interactive.rs
Line: 3538

Comment:
**Remote Override Saves Locally**

If a non-SSH remote client and server use separate Jcode homes, selecting a swarm model saves the override in the client's configuration while worker spawning reads the server's configuration. The UI confirms a save, but workers can keep using the server's setting or inherit the coordinator model. Ensure the selection reaches the server before merging.

**Knowledge Base Used:**
- [Terminal user interface](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/terminal-user-interface.md)
- [Multi-agent swarm coordination](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/multi-agent-swarm.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Comment on lines +831 to +834
assert!(
identity.model.is_some() || identity.provider_key.is_none(),
"error fallback should attempt config default model, not silently return all-None"
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Fallback Test Misses Regression

The assertion passes when both the model and provider key are None. The test therefore misses the all-None fallback regression it is intended to catch, allowing that regression to pass unnoticed. Set a known default in the test and assert the resulting identity.

Artifacts

Scoped fallback mutation and test source

  • This authored script runs the same targeted Cargo test before and after replacing the error branch, then restores the source in a finally block; it defines the reproduction.

Targeted test output with unchanged resolver

  • The targeted Cargo test ran against the unchanged resolver and exited 0; the baseline passes.

Targeted test output with all-None fallback

  • The same Cargo test compiled and ran with the error branch returning the default all-None identity and exited 0; the test does not catch that regression.

Mutation runner and test exit codes

  • The Python reproduction command completed with exit code 0 and reported exit code 0 for each test run; both variants passed.

Source restoration check

  • The executed diff check returned 0 and the numbered source output identifies the assertion and fallback branch; the tracked source was restored.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-app-core/src/server/comm_session_tests.rs
Line: 831-834

Comment:
**Fallback Test Misses Regression**

The assertion passes when both the model and provider key are `None`. The test therefore misses the all-None fallback regression it is intended to catch, allowing that regression to pass unnoticed. Set a known default in the test and assert the resulting identity.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Comment on lines +305 to +315
crate::env::remove_var("JCODE_PROVIDER_PROFILE_NAME");
crate::env::remove_var("JCODE_NAMED_PROVIDER_PROFILE");
crate::env::remove_var("JCODE_PROVIDER_PROFILE_ACTIVE");
crate::env::remove_var("JCODE_OPENROUTER_API_BASE");
crate::env::remove_var("JCODE_OPENROUTER_ALLOW_NO_AUTH");
crate::env::remove_var("JCODE_OPENROUTER_MODEL");
crate::env::remove_var("JCODE_OPENROUTER_STATIC_MODELS");
crate::env::remove_var("JCODE_OPENROUTER_CACHE_NAMESPACE");
crate::env::remove_var("JCODE_OPENROUTER_PROVIDER_FEATURES");
crate::env::remove_var("JCODE_OPENROUTER_TRANSPORT_STATE");
crate::env::remove_var("JCODE_OPENROUTER_MODEL_CATALOG");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Tests Discard Environment Values

The Jcode test clears provider-profile and OpenRouter variables without restoring their previous values; the Ollama test also clears three profile variables absent from its restore list. If those values were set before either test, later tests in the same process can see them missing, making results depend on test order.

Knowledge Base Used: Provider selection and runtime adapters

Artifacts

Environment reproduction source

  • This authored generator extracts the relevant blocks from the tracked test file and prepares Rust programs for comparison.

Baseline Rust source

  • This generated Rust source runs the same cleanup with the disputed removals omitted, establishing the comparison condition.

Rust source with test removals

  • This generated Rust source includes the removal, saved-key, and restoration blocks extracted from the tests.

Baseline run with sentinel values retained

  • The compiled baseline ran successfully and retained all sampled environment sentinels.

Test-block run with sentinel values erased

  • The compiled extracted test blocks ran successfully and left the sampled pre-existing values unset.

Full-test attempt waiting for build lock

  • The targeted Cargo command waited for a shared build-directory lock and was terminated before running the test.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cli/provider_init_tests.rs
Line: 305-315

Comment:
**Tests Discard Environment Values**

The Jcode test clears provider-profile and OpenRouter variables without restoring their previous values; the Ollama test also clears three profile variables absent from its restore list. If those values were set before either test, later tests in the same process can see them missing, making results depend on test order.

**Knowledge Base Used:** [Provider selection and runtime adapters](https://app.greptile.com/solo-systems/-/custom-context/knowledge-base/1jehuang/jcode/-/docs/provider-selection-and-runtime-adapters.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Swarm agents use hardcoded/random model instead of inheriting current session model

1 participant