Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/ding/composer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,17 @@ pub(super) fn strip_ansi(input: &str) -> String {
/// it can type into, or submit, a human's live draft. Scrollback is above the live composer by
/// construction, so picking the lowest needs no per-pair special case.
pub(super) fn classify_composer(screen: &str, expected: &str) -> ComposerState {
classify_located_composer(screen, expected).0
}

/// The same routing, additionally naming which maintained harness owned the composer that was
/// classified. `None` means no maintained composer was locatable at all — which is the difference
/// between "a human is drafting in a harness we understand" and "we do not recognise this pane",
/// and the two want different operator responses.
pub(super) fn classify_located_composer(
screen: &str,
expected: &str,
) -> (ComposerState, Option<&'static str>) {
let plain = strip_ansi(screen);
let screen = Screen {
raw: screen,
Expand All @@ -190,9 +201,9 @@ pub(super) fn classify_composer(screen: &str, expected: &str) -> ComposerState {
.map(|located| (located.row, harness))
})
.max_by_key(|(row, _)| *row)
.map(|(_, harness)| harness.classify(&screen, expected))
.map(|(_, harness)| (harness.classify(&screen, expected), Some(harness.name())))
// No maintained composer is locatable, so nothing is proven either way.
.unwrap_or(ComposerState::Ambiguous)
.unwrap_or((ComposerState::Ambiguous, None))
}

/// Route post-submit receipt classification through the lowest maintained live composer, using
Expand Down
4 changes: 4 additions & 0 deletions src/ding/harness/claude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ use crate::ding::composer::{
pub(super) struct Claude;

impl Harness for Claude {
fn name(&self) -> &'static str {
"claude"
}

fn locate(&self, screen: &Screen<'_>) -> Option<Located> {
located_bottom_claude_composer(screen.plain).map(|(row, _, _)| Located { row })
}
Expand Down
4 changes: 4 additions & 0 deletions src/ding/harness/codex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ use crate::ding::composer::{
pub(super) struct Codex;

impl Harness for Codex {
fn name(&self) -> &'static str {
"codex"
}

fn locate(&self, screen: &Screen<'_>) -> Option<Located> {
// The markers are ANSI, so this locator works in raw byte offsets while the router compares
// stripped rows. Every marker starts at an `\x1b[` boundary, so stripping the prefix is
Expand Down
3 changes: 3 additions & 0 deletions src/ding/harness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ pub(super) struct Located {
}

pub(super) trait Harness {
/// How this harness is named in operator-facing diagnostics.
fn name(&self) -> &'static str;

/// Locate this harness's composer, if this screen has one.
fn locate(&self, screen: &Screen<'_>) -> Option<Located>;

Expand Down
4 changes: 4 additions & 0 deletions src/ding/harness/opencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ use crate::ding::composer::ComposerState;
pub(super) struct OpenCode;

impl Harness for OpenCode {
fn name(&self) -> &'static str {
"opencode"
}

fn locate(&self, screen: &Screen<'_>) -> Option<Located> {
locate_composer(screen.plain).map(|composer| Located {
row: composer.start,
Expand Down
203 changes: 192 additions & 11 deletions src/ding/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::run::{CAPTURE_CAP_BYTES, read_bounded_tail, reap_detached};
use crate::status;
use crate::supervisor_chain::{SUPERVISOR_CHAIN_LIMIT, chain_bus_ids, resolve_spec};

use composer::{ComposerState, classify_composer, classify_receipt};
use composer::{ComposerState, classify_composer, classify_located_composer, classify_receipt};
use harness::ReceiptState;

const BRACKETED_PASTE_START: &str = "\x1b[200~";
Expand Down Expand Up @@ -243,15 +243,82 @@ pub enum PokeOutcome {
/// A maintained adapter positively proved that the exact staged notice is absent. Queue state
/// decides whether an archive receipt makes that proof sufficient to relinquish ownership.
NotRetained,
Deferred,
Deferred(DeferralReason),
}

/// Why one attempt performed no input at all.
///
/// A deferral is the one outcome that both delivers nothing and leaves nothing behind, so it is
/// the one that must say why. The two composer verdicts are deliberately distinct: a human drafting
/// in a harness we understand is a wait, while a pane no maintained harness can locate is a gap in
/// coverage that will never resolve on its own.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum DeferralReason {
/// The lowest maintained composer holds text that is not the exact notice — typically a human
/// draft. Named by harness only: the text on that pane belongs to whoever is typing it.
ComposerChanged { harness: &'static str },
/// A maintained harness located its composer but proved nothing about this screen — an active
/// turn, a modal, or a footer it does not recognise. Distinct from an unlocatable pane: this
/// one is covered and can clear on its own, so it is a wait rather than a coverage gap.
ComposerUnproven { harness: &'static str },
/// No maintained harness could locate a composer on this pane, so nothing is proven either
/// way. An unrecognised, resized, or not-yet-drawn TUI lands here.
NoMaintainedComposer,
/// The poker performs no input of its own; delivery is somebody else's job.
NoInputPerformed,
}

impl std::fmt::Display for DeferralReason {
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::ComposerChanged { harness } => write!(
formatter,
"the {harness} composer holds other text (a draft or an unfinished turn); waiting rather than typing over it"
),
Self::ComposerUnproven { harness } => write!(
formatter,
"the {harness} composer was located but proved nothing about this screen (an active turn, a modal, or an unrecognised footer); waiting for it to settle"
),
Self::NoMaintainedComposer => formatter.write_str(
"no maintained harness could locate a composer on this pane; nothing will be delivered here until one can",
),
Self::NoInputPerformed => formatter.write_str("this poker performs no input"),
}
}
}

/// Whether a deferral is news, so a pane stuck in one verdict costs one log line rather than one
/// per retry. The first deferral after progress is news, a changed verdict is news, and the same
/// verdict repeating is the same fact.
#[derive(Debug, Default)]
pub struct DeferralJournal {
last: Option<DeferralReason>,
}

impl DeferralJournal {
pub fn observe(&mut self, current: Option<DeferralReason>) -> bool {
if self.last == current {
return false;
}
self.last = current;
current.is_some()
}
}

/// What one `flush_pending` chose not to do. Deliberately a plain struct rather than an
/// `Option`: callers that only want the flush keep compiling as statements.
#[derive(Debug, Default, Clone, Copy)]
pub struct FlushReport {
/// Set when the front notice performed no input this pass.
pub deferred: Option<DeferralReason>,
}

/// How DING delivers a poke and checks liveness, abstracted so the watch loop is testable without a
/// real `pty`.
pub trait Poker {
fn poke(&self, text: &str) -> anyhow::Result<PokeOutcome>;
fn retry_staged(&self, _text: &str) -> anyhow::Result<PokeOutcome> {
Ok(PokeOutcome::Deferred)
Ok(PokeOutcome::Deferred(DeferralReason::NoInputPerformed))
}
fn adopt_staged(&self, _candidates: &[String]) -> anyhow::Result<Option<String>> {
Ok(None)
Expand Down Expand Up @@ -569,7 +636,8 @@ fn observed_poke_with_window(
before_submit: &mut dyn FnMut() -> anyhow::Result<()>,
observation_window: Duration,
) -> anyhow::Result<PokeOutcome> {
match classify_composer(&peek()?, text) {
let (state, harness) = classify_located_composer(&peek()?, text);
match state {
ComposerState::ExactSafe => {
return submit_after_final_observation(
text,
Expand All @@ -583,7 +651,18 @@ fn observed_poke_with_window(
ComposerState::ExactBlocked => return Ok(PokeOutcome::Staged),
ComposerState::EmptySafe => {}
ComposerState::Changed | ComposerState::Ambiguous => {
return Ok(PokeOutcome::Deferred);
// Whether a harness was located is the difference between a wait and a coverage gap,
// so it decides the reason rather than being folded into one catch-all.
return Ok(PokeOutcome::Deferred(match (state, harness) {
(ComposerState::Changed, Some(harness)) => {
DeferralReason::ComposerChanged { harness }
}
// `Changed` cannot arise without a located harness — only a located composer can
// be read as holding other text — but the classifier owns that invariant, not
// this call site, so an unlocated pane is reported as exactly what was observed.
(_, Some(harness)) => DeferralReason::ComposerUnproven { harness },
(_, None) => DeferralReason::NoMaintainedComposer,
}));
}
}

Expand Down Expand Up @@ -969,6 +1048,7 @@ pub fn run_ding(
let mut logged_waiting = false;
let mut last_refresh: Option<Instant> = None;
let mut next_delivery_attempt: Option<Instant> = None;
let mut deferrals = DeferralJournal::default();

loop {
if stop.load(Ordering::SeqCst) {
Expand Down Expand Up @@ -1022,7 +1102,15 @@ pub fn run_ding(
}
}
}
flush_pending(context, status_path, &mut pending, poker);
let report = flush_pending(context, status_path, &mut pending, poker);
if deferrals.observe(report.deferred)
&& let Some(reason) = report.deferred
{
tracing::warn!(
"st2 ding: delivery deferred for '{}', no input performed: {reason}",
context.recipient
);
}
next_delivery_attempt = (startup_candidates.is_some() || !pending.is_empty())
.then(|| Instant::now() + DELIVERY_RETRY_BACKOFF);
}
Expand Down Expand Up @@ -1084,9 +1172,10 @@ fn flush_pending(
status_path: Option<&Path>,
pending: &mut VecDeque<PendingNotice>,
poker: &dyn Poker,
) {
) -> FlushReport {
let mut report = FlushReport::default();
if delivery_suppressed(status_path) {
return;
return report;
}

let mut resolver = None;
Expand Down Expand Up @@ -1115,7 +1204,7 @@ fn flush_pending(
notice.set_staged_text(Some(text));
break;
}
Ok(PokeOutcome::Deferred) if was_staged => {
Ok(PokeOutcome::Deferred(_)) if was_staged => {
// The exact owned payload disappeared or changed. Adopted startup text has the
// generic recovery notice behind it, while unread ordinary work may make one later
// fresh guarded attempt. Archived work is done.
Expand All @@ -1125,13 +1214,20 @@ fn flush_pending(
}
pending.pop_front();
}
Ok(PokeOutcome::Deferred) => break,
// The one outcome that delivers nothing and leaves nothing behind. Report it out so
// the watch loop can say so; an unreported break here is how an eleven-day fleet-wide
// delivery failure stayed invisible.
Ok(PokeOutcome::Deferred(reason)) => {
report.deferred = Some(reason);
break;
}
Err(error) => {
tracing::warn!("st2 ding: {error}");
break;
}
}
}
report
}

/// Set by SIGINT/SIGTERM so `st2 ding` exits cleanly when st2 tears the sidecar down.
Expand Down Expand Up @@ -1294,7 +1390,7 @@ mod tests {
fn poke(&self, text: &str) -> anyhow::Result<PokeOutcome> {
self.calls.lock().unwrap().push(text.to_string());
if self.defer.load(Ordering::SeqCst) {
return Ok(PokeOutcome::Deferred);
return Ok(PokeOutcome::Deferred(DeferralReason::NoMaintainedComposer));
}
let mut failures = self.failures.lock().unwrap();
if *failures > 0 {
Expand Down Expand Up @@ -3844,4 +3940,89 @@ Enter to select · ↑/↓ to navigate · Esc to cancel";
SessionLiveness::Dead
);
}

/// A deferral is the one outcome that both delivers nothing and leaves nothing behind, so it
/// has to carry why. The two causes want different responses: a human drafting in a harness we
/// understand clears on its own, a pane no harness can locate never will.
#[test]
fn a_deferral_names_the_cause_that_produced_it() {
let text = "[DING] ? cos: guarded [id:abc123]";
for (screen, expected) in [
(
human_codex_screen(),
DeferralReason::ComposerChanged { harness: "codex" },
),
// Located, but an unrecognised footer proves nothing about the screen. This is a wait,
// not a coverage gap, and must not be reported as an unlocatable pane.
(
idle_codex_screen_with_footer("Esc to interrupt"),
DeferralReason::ComposerUnproven { harness: "codex" },
),
(
"unrecognized renderer".to_string(),
DeferralReason::NoMaintainedComposer,
),
] {
let outcome = observed_poke_with_window(
text,
&mut || Ok(screen.clone()),
&mut || Ok(()),
&mut || Ok(()),
&mut || {},
&mut || Ok(()),
Duration::ZERO,
)
.unwrap();
assert_eq!(outcome, PokeOutcome::Deferred(expected));
}
}

/// The deferral has to reach the watch loop, which is the only place that knows the recipient
/// and can decide whether it is worth saying.
#[test]
fn flush_reports_a_deferral_outward() {
let catalog = tempfile::tempdir().unwrap();
let context = DingContext {
catalog_root: catalog.path(),
this_host: "h",
recipient: "h.recipient",
};
let notice = || {
VecDeque::from([PendingNotice::message(msg(
"1785070000000-abc123.md",
"h.sender",
Some("hello"),
))])
};

let poker = RecordingPoker::live();
poker.defer.store(true, Ordering::SeqCst);
assert_eq!(
flush_pending(context, None, &mut notice(), &poker).deferred,
Some(DeferralReason::NoMaintainedComposer)
);

// A delivery that lands has nothing to report.
let poker = RecordingPoker::live();
assert_eq!(
flush_pending(context, None, &mut notice(), &poker).deferred,
None
);
}

/// A pane stuck in one verdict costs one line, not one per retry — but a changed verdict, and
/// the first deferral after delivery resumed, are both news.
#[test]
fn only_a_changed_deferral_verdict_is_worth_reporting() {
let changed = DeferralReason::ComposerChanged { harness: "codex" };
let unknown = DeferralReason::NoMaintainedComposer;
let mut journal = DeferralJournal::default();

assert!(journal.observe(Some(changed)));
assert!(!journal.observe(Some(changed)));
assert!(journal.observe(Some(unknown)));
assert!(!journal.observe(Some(unknown)));
assert!(!journal.observe(None));
assert!(journal.observe(Some(unknown)));
}
}
Loading