mirror of
https://github.com/codewhale-hq/Codewhale.git
synced 2026-11-08 17:10:50 -10:00
fix(session): /edit replaces the revised exchange again; resume by id survives a stray copy
/edit: since the #6788 change the command layer only stages a conversation rollback and the UI applies it after the Engine acknowledges it. The submit path for a pending /edit still ran "/undo" and discarded the result, so nothing was rolled back: the old prompt and its answer stayed in the transcript, the model context and the saved session, and the edited prompt was appended. 0.10.0 replaced them. The submit path now stages the rollback and applies it through the same action /retry uses, with the edited text as the replacement turn. Only the conversation is rolled back; files are not restored by an edit. Resume: a copy of a session file under another name is listed with the original's id, which made that id an "ambiguous prefix" and the session unresumable. Duplicate listings of one id now resolve to that session. Found by native acceptance; the same happens on earlier builds. Changelog: the two fixes above, and a Changed entry for the process-scoped PowerShell execution-policy setting that shipped earlier in this cycle without one (#6745). Tests (Rust 1.99, macOS arm64): - codewhale-tui --lib conversation_undo: test result: ok. 2 passed; 0 failed. The lifecycle test now drives /edit, submits, and checks the provider request, the app history and the saved session. - Control: with the rollback replaced by a plain send, that test fails with left: ["keep this", "after reopen", "edited prompt"] right: ["keep this", "edited prompt"]. - session_manager:: 130 passed; edit 197 passed; debug_mutation 27 passed; resume 148 passed; 0 failed in each. - CI-policy Clippy on the workspace, all targets and features: clean. Blocking-call budget within budget; changelog sync check up to date. Not run: native /edit in the TUI on this commit; Linux; Windows. Signed-off-by: CodeWhale Bot <[email protected]>
This commit is contained in:
1 parent
3665adde81
commit
6e2623a465
6 files changed
+126
-17
No files matched your search
@@ -15,6 +15,13 @@ quieter, and Fleet runs can be checked before they spend anything.
|
||||
|
||||
### Changed
|
||||
|
||||
- On Windows, every PowerShell command Codewhale starts now passes
|
||||
`-ExecutionPolicy Bypass` for that process only, so a local `Restricted` or
|
||||
`AllSigned` policy no longer blocks multi-line commands. A policy set by
|
||||
Group Policy still wins and the command is refused with PowerShell's own
|
||||
message. Scripts that a command invokes run under the same process-scoped
|
||||
setting. Not verified on a native Windows machine
|
||||
([#6745](https://github.com/Hmbown/Codewhale/issues/6745)).
|
||||
- The website's not-found page now uses the Codwhale poster and typo joke,
|
||||
with English/Chinese recovery links to home and docs
|
||||
([#6419](https://github.com/Hmbown/Codewhale/issues/6419),
|
||||
@@ -122,6 +129,11 @@ quieter, and Fleet runs can be checked before they spend anything.
|
||||
changing the draft. Retained compaction summaries survive the rewind and
|
||||
cannot be selected as editable user prompts
|
||||
([#6788](https://github.com/Hmbown/Codewhale/issues/6788)).
|
||||
- `/edit` replaces the exchange it revises through the same rollback: the old
|
||||
prompt and its answer leave the transcript, the model's context and the
|
||||
saved session before the edited prompt is sent.
|
||||
- A session can still be resumed by its id when a stray copy of its file sits
|
||||
in the sessions directory; before, the id was reported as ambiguous.
|
||||
- Agents follow the Permissions you choose while they run: switching to
|
||||
Full Access reaches an agent that is already working, instead of leaving
|
||||
it with the Auto-Review guardian that denied it. Tightening reaches it too.
|
||||
|
||||
@@ -15,6 +15,13 @@ quieter, and Fleet runs can be checked before they spend anything.
|
||||
|
||||
### Changed
|
||||
|
||||
- On Windows, every PowerShell command Codewhale starts now passes
|
||||
`-ExecutionPolicy Bypass` for that process only, so a local `Restricted` or
|
||||
`AllSigned` policy no longer blocks multi-line commands. A policy set by
|
||||
Group Policy still wins and the command is refused with PowerShell's own
|
||||
message. Scripts that a command invokes run under the same process-scoped
|
||||
setting. Not verified on a native Windows machine
|
||||
([#6745](https://github.com/Hmbown/Codewhale/issues/6745)).
|
||||
- The website's not-found page now uses the Codwhale poster and typo joke,
|
||||
with English/Chinese recovery links to home and docs
|
||||
([#6419](https://github.com/Hmbown/Codewhale/issues/6419),
|
||||
@@ -122,6 +129,11 @@ quieter, and Fleet runs can be checked before they spend anything.
|
||||
changing the draft. Retained compaction summaries survive the rewind and
|
||||
cannot be selected as editable user prompts
|
||||
([#6788](https://github.com/Hmbown/Codewhale/issues/6788)).
|
||||
- `/edit` replaces the exchange it revises through the same rollback: the old
|
||||
prompt and its answer leave the transcript, the model's context and the
|
||||
saved session before the edited prompt is sent.
|
||||
- A session can still be resumed by its id when a stray copy of its file sits
|
||||
in the sessions directory; before, the id was reported as ambiguous.
|
||||
- Agents follow the Permissions you choose while they run: switching to
|
||||
Full Access reaches an agent that is already working, instead of leaving
|
||||
it with the Auto-Review guardian that denied it. Tightening reaches it too.
|
||||
|
||||
@@ -53,6 +53,15 @@ mod session_lifecycle_regression_tests;
|
||||
use std::sync::OnceLock;
|
||||
|
||||
pub(crate) use groups::config::config::set_workspace_trust;
|
||||
|
||||
/// Stage a rollback of the last exchange for the UI to apply, or `None` when
|
||||
/// there is no user message to roll back. Nothing is mutated here.
|
||||
pub(crate) fn staged_conversation_undo(
|
||||
app: &mut crate::tui::app::App,
|
||||
) -> Option<codewhale_command_contract::facets::SessionSyncPayload> {
|
||||
let undone = contract::debug_operations::undo_conversation_for_engine(app);
|
||||
(undone.removed > 0).then_some(undone.sync)
|
||||
}
|
||||
pub use traits::CommandInfo;
|
||||
|
||||
// Long-standing public paths that predate the group layout.
|
||||
|
||||
@@ -3014,10 +3014,14 @@ impl SessionManager {
|
||||
pub(crate) fn resolve_session_id_prefix(&self, prefix: &str) -> std::io::Result<String> {
|
||||
let sessions = self.list_sessions()?;
|
||||
|
||||
let matches: Vec<_> = sessions
|
||||
// One session listed more than once (a stray copy of its file under
|
||||
// another name) is still one session, not an ambiguous prefix.
|
||||
let mut matches: Vec<_> = sessions
|
||||
.into_iter()
|
||||
.filter(|s| s.id.starts_with(prefix))
|
||||
.collect();
|
||||
matches.sort_by(|a, b| a.id.cmp(&b.id));
|
||||
matches.dedup_by(|a, b| a.id == b.id);
|
||||
|
||||
match matches.len() {
|
||||
0 => Err(std::io::Error::new(
|
||||
@@ -7457,6 +7461,15 @@ mod tests {
|
||||
std::fs::copy(&source, source.with_file_name("impostor.json")).expect("copy");
|
||||
|
||||
assert_eq!(manager.load_session(&id).expect("own id").metadata.id, id);
|
||||
// The stray copy does not make the real session ambiguous.
|
||||
assert_eq!(
|
||||
manager
|
||||
.load_session_by_prefix(&id)
|
||||
.expect("resume by id with a copy present")
|
||||
.metadata
|
||||
.id,
|
||||
id
|
||||
);
|
||||
let err = manager
|
||||
.load_session("impostor")
|
||||
.expect_err("mismatched id must fail");
|
||||
|
||||
@@ -1603,20 +1603,16 @@ async fn submit_decided_composer_input(
|
||||
// message, undo the last exchange before dispatching the
|
||||
// replacement. Sync the engine session so it also drops the old
|
||||
// exchange.
|
||||
if app.edit_in_progress {
|
||||
crate::commands::execute("/undo", app);
|
||||
app.edit_in_progress = false;
|
||||
let _ = engine_handle
|
||||
.send(Op::SyncSession {
|
||||
session_id: app.current_session_id.clone(),
|
||||
messages: app.api_messages.as_ref().clone(),
|
||||
system_prompt: app.system_prompt.clone(),
|
||||
system_prompt_override: false,
|
||||
model: app.model.clone(),
|
||||
workspace: app.workspace.clone(),
|
||||
mode: app.mode,
|
||||
})
|
||||
.await;
|
||||
if let Some(result) = edit_replacement_result(app, &input) {
|
||||
return apply_command_result(
|
||||
terminal,
|
||||
app,
|
||||
engine_handle,
|
||||
task_manager,
|
||||
config,
|
||||
result,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
let (queued, recovery) = message_from_submitted_input(app, input);
|
||||
dispatch_composer_message(app, config, engine_handle, queued, recovery, action).await?;
|
||||
@@ -1624,6 +1620,32 @@ async fn submit_decided_composer_input(
|
||||
Ok(false)
|
||||
}
|
||||
|
||||
/// The replacement for an exchange being revised with `/edit`: roll the last
|
||||
/// exchange back through the same Engine-acknowledged, durably saved path as
|
||||
/// `/retry`, then send `input` in its place. `None` when no edit is pending
|
||||
/// or there is nothing to replace, so the input is sent as a normal turn.
|
||||
///
|
||||
/// The rollback is staged, not applied, by the command layer (#6788); running
|
||||
/// `/undo` here and discarding its result left the old exchange in the
|
||||
/// transcript, the model context and the saved session.
|
||||
pub(super) fn edit_replacement_result(
|
||||
app: &mut App,
|
||||
input: &str,
|
||||
) -> Option<commands::CommandResult> {
|
||||
if !std::mem::take(&mut app.edit_in_progress) {
|
||||
return None;
|
||||
}
|
||||
let sync = crate::commands::staged_conversation_undo(app)?;
|
||||
Some(commands::CommandResult {
|
||||
message: None,
|
||||
action: Some(AppAction::ConversationUndo {
|
||||
sync,
|
||||
retry_input: Some(input.to_string()),
|
||||
}),
|
||||
is_error: false,
|
||||
})
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_lines, clippy::too_many_arguments)]
|
||||
/// Whether the git probe may run this tick: whenever the workspace-context
|
||||
/// refresh may, and also during a live turn or agent run while the Git view
|
||||
|
||||
@@ -125,6 +125,7 @@ fn undo_retry_apply_path_keeps_engine_request_and_reopened_session_consistent()
|
||||
canned::simple_text_turn("retry answer one"),
|
||||
canned::simple_text_turn("retry answer two"),
|
||||
canned::simple_text_turn("answer after reopen"),
|
||||
canned::simple_text_turn("edited answer"),
|
||||
]));
|
||||
let (engine, mut handle) = Engine::new_with_model_client(
|
||||
EngineConfig {
|
||||
@@ -467,6 +468,46 @@ fn undo_retry_apply_path_keeps_engine_request_and_reopened_session_consistent()
|
||||
assert!(!inbound.contains("undone answer"));
|
||||
assert!(!inbound.contains("retry answer"));
|
||||
|
||||
// `/edit` then submit replaces the last exchange: the old prompt and
|
||||
// its answer leave the transcript, the provider request and the saved
|
||||
// session, and the edited prompt takes their place.
|
||||
let loaded = commands::execute("/edit", &mut app);
|
||||
assert!(!loaded.is_error, "{:?}", loaded.message);
|
||||
assert_eq!(app.input, "after reopen");
|
||||
assert!(app.edit_in_progress);
|
||||
let result = super::super::event_loop::edit_replacement_result(&mut app, "edited prompt")
|
||||
.expect("a pending edit stages a replacement");
|
||||
assert!(!app.edit_in_progress);
|
||||
apply_command_result(
|
||||
&mut terminal,
|
||||
&mut app,
|
||||
&mut handle,
|
||||
&tasks,
|
||||
&mut config,
|
||||
result,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
settled(&mut app, &handle, "edited answer").await;
|
||||
let requests = mock.captured_requests();
|
||||
assert_eq!(requests.len(), 6);
|
||||
assert_eq!(
|
||||
prompts(&requests.last().unwrap().messages),
|
||||
["keep this", "edited prompt"]
|
||||
);
|
||||
let inbound = serde_json::to_string(&requests.last().unwrap().messages).unwrap();
|
||||
assert!(!inbound.contains("after reopen"));
|
||||
assert_eq!(prompts(&app.api_messages), ["keep this", "edited prompt"]);
|
||||
assert_eq!(
|
||||
prompts(&manager.load_session(&id).unwrap().messages),
|
||||
["keep this"],
|
||||
"the edit rollback is durable before the replacement turn"
|
||||
);
|
||||
// With no edit pending, a submit is an ordinary turn.
|
||||
assert!(
|
||||
super::super::event_loop::edit_replacement_result(&mut app, "plain").is_none()
|
||||
);
|
||||
|
||||
// Return to the retained exchange before probing durable-save failure.
|
||||
let result = commands::execute("/undo", &mut app);
|
||||
apply_command_result(
|
||||
@@ -498,7 +539,7 @@ fn undo_retry_apply_path_keeps_engine_request_and_reopened_session_consistent()
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
mock.captured_requests().len(),
|
||||
5,
|
||||
6,
|
||||
"save failure must forbid retry inference"
|
||||
);
|
||||
assert!(
|
||||
@@ -549,7 +590,7 @@ fn undo_retry_apply_path_keeps_engine_request_and_reopened_session_consistent()
|
||||
.text
|
||||
.contains("retry was not sent")
|
||||
);
|
||||
assert_eq!(mock.captured_requests().len(), 5);
|
||||
assert_eq!(mock.captured_requests().len(), 6);
|
||||
tasks.shutdown_and_wait().await.unwrap();
|
||||
assert!(actor.try_send(PersistRequest::Shutdown));
|
||||
actor_task.await.unwrap();
|
||||
|
||||
Reference in new issue
Block a user