Skip to content

Commit cd71927

Browse files
committed
fix: back off deferred DING probes
1 parent f272019 commit cd71927

4 files changed

Lines changed: 76 additions & 27 deletions

File tree

INVARIANTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ materialization, messaging, DING, or presence must preserve them.
1212
| **Exactly-once-safe native bus** | Messages use stable `<unix-ms>-<rand6>.md` files. An archive filename is a durable receipt that shadows and cleans restored inbox replicas and makes repeated archive cleanup idempotent. | `src/message.rs::filename_grammar`; `src/message.rs::archive_receipt_suppresses_and_idempotently_cleans_a_restored_inbox_copy`; `tests/message.rs` |
1313
| **Fail-closed observed native DING** | Each unread message becomes one normalized `[DING]` frame. A maintained Codex or Claude composer must be positively empty before a bracketed paste, then show the exact notice in two immediately adjacent inspections before a separate bare Return. Human, modal, active, changed, timed-out, and unknown states never receive Return. Once paste starts, inspect-only staged ownership prevents duplicate paste across command failures, archive races, and restart adoption. Startup backlog otherwise becomes one generic recovery DING; new arrivals remain FIFO; `busy` delivers immediately; only fresh `dnd` defers. | `src/ding.rs::poke_text_normalizes_and_bounds_untrusted_fields`; `src/ding.rs::malicious_controls_cannot_escape_the_single_paste_frame`; `src/ding.rs::pty_stage_and_submit_are_separate_exact_sequences`; `src/ding.rs::maintained_composer_classifiers_require_exact_idle_state`; `src/ding.rs::paste_then_two_exact_observations_precede_return`; `src/ding.rs::changed_modal_ambiguous_and_bounded_timeout_never_return`; `src/ding.rs::final_observation_change_and_staged_retry_are_fail_closed`; `src/ding.rs::staged_ownership_survives_archive_and_never_repastes`; `src/ding.rs::pty_commands_have_a_real_outer_timeout`; `src/ding.rs::session_watch_has_startup_grace_debounce_and_live_reset`; `src/ding.rs::new_arrivals_is_fifo_and_archive_receipts_prevent_reding`; `src/ding.rs::pending_delivery_ignores_busy_but_respects_fresh_dnd_archive_and_retry`; `src/ding.rs::startup_recovery_notice_retries_in_memory`; `src/ding.rs::startup_backlog_gets_one_generic_recovery_then_new_arrivals_poke` |
1414
| **Mutation-only filesystem wakeups** | Supervisor and DING filesystem watchers ignore read/open access events and wake early only for create, modify, rename, or remove events. Their own catalog and inbox reads therefore cannot bypass the bounded timer cadence or form a Linux inotify CPU loop. | `src/watch.rs::only_mutations_wake_watch_loops`; `src/watch.rs::linux_reads_are_silent_but_real_mutations_wake`; `src/ding.rs::idle_ding_does_not_spin_on_its_own_inbox_reads`; `src/run.rs::idle_supervisor_does_not_spin_on_its_own_catalog_reads` |
15+
| **Bounded DING PTY probe churn** | An unsafe or active composer retains its FIFO notice but deferred delivery retries use a bounded backoff, so each inbox poll cannot spawn another short-lived PTY probe. | `src/ding.rs::deferred_delivery_backoff_bounds_short_lived_pty_attempts` |
1516
| **Agent-declared presence discipline** | The shipped bus contract requires agents to declare `busy` before executing work, use `available` only while yielding or ready, and reserve `dnd` for an explicit hold. Both native harnesses materialize that contract. Busy remains observable but does not suppress DING; fresh `dnd` is the only delivery gate. | `tests/compile_agent.rs::compile_agent_generates_claude_then_materializes_verbatim_persona`; `tests/compile_agent.rs::compile_agent_generates_codex_then_materializes_composed_agents_md`; `src/ding.rs::pending_delivery_ignores_busy_but_respects_fresh_dnd_archive_and_retry` |
1617
| **Stable roster JSON** | `st2 agents --json [--enrich]` preserves field names, order, null handling, presence, explicit retirement state, activity, and inbox counts. Human output marks retired declarations without changing active rows. | `src/agents.rs::agents_json_has_stable_wire_shape`; `tests/status_agents.rs::roster_json_and_human_output_distinguish_retirement_from_presence` |
1718
| **Agent-declared presence** | Refresh preserves non-DND declared status and only advances liveness; a missing status starts as `available`, while `dnd` is never refreshed and an unrefreshed declaration ages to `unknown`. | `src/status.rs::refresh_preserves_value_and_bumps_mtime`; `src/status.rs::refresh_leaves_dnd_to_age_out`; `src/status.rs::refresh_missing_writes_available_default`; `src/status.rs::stale_mtime_reads_as_unknown_regardless_of_contents` |

README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,9 @@ Agents must declare `busy` before actively executing work and return to `availab
247247
yielding or ready for new work, but `busy` never suppresses DING. Fresh `dnd` is the only delivery
248248
hold. The sidecar does not refresh `dnd`, so an abandoned hold becomes stale after 15 minutes and
249249
delivery resumes. New arrivals remain FIFO, same-filename archive receipts shadow and clean restored
250-
inbox duplicates, and failed or uncertain PTY operations retain the notice for safe retry. On start
250+
inbox duplicates, and failed or uncertain PTY operations retain the notice for safe retry. Unsafe
251+
delivery retries use a bounded backoff, so an active composer cannot make the sidecar spawn a fresh
252+
PTY probe on every inbox poll. On start
251253
or restart, the sidecar first adopts an exact staged recovery/backlog notice when present, then sends
252254
one generic check-inbox recovery DING if unread work remains; it does not replay every backlog
253255
message.

docs/vrs/spec.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,9 @@ atomic inbox file → DING attempt → agent reads → archive receipt
7171
`busy` does not. Failed delivery remains retryable. Sidecar restart emits a
7272
bounded recovery notice instead of replaying the inbox. Delivery may wake an
7373
agent while it is working, but an active or uncertain human composer must be
74-
left untouched. Inbox reads do not wake the sidecar; only mutations bypass
75-
its bounded poll cadence.
74+
left untouched. Unsafe delivery retries use a bounded backoff so an active
75+
composer cannot create a short-lived PTY probe on every inbox poll. Inbox
76+
reads do not wake the sidecar; only mutations bypass its bounded poll cadence.
7677

7778
## State and scope
7879

src/ding.rs

Lines changed: 69 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ const RECOVERY_POKE: &str = "[DING] unread st2 messages remain; check your inbox
3232
const PTY_COMMAND_TIMEOUT: Duration = Duration::from_millis(600);
3333
const COMPOSER_OBSERVATION_WINDOW: Duration = Duration::from_millis(450);
3434
const COMPOSER_OBSERVATION_POLL: Duration = Duration::from_millis(10);
35+
/// A human or active turn can keep a staged notice unsafe for minutes. Retrying `pty peek` every
36+
/// inbox poll creates a short-lived child for each attempt, so keep the correctness fallback but
37+
/// bound that descendant churn independently of the filesystem poll cadence.
38+
const DELIVERY_RETRY_BACKOFF: Duration = Duration::from_secs(15);
3539

3640
/// The `<rand6>` of a `<unix-ms>-<rand6>.md` filename — the stable id an agent dedups re-pokes on.
3741
/// Falls back to the `.md`-stripped stem for anything off-grammar.
@@ -1030,6 +1034,7 @@ pub fn run_ding(
10301034
let mut watch = SessionWatch::default();
10311035
let mut logged_waiting = false;
10321036
let mut last_refresh: Option<Instant> = None;
1037+
let mut next_delivery_attempt: Option<Instant> = None;
10331038

10341039
loop {
10351040
if stop.load(Ordering::SeqCst) {
@@ -1050,36 +1055,41 @@ pub fn run_ding(
10501055
last_refresh = Some(Instant::now());
10511056
}
10521057

1053-
if !delivery_suppressed(status_path)
1054-
&& let Some(candidates) = startup_candidates.as_ref()
1055-
{
1056-
match poker.adopt_staged(candidates) {
1057-
Ok(Some(text)) => {
1058-
if text == RECOVERY_POKE {
1059-
if let Some(recovery) = pending
1060-
.iter_mut()
1061-
.find(|notice| matches!(notice, PendingNotice::Recovery { .. }))
1062-
{
1063-
recovery.set_staged_text(Some(text));
1064-
}
1065-
} else {
1066-
pending.push_front(PendingNotice::adopted(text));
1067-
}
1068-
startup_candidates = None;
1069-
}
1070-
Ok(None) => startup_candidates = None,
1071-
Err(error) => {
1072-
eprintln!("st2 ding: startup staged-notice adoption failed: {error}")
1073-
}
1074-
}
1075-
}
10761058
pending.extend(
10771059
new_arrivals(inbox_dir, &mut seen)
10781060
.into_iter()
10791061
.map(PendingNotice::message),
10801062
);
10811063
prune_archived_pending(inbox_dir, &mut pending);
1082-
flush_pending(status_path, &mut pending, poker);
1064+
1065+
let delivery_due =
1066+
next_delivery_attempt.is_none_or(|deadline| Instant::now() >= deadline);
1067+
if delivery_due && !delivery_suppressed(status_path) {
1068+
if let Some(candidates) = startup_candidates.as_ref() {
1069+
match poker.adopt_staged(candidates) {
1070+
Ok(Some(text)) => {
1071+
if text == RECOVERY_POKE {
1072+
if let Some(recovery) = pending
1073+
.iter_mut()
1074+
.find(|notice| matches!(notice, PendingNotice::Recovery { .. }))
1075+
{
1076+
recovery.set_staged_text(Some(text));
1077+
}
1078+
} else {
1079+
pending.push_front(PendingNotice::adopted(text));
1080+
}
1081+
startup_candidates = None;
1082+
}
1083+
Ok(None) => startup_candidates = None,
1084+
Err(error) => {
1085+
eprintln!("st2 ding: startup staged-notice adoption failed: {error}")
1086+
}
1087+
}
1088+
}
1089+
flush_pending(status_path, &mut pending, poker);
1090+
next_delivery_attempt = (startup_candidates.is_some() || !pending.is_empty())
1091+
.then(|| Instant::now() + DELIVERY_RETRY_BACKOFF);
1092+
}
10831093
} else if !watch.seen_alive && !logged_waiting {
10841094
eprintln!(
10851095
"st2 ding: target pty session not yet registered; waiting before enabling exit-when-gone."
@@ -1241,6 +1251,7 @@ mod tests {
12411251
#[derive(Default)]
12421252
struct RecordingPoker {
12431253
alive: AtomicBool,
1254+
defer: AtomicBool,
12441255
probes: AtomicUsize,
12451256
failures: Mutex<usize>,
12461257
calls: Mutex<Vec<String>>,
@@ -1258,6 +1269,9 @@ mod tests {
12581269
impl Poker for RecordingPoker {
12591270
fn poke(&self, text: &str) -> anyhow::Result<PokeOutcome> {
12601271
self.calls.lock().unwrap().push(text.to_string());
1272+
if self.defer.load(Ordering::SeqCst) {
1273+
return Ok(PokeOutcome::Deferred);
1274+
}
12611275
let mut failures = self.failures.lock().unwrap();
12621276
if *failures > 0 {
12631277
*failures -= 1;
@@ -1879,6 +1893,37 @@ mod tests {
18791893
assert!(!calls.iter().any(|call| call.contains("seeded")));
18801894
}
18811895

1896+
#[test]
1897+
fn deferred_delivery_backoff_bounds_short_lived_pty_attempts() {
1898+
let agent = tempfile::tempdir().unwrap();
1899+
let inbox = inbox_dir(agent.path());
1900+
let status_path = status::status_path(agent.path());
1901+
status::set_state(&status_path, status::State::Available).unwrap();
1902+
send_to_inbox(&inbox, "alice", Some("active"), None, &[], "active").unwrap();
1903+
1904+
let poker = RecordingPoker::live();
1905+
poker.defer.store(true, Ordering::SeqCst);
1906+
let stop = AtomicBool::new(false);
1907+
let config = DingConfig {
1908+
poll: Duration::from_millis(20),
1909+
status_refresh: Duration::from_secs(60),
1910+
};
1911+
1912+
std::thread::scope(|scope| {
1913+
scope.spawn(|| {
1914+
std::thread::sleep(Duration::from_millis(250));
1915+
stop.store(true, Ordering::SeqCst);
1916+
});
1917+
run_ding(&inbox, Some(&status_path), &poker, &config, &stop).unwrap();
1918+
});
1919+
1920+
assert_eq!(
1921+
poker.calls.lock().unwrap().len(),
1922+
1,
1923+
"an unsafe composer must not respawn pty peek/send children every inbox poll"
1924+
);
1925+
}
1926+
18821927
#[cfg(target_os = "linux")]
18831928
#[test]
18841929
fn idle_ding_does_not_spin_on_its_own_inbox_reads() {

0 commit comments

Comments
 (0)