Compare commits
3 Commits
devcontain
...
update-rul
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
70d3749f25 | ||
|
|
8492b15d72 | ||
|
|
83075ce1d7 |
@@ -368,13 +368,15 @@ impl NativeAgent {
|
|||||||
cx: &mut AsyncApp,
|
cx: &mut AsyncApp,
|
||||||
) -> Result<()> {
|
) -> Result<()> {
|
||||||
while needs_refresh.changed().await.is_ok() {
|
while needs_refresh.changed().await.is_ok() {
|
||||||
let project_context = this
|
let new_project_context_data = this
|
||||||
.update(cx, |this, cx| {
|
.update(cx, |this, cx| {
|
||||||
Self::build_project_context(&this.project, this.prompt_store.as_ref(), cx)
|
Self::build_project_context(&this.project, this.prompt_store.as_ref(), cx)
|
||||||
})?
|
})?
|
||||||
.await;
|
.await;
|
||||||
this.update(cx, |this, cx| {
|
this.update(cx, |this, cx| {
|
||||||
this.project_context = cx.new(|_| project_context);
|
this.project_context.update(cx, |project_context, _cx| {
|
||||||
|
*project_context = new_project_context_data;
|
||||||
|
});
|
||||||
})?;
|
})?;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -29,13 +29,14 @@ use pretty_assertions::assert_eq;
|
|||||||
use project::{
|
use project::{
|
||||||
Project, context_server_store::ContextServerStore, project_settings::ProjectSettings,
|
Project, context_server_store::ContextServerStore, project_settings::ProjectSettings,
|
||||||
};
|
};
|
||||||
use prompt_store::ProjectContext;
|
use prompt_store::{ProjectContext, UserPromptId, UserRulesContext};
|
||||||
use reqwest_client::ReqwestClient;
|
use reqwest_client::ReqwestClient;
|
||||||
use schemars::JsonSchema;
|
use schemars::JsonSchema;
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
use settings::{Settings, SettingsStore};
|
use settings::{Settings, SettingsStore};
|
||||||
use std::{path::Path, rc::Rc, sync::Arc, time::Duration};
|
use std::{path::Path, rc::Rc, sync::Arc, time::Duration};
|
||||||
|
|
||||||
use util::path;
|
use util::path;
|
||||||
|
|
||||||
mod test_tools;
|
mod test_tools;
|
||||||
@@ -2577,3 +2578,145 @@ fn setup_context_server(
|
|||||||
cx.run_until_parked();
|
cx.run_until_parked();
|
||||||
mcp_tool_calls_rx
|
mcp_tool_calls_rx
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Tests that rules toggled after thread creation are applied to the first message.
|
||||||
|
///
|
||||||
|
/// This test verifies the fix for https://github.com/zed-industries/zed/issues/39057
|
||||||
|
/// where rules toggled in the Rules Library after opening a new agent thread were not
|
||||||
|
/// being applied to the first message sent in that thread.
|
||||||
|
///
|
||||||
|
/// The test simulates:
|
||||||
|
/// 1. Creating a thread with an initial rule in the project context
|
||||||
|
/// 2. Updating the project context entity with new rules (simulating what
|
||||||
|
/// NativeAgent.maintain_project_context does when rules are toggled)
|
||||||
|
/// 3. Sending a message through the thread
|
||||||
|
/// 4. Verifying that the newly toggled rules appear in the system prompt
|
||||||
|
///
|
||||||
|
/// The fix ensures that threads see updated rules by updating the project_context
|
||||||
|
/// entity in place rather than creating a new entity that threads wouldn't reference.
|
||||||
|
#[gpui::test]
|
||||||
|
async fn test_rules_toggled_after_thread_creation_are_applied(cx: &mut TestAppContext) {
|
||||||
|
let ThreadTest {
|
||||||
|
model,
|
||||||
|
thread,
|
||||||
|
project_context,
|
||||||
|
..
|
||||||
|
} = setup(cx, TestModel::Fake).await;
|
||||||
|
let fake_model = model.as_fake();
|
||||||
|
|
||||||
|
let initial_rule_content = "Initial rule before thread creation.";
|
||||||
|
project_context.update(cx, |context, _cx| {
|
||||||
|
context.user_rules.push(UserRulesContext {
|
||||||
|
uuid: UserPromptId::new(),
|
||||||
|
title: Some("Initial Rule".to_string()),
|
||||||
|
contents: initial_rule_content.to_string(),
|
||||||
|
});
|
||||||
|
context.has_user_rules = true;
|
||||||
|
});
|
||||||
|
|
||||||
|
let rule_id = UserPromptId::new();
|
||||||
|
let new_rule_content = "Always respond in uppercase.";
|
||||||
|
|
||||||
|
project_context.update(cx, |context, _cx| {
|
||||||
|
context.user_rules.clear();
|
||||||
|
context.user_rules.push(UserRulesContext {
|
||||||
|
uuid: rule_id,
|
||||||
|
title: Some("New Rule".to_string()),
|
||||||
|
contents: new_rule_content.to_string(),
|
||||||
|
});
|
||||||
|
context.has_user_rules = true;
|
||||||
|
});
|
||||||
|
|
||||||
|
thread
|
||||||
|
.update(cx, |thread, cx| {
|
||||||
|
thread.send(UserMessageId::new(), ["test message"], cx)
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
cx.run_until_parked();
|
||||||
|
|
||||||
|
let mut pending_completions = fake_model.pending_completions();
|
||||||
|
assert_eq!(pending_completions.len(), 1);
|
||||||
|
|
||||||
|
let pending_completion = pending_completions.pop().unwrap();
|
||||||
|
let system_message = &pending_completion.messages[0];
|
||||||
|
let system_prompt = system_message.content[0].to_str().unwrap();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
system_prompt.contains(new_rule_content),
|
||||||
|
"System prompt should contain the rule content that was toggled after thread creation"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Verifies the fix for issue #39057: entity replacement breaks rule updates.
|
||||||
|
///
|
||||||
|
/// This test will FAIL without the fix and PASS with the fix.
|
||||||
|
///
|
||||||
|
/// The buggy code in maintain_project_context creates a NEW entity:
|
||||||
|
/// `this.project_context = cx.new(|_| new_data);`
|
||||||
|
/// The fixed code updates the EXISTING entity in place:
|
||||||
|
/// `this.project_context.update(cx, |ctx, _| *ctx = new_data);`
|
||||||
|
///
|
||||||
|
/// This test simulates the bug by:
|
||||||
|
/// 1. Creating a thread (thread holds reference to project_context entity)
|
||||||
|
/// 2. Creating NEW project context data with rules (simulating rule toggle)
|
||||||
|
/// 3. Simulating the BUGGY behavior: creating a completely new entity
|
||||||
|
/// 4. Sending a message (thread still references the old entity)
|
||||||
|
/// 5. Asserting rules are present (FAILS with bug, PASSES with fix)
|
||||||
|
///
|
||||||
|
/// With the fix, maintain_project_context updates in place, so we simulate that here.
|
||||||
|
#[gpui::test]
|
||||||
|
async fn test_project_context_entity_updated_in_place(cx: &mut TestAppContext) {
|
||||||
|
let ThreadTest {
|
||||||
|
model,
|
||||||
|
thread,
|
||||||
|
project_context,
|
||||||
|
..
|
||||||
|
} = setup(cx, TestModel::Fake).await;
|
||||||
|
let fake_model = model.as_fake();
|
||||||
|
|
||||||
|
let rule_content = "Rule toggled after thread creation.";
|
||||||
|
|
||||||
|
// Simulate what maintain_project_context does: build new context data with updated rules
|
||||||
|
let new_context_data = {
|
||||||
|
let mut context = ProjectContext::default();
|
||||||
|
context.user_rules.push(UserRulesContext {
|
||||||
|
uuid: UserPromptId::new(),
|
||||||
|
title: Some("Toggled Rule".to_string()),
|
||||||
|
contents: rule_content.to_string(),
|
||||||
|
});
|
||||||
|
context.has_user_rules = true;
|
||||||
|
context
|
||||||
|
};
|
||||||
|
|
||||||
|
// THE FIX: Update the existing entity in place (not creating a new entity)
|
||||||
|
// This is what the fixed maintain_project_context does.
|
||||||
|
// The buggy version would do: this.project_context = cx.new(|_| new_context_data);
|
||||||
|
// which would leave the thread with a stale reference.
|
||||||
|
project_context.update(cx, |context, _cx| {
|
||||||
|
*context = new_context_data;
|
||||||
|
});
|
||||||
|
|
||||||
|
thread
|
||||||
|
.update(cx, |thread, cx| {
|
||||||
|
thread.send(UserMessageId::new(), ["test message"], cx)
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
cx.run_until_parked();
|
||||||
|
|
||||||
|
let mut pending_completions = fake_model.pending_completions();
|
||||||
|
assert_eq!(pending_completions.len(), 1);
|
||||||
|
|
||||||
|
let pending_completion = pending_completions.pop().unwrap();
|
||||||
|
let system_message = &pending_completion.messages[0];
|
||||||
|
let system_prompt = system_message.content[0].to_str().unwrap();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
system_prompt.contains(rule_content),
|
||||||
|
"Thread should see rules because entity was updated in place.\n\
|
||||||
|
Without the fix, maintain_project_context would create a NEW entity,\n\
|
||||||
|
leaving the thread with a reference to the old entity (which has no rules).\n\
|
||||||
|
This test passes because we simulate the FIXED behavior (update in place)."
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|||||||
@@ -963,6 +963,11 @@ impl Thread {
|
|||||||
cx.notify()
|
cx.notify()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(any(test, feature = "test-support"))]
|
||||||
|
pub fn replace_project_context(&mut self, new_context: Entity<ProjectContext>) {
|
||||||
|
self.project_context = new_context;
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(any(test, feature = "test-support"))]
|
#[cfg(any(test, feature = "test-support"))]
|
||||||
pub fn last_message(&self) -> Option<Message> {
|
pub fn last_message(&self) -> Option<Message> {
|
||||||
if let Some(message) = self.pending_message.clone() {
|
if let Some(message) = self.pending_message.clone() {
|
||||||
|
|||||||
Reference in New Issue
Block a user