codewhale-hq/Codewhale
1
0 Fork 0
mirror of https://github.com/codewhale-hq/Codewhale.git synced 2026-11-07 17:09:13 -10:00
Code Issues Packages Projects Releases 1 Wiki Activity

fix(resume): release stale session holders instead of refusing resume

Browse Source 
Refs #6954

Two refusals in the saved-history holder gate of mutate_in_owner blocked
real sessions after 0.10.2 (acceptance report on #6954).

A. Admission before writes. A resume bound the target thread to the
document (seed transaction plus set_thread_session_checkpoint) before the
first Engine load took the Runtime's exclusive workspace coordination
flock. When another Codewhale process held that flock, the attempt failed
after a bound, uncommitted holder was already durable, and the next
resume (fresh key) refused it. New
RuntimeThreadManager::admit_history_execution runs the existing
ensure_accepting_execution plus subagent_manager admission (cached, so the
later Engine load reuses it). It is called in mutate_in_owner before any
reservation or write, and in settle_prepared_history_operation before its
seed/rebind, so the retained-key and recover paths are covered too.

B. Holder gate decides staleness by turns, not bytes. The checkpoint
fingerprints the raw head at seed time, and the gate's "live" side is the
Engine restore projection, so a document whose head the TUI rewrote
(compaction checkpoint, contract pruning) or that still holds raw
sub-agent handoffs failed with "saved holder checkpoint differs" or
"successor outside the selected document". New
checkpoint_covers_every_turn(checkpoint, turns) is true when the
checkpoint's covered_turn_id is the holder's last turn record (or both are
empty), i.e. the holder ran nothing after it was bound. It is computed
from get_thread_detail's turns, the same position rule restore uses.
- Holder covers every turn and its checkpoint covers the whole document:
  reuse it without loading its Engine (the projection-aware SyncSession
  acknowledgement still proves the install). This avoids minting a new
  thread on every resume of a document with raw handoffs.
- Holder covers every turn otherwise (document grew, or head rewritten):
  release it.
- Holder ran turns past its checkpoint: unchanged rules. Checkpoint must
  be a prefix, then snapshot; equal reuses, strict prefix releases (after
  re-reading the record, because the snapshot can rewrite a legacy
  checkpoint's messages_len), anything else still fails closed with
  "successor outside the selected document".
Every release now runs through history_owner_work (#6149).

C. Release can no longer silently miss. unbind_stale_session returns
whether it unbound and, for the release path only, also pins
latest_turn_id in its compare-and-swap under thread_mutation.
release_stale_session_holder fails with "saved holder changed during
admission; retry" when nothing was unbound, so a fresh mint can never
create a second holder. The restore caller keeps its tolerant behaviour
(no turn pin, result ignored); runtime_threads::tests::recovery::
saved_checkpoint_validation_and_forks_preserve_history depends on that.

Removed the two gate comments the change made false.

Evidence (source 3f0bcb4166 + this diff, macOS arm64, all under
scripts/build-lock.py):
- scripts/dev-test.sh tui mounted_history_tests: 14 tests run: 14 passed
- scripts/dev-test.sh tui runtime_store_convergence: 14 tests run: 14 passed
- scripts/dev-test.sh tui runtime_threads::tests::seed: 3 tests run: 3 passed
- scripts/dev-test.sh tui thread_history: 5 tests run: 5 passed
- scripts/dev-test.sh tui saved_checkpoint_validation_and_forks_preserve_history:
  1 test run: 1 passed
- scripts/dev-cargo.sh test -p codewhale-tui --features long-running-tests
  --test cucumber --locked a_continued_session_resumes_again_after_each_turn
  (hermetic PTY, no provider): test result: ok. 1 passed; 0 failed
- CI-form clippy -p codewhale-tui --all-targets --all-features --locked
  -D warnings: clean
- cargo fmt --all -- --check: clean
- scripts/check-blocking-calls-budget.py: 539 sites, within budget

No test needed changing; none asserted either refusal message.

Review: an adversarial review returned "push". From its low findings, the
post-snapshot re-read now also requires the holder's latest_turn_id to be
unchanged before the strict-prefix release. That race is unreachable while
the config_admission write guard is held. After this amendment, the same
five focused sets re-ran: 37 tests run, 37 passed; clippy exit 0; fmt clean.

Limits: neither reported variant was reproduced end to end against real
data; the manual two-terminal repro (flock held by another TUI) and the
head-rewrite repro were not run. `codewhale resume` still refuses while
another TUI holds the workspace coordination flock; it just no longer
leaves a holder behind. The lib.rs wrapper still suggests exact-key
recovery for refusals that now happen before any reservation (harmless:
lookup returns Absent). An idle holder whose background sub-agents or
shell jobs are still running (not counted as live work) can be released
on the Runtime host path, as the strict-prefix release already could.
Released threads stay in the session store with a reconcile receipt.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01NBKbhx19BRbZtvWK8rVKjz
This commit is contained in:
CodeWhale BotandClaude Opus 5.5 committed 2026-10-17 00:19:42 -07:00
1 parent 3f0bcb4166
commit 15f1804fe8
2 files changed
+97 -50
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/tui/src/runtime_api/thread_history.rs
+59 -40
View File
@@ -990,6 +990,7 @@ async fn settle_prepared_history_operation(
})
.await?;
if let Some((saved, _target_lease)) = saved {
runtime.admit_history_execution().await?;
let from = witness.seed_from_message_index;
ensure!(
from <= saved.messages.len(),
@@ -1101,6 +1102,7 @@ async fn mutate_in_owner(
return Ok(receipt);
}
}
runtime.admit_history_execution().await?;
let is_resume = matches!(&request.mutation, CanonicalThreadMutation::Resume { .. });
let mut create = CreateThreadRequest::default();
let mut source_thread = None;
@@ -1238,48 +1240,65 @@ async fn mutate_in_owner(
.saved_session_checkpoint
.as_ref()
.ok_or_else(|| anyhow!("saved holder checkpoint is missing"))?;
// The checkpoint records the history the holder was
// seeded with; turns the holder ran since then extend the
// document past it. Like the other checkpoint guards here,
// require it to cover a prefix, and let the exact
// live == document check below prove that everything
// after that prefix came from this holder. Requiring full
// coverage refused every second resume of a continued
// session.
ensure!(
crate::runtime_threads::checkpoint_prefix_len(
checkpoint,
&observed.messages
)?
.is_some(),
"saved holder checkpoint differs from the saved history; refresh history"
);
let full = snapshot_in_runtime(&runtime, &sessions_dir, thread.id.clone())
.await
.map_err(|error| anyhow!(error.message))?;
let live: SavedSession = serde_json::from_value(full.session)?;
if live.messages == observed.messages && live.journal == observed.journal {
source_thread = Some(thread);
} else if live.messages.len() < observed.messages.len()
&& observed.messages.starts_with(&live.messages)
{
// After a resume the TUI runs its turns in its own
// engine and saves them to the document; this holder
// stayed at the history it was seeded with. It is
// strictly behind the document and has no live work
// (checked above), so it holds nothing the document
// lacks: release its binding (receipt kept, turns
// kept) and mount the document fresh. Refusing here
// made every continued session impossible to resume
// a second time.
runtime.release_stale_session_holder(
&thread,
"the saved document advanced past this thread after a resume",
)?;
let covered = crate::runtime_threads::checkpoint_prefix_len(
checkpoint,
&observed.messages,
)?;
let released = if crate::runtime_threads::checkpoint_covers_every_turn(
checkpoint,
&detail.turns,
) {
match covered {
Some(len) if len == observed.messages.len() => {
source_thread = Some(thread);
None
}
Some(_) => Some((
thread,
"the saved document advanced past this thread after a resume",
)),
None => Some((
thread,
"the saved document's head was rewritten after this thread's checkpoint",
)),
}
} else {
bail!(
"saved holder has a successor outside the selected document; refresh history"
ensure!(
covered.is_some(),
"saved holder checkpoint differs from the saved history; refresh history"
);
let full = snapshot_in_runtime(&runtime, &sessions_dir, thread.id.clone())
.await
.map_err(|error| anyhow!(error.message))?;
let live: SavedSession = serde_json::from_value(full.session)?;
if live.messages == observed.messages && live.journal == observed.journal {
source_thread = Some(thread);
None
} else if live.messages.len() < observed.messages.len()
&& observed.messages.starts_with(&live.messages)
{
let fresh = runtime.get_thread(&thread.id).await?;
ensure!(
fresh.session_id == thread.session_id
&& fresh.latest_turn_id == thread.latest_turn_id,
"saved holder changed during admission; retry"
);
Some((
fresh,
"the saved document advanced past this thread after a resume",
))
} else {
bail!(
"saved holder has a successor outside the selected document; refresh history"
);
}
};
if let Some((holder, reason)) = released {
let release_runtime = Arc::clone(&runtime);
history_owner_work(move || {
release_runtime.release_stale_session_holder(&holder, reason)
})
.await?;
}
}
observed
crates/tui/src/runtime_threads.rs
+38 -10
View File
@@ -931,6 +931,16 @@ pub(crate) fn checkpoint_prefix_len(
Ok(None)
}
pub(crate) fn checkpoint_covers_every_turn(
checkpoint: &SavedSessionCheckpoint,
turns: &[TurnRecord],
) -> bool {
match checkpoint.covered_turn_id.as_deref() {
Some(id) => turns.last().is_some_and(|turn| turn.id == id),
None => turns.is_empty(),
}
}
/// A thread's saved-session binding no longer describes any readable
/// document: the document is gone, or its prefix is not the one the thread
/// was bound to. The thread still owns its turns, so the binding is dropped
@@ -16453,6 +16463,11 @@ impl RuntimeThreadManager {
Ok(manager)
}
pub(crate) async fn admit_history_execution(&self) -> Result<()> {
self.ensure_accepting_execution()?;
self.subagent_manager().await.map(drop)
}
/// The thread's engine only when it is already live in this process.
/// Control routes that act on in-flight work (stopping an agent run) use
/// this: loading a cold engine cannot reach work that is not running here.
@@ -16594,7 +16609,7 @@ impl RuntimeThreadManager {
// The binding describes no readable document. The thread's
// own turns are its history; drop the dead link (keeping it
// in a receipt) instead of stranding the thread (#6144).
self.unbind_stale_session(thread, &stale.reason)?;
self.unbind_stale_session(thread, &stale.reason, false)?;
(Vec::new(), 0)
}
};
@@ -16603,15 +16618,22 @@ impl RuntimeThreadManager {
}
/// Drop `thread`'s saved-session binding when the stored record still
/// carries exactly the binding that was found stale, and record the old
/// binding in the session reconcile receipts.
fn unbind_stale_session(&self, thread: &ThreadRecord, reason: &str) -> Result<()> {
/// carries exactly the binding that was found stale (and, with
/// `pin_latest_turn`, the same latest turn), and record the old binding in
/// the session reconcile receipts. Returns whether the binding was dropped.
fn unbind_stale_session(
&self,
thread: &ThreadRecord,
reason: &str,
pin_latest_turn: bool,
) -> Result<bool> {
let _thread_mutation = self.store.thread_mutation.lock();
let mut stored = self.store.load_thread(&thread.id)?;
if stored.session_id != thread.session_id
|| stored.saved_session_checkpoint != thread.saved_session_checkpoint
|| (pin_latest_turn && stored.latest_turn_id != thread.latest_turn_id)
{
return Ok(());
return Ok(false);
}
crate::session_reconcile::record_thread_unbound(
&self.session_store_binding().data_dir,
@@ -16621,18 +16643,24 @@ impl RuntimeThreadManager {
stored.session_id = None;
stored.saved_session_checkpoint = None;
stored.updated_at = Utc::now();
self.store.save_thread(&stored)
self.store.save_thread(&stored)?;
Ok(true)
}
/// Release a canonical holder whose saved document has moved past it, using
/// the same compare-and-swap unbind and reconcile receipt as a stale
/// binding found while restoring messages.
/// Release a canonical holder that holds nothing its saved document lacks,
/// using the same compare-and-swap unbind and reconcile receipt as a stale
/// binding found while restoring messages. Fails when the stored record
/// changed since `thread` was read.
pub(crate) fn release_stale_session_holder(
&self,
thread: &ThreadRecord,
reason: &str,
) -> Result<()> {
self.unbind_stale_session(thread, reason)
anyhow::ensure!(
self.unbind_stale_session(thread, reason, true)?,
"saved holder changed during admission; retry"
);
Ok(())
}
/// Unbind every thread in this store that names `session_id` (#6144).