fix(ding): say why a delivery was deferred instead of breaking silently

[schickling-assistant]

Problem

flush_pending had exactly this:

Ok(PokeOutcome::Deferred) => break,

No log line, no state change, nothing written anywhere. classify_composer returns Deferred for Changed | Ambiguous, so any pane the classifier cannot positively type as empty-and-safe got zero delivery and zero diagnostic: no text on the pane, no line in the log, nothing in the inbox to show a delivery had been attempted and abandoned.

A deferral is the one outcome that both delivers nothing and leaves nothing behind. It was also the one outcome that said nothing, which makes it the only failure mode that cannot be noticed. In one observed deployment that silence covered an eleven-day delivery failure. Two supporting numbers from the same observation window: 913 delivery failures were recorded across the ding logs of a fleet of agent sessions (768 timed out after 2.0s, 145 staged retry observation failed) — those at least produced a line; the deferrals produced none. Separately, a session whose pane no maintained harness recognised received a message and produced no pane text and no log line at all over three minutes.

This is not a timeout problem, and this PR deliberately does not touch PTY_COMMAND_TIMEOUT. Two sessions with 110+ timeouts each had empty inboxes, so timeouts alone do not produce backlog. The defect is the silence, not the duration.

Goal

A delivery that performs no input says so, and says why — cheaply enough that it is safe to leave on, and specifically enough that an operator can tell a wait apart from a coverage gap.

Preview of the effect

Before, a session whose pane could not be classified produced no output at all — the loop broke and retried every 15 s, forever, silently. After, the first deferral prints one line naming the recipient and the cause:

WARN st2 ding: delivery deferred for 'h.recipient', no input performed: the codex composer
  holds other text (a draft or an unfinished turn); waiting rather than typing over it

WARN st2 ding: delivery deferred for 'h.recipient', no input performed: the claude composer
  was located but proved nothing about this screen (an active turn, a modal, or an
  unrecognised footer); waiting for it to settle

WARN st2 ding: delivery deferred for 'h.recipient', no input performed: no maintained harness
  could locate a composer on this pane; nothing will be delivered here until one can

The three read differently on purpose. The first two are waits — someone is typing, or a turn is in flight — and clear on their own. The third is a coverage gap: nothing about that pane is understood, and it will never clear without a harness change. That is the line worth paging on.

Volume: one line per verdict, not one per retry. A pane stuck in one state costs a single line no matter how long it stays stuck; a changed verdict, or the first deferral after delivery resumed, prints again. No pane text is ever logged.

Decisions

PokeOutcome::Deferred carries the reason. "Deferred" with no reason is precisely the defect, so the outcome type is where the reason belongs rather than a side channel.

Four reasons, because they want different responses. ComposerChanged { harness } is a human drafting in a harness we understand. ComposerUnproven { harness } is a located composer that proved nothing about this screen — an active turn, a modal, an unrecognised footer. Both are waits. NoMaintainedComposer is a pane no maintained harness could locate at all, which never clears on its own. NoInputPerformed covers the trait default. Folding the first three together would throw away the wait-versus-gap distinction that makes the line worth reading.

No pane text is logged, ever. Changed means a human's unsent draft may be on that pane. The diagnostic is the verdict plus the harness name, which is enough to separate the causes without putting someone's text in a log.

The harness name comes from a new classify_located_composer. The router already picked a harness and then discarded it; that name is the whole difference between a wait and a gap. Harness gains a name(), and classify_composer delegates to the new function, so every existing caller is untouched.

flush_pending reports outward rather than logging inline. It has no recipient and no cross-tick state; the watch loop has both. FlushReport is a plain struct rather than an Option on purpose, so the existing statement-position callers compile unchanged.

Log the edge, not the level. Delivery retries at DELIVERY_RETRY_BACKOFF (15 s), so a permanently unrecognised pane would otherwise emit roughly 5,760 identical lines a day. DeferralJournal reports only on change. Observation stays fail-open and off the delivery path: no I/O, no allocation, one comparison.

Temporarily based on another branch — see Concerns.

Verification

$ cargo test --lib ding::
test result: ok. 73 passed; 0 failed; 0 ignored; 0 measured; 483 filtered out

$ cargo test --lib
test result: ok. 556 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

CI: run 33272028700 — check-aarch64-darwin pass 5m27s, check-x86_64-linux running at time of writing (it passed at 57m03s on the identical tree in the previous run).

Three new tests, each checked by inverting it and confirming it fails:

• a_deferral_names_the_cause_that_produced_it — a human codex draft yields ComposerChanged { harness: "codex" }; a located codex composer under a non-idle footer yields ComposerUnproven { harness: "codex" }; an unrecognised renderer yields NoMaintainedComposer. Inverting the middle row fails with left: ComposerUnproven { harness: "codex" }, which is the mislabel this PR removes.
• flush_reports_a_deferral_outward — a deferring poker sets report.deferred; a delivering one leaves it None.
• only_a_changed_deferral_verdict_is_worth_reporting — the second consecutive identical deferral reports nothing; a changed verdict, and the first deferral after delivery resumed, both do. Removing the dedupe fails this.

Complexity

One enum with four variants, one Display impl, a one-field journal struct with a single observe method, and one plain report struct. PokeOutcome::Deferred gains a payload, touching six call sites. Harness gains a name(), implemented once per harness as a string literal.

The journal is the only piece that could be argued away, and the argument against removing it is quantitative: without it a stuck pane emits ~5,760 lines a day, which would get the logging turned off, which returns the system to the silence this PR exists to remove.

Concerns

• The reason set is only as good as the classifier. If a harness returns Ambiguous for a case that is really a wait, the operator sees "coverage gap" for something that would clear on its own. The distinction is now representable; keeping it accurate is ongoing work on the harness adapters.
• NoInputPerformed is reachable from the Poker trait default, so a poker that legitimately performs no input will log on its first deferral. Cheap, and arguably correct, but it is noise for a caller that never intended to deliver.
• A deferral in the was_staged arm is still not reported. That arm relinquishes ownership and leaves the notice pending for a fresh attempt, so it is a state transition rather than a silent no-op. Scoping this PR to the unreported break at the heart of the defect kept the change reviewable; that arm is worth a second look.

Why this PR is temporarily based on another branch

Base: schickling-assistant/2026-08-29-resync-darwin-watch-purge (#383), not main. Temporary, and unrelated to this change.

check-aarch64-darwin was failing here on #368 — run::tests::compile_invalid_seat_does_not_block_existing_live_resync_watch, which fails on main and on unrelated PRs and touches nothing this PR changes. It reproduced twice on this branch (most recently job 99124668684, same assertion at src/run.rs:4484), each time with 538 passed / 1 failed and nothing from ding:: among the failures. Linux was green throughout.

#383 fixes #368: a lost-event bug in notify's FSEvents backend, where every watch/unwatch restarts the shared stream at kFSEventStreamEventIdSinceNow and discards events already queued for directories that never left the watch set. Linux inotify keeps its queue, which is why only darwin saw it.

Stacked via gh stack link (stack #387): base ref only — no rebase, no force-push, and #383's branch was not touched. The two changes are disjoint (src/ding/* here; src/resync.rs and the resync spec there) and git merge-tree reports a clean merge. Verified that CI genuinely carries the fix rather than trusting the base ref: both e2d4fc98 (#383's head) and this branch's head are ancestors of refs/pull/375/merge.

This PR returns to main as its base once #383 merges. It carries no dependency on #383 beyond the CI baseline.

Friction & bottlenecks

• Changing a PR's base does not re-run CI. GitHub emits pull_request with action edited, and a bare on: pull_request workflow runs only for opened/synchronize/reopened. Verified rather than assumed: after the base change, the branch still listed only the pre-restack runs. Without a trigger the PR would have sat on a stale red result indefinitely. Cost one empty commit (chore: re-run CI against the new stacked base) to produce a synchronize; it can be squashed when the base returns to main. Worth knowing for anyone else stacking to escape a red baseline.
• cargo test (all targets) intermittently fails codex_app_server::tests::runtime_owner_lock_is_nonblocking_and_released_on_close on lock contention. Reproduces on a branch touching neither module, passes when the lib suite runs alone. Unrelated to this change; logged, not tracked.

Follow-ups

• The was_staged deferral arm noted under Concerns.
• Reduce the number of panes that reach NoMaintainedComposer at all — every one of those is a harness the adapters do not cover. This PR makes them countable for the first time, which is the prerequisite.

References

Refs #368 — the darwin failure this PR was blocked behind.
Depends on #383 (stack #387) for the CI baseline only; base returns to main on merge.

Posted on behalf of @schickling

field	value
agent_identity	dev3.direct.claude.paqjmjfq
session	dev3.paqjmjfq
agent_persona	generalist
agent_supervisor	unavailable
agent_tool	Claude Code
agent_tool_version	2.1.250
agent_runtime	Claude Code 2.1.250
tooling_profile	dotfiles@a1a5f89

[schickling-assistant]

The check-aarch64-darwin failure here is pre-existing on darwin and unrelated to this change.

Failing test:

run::tests::compile_invalid_seat_does_not_block_existing_live_resync_watch ... FAILED
thread ... panicked at src/run.rs:4484:14:
correcting another declaration must not reseed and hide the live transition
test result: FAILED. 538 passed; 1 failed

The same test, with the same assertion message, also fails on #366 — a different branch and an
unrelated change:

run::tests::compile_invalid_seat_does_not_block_existing_live_resync_watch ... FAILED
panicked at src/run.rs:4100
correcting another declaration must not reseed and hide the live transition

(The line numbers differ only because the two branches differ.) check-x86_64-linux passes on
both. This PR touches ding delivery logging; the failing test covers resync-watch reseeding,
which this change does not go near.

Flagging rather than fixing: the darwin resync-watch failure looks like it wants its own
investigation, and bisecting it from a PR branch would be a coin flip.

Posted on behalf of @schickling

field	value
agent_identity	dev3.direct.claude.paqjmjfq
session	dev3.paqjmjfq
agent_persona	generalist
agent_supervisor	unavailable
agent_tool	Claude Code
agent_tool_version	2.1.250
agent_runtime	Claude Code 2.1.250
tooling_profile	dotfiles@a1a5f89

[schickling-assistant]

Tracked already: #368 — "main is red on check-aarch64-darwin: compile_invalid_seat_does_not_block_existing_live_resync_watch times out waiting for the resync event".

So this is not merely pre-existing on other branches, it is red on main. Nothing for this PR to fix.

Posted on behalf of @schickling

field	value
agent_identity	dev3.direct.claude.paqjmjfq
session	dev3.paqjmjfq
agent_persona	generalist
agent_supervisor	unavailable
agent_tool	Claude Code
agent_tool_version	2.1.250
agent_runtime	Claude Code 2.1.250
tooling_profile	dotfiles@a1a5f89

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-29T13:49:27.017957Z	ffd8aa8	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ffd8aa8ec5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".