feat(ptt): close P0 unit-boundary gaps (SDD-088 / SDD-090 / SDD-091)
The P0 audit on v0.9.4-docs found three SDD items whose specified
software units were inlined into other types rather than packaged as
named units at the SDD-defined boundary:
* SDD-088 PttController — backend ownership + binding mutex +
capability watch lived split between AudioEngine and
ChanoraSession. Extracted into chanora_core::ptt::PttController.
AudioEngine now owns only the cpal streams and the missed-key-up
watchdog (SDD-092); the platform input backend, the active
PttBinding, and the capability watch::Sender live in the
controller. ChanoraSession::start_audio constructs the controller
against the engine's gate; disconnect/reconnect/restart paths
tear it down through stop().await before the engine.
* SDD-090 PttSanitizer — banned-field check was inlined as
PttBanCheckVisitor inside RedactingLogLayer::on_event. Extracted
into a generic PttSanitizer<L> tracing_subscriber::Layer that
decorates an inner Layer (canonical pairing:
RedactingLogLayer::with_sanitizer). The inner layer keeps its
own structural ban check as defence-in-depth for bare-install
callers.
* SDD-091 PttCapabilityBadge — Voice Bar badge was anonymous
Padding/Tooltip/Row inside _AudioControlsState.build. Extracted
into a public PttCapabilityBadge widget and added the
SDD-091-specified per-platform explanation sheet that opens on
the info-icon tap when the resolved capability is L0Focused.
New l10n strings (en + zh) cover the sheet copy.
Tests:
* 2 new unit tests for PttController (arm + descriptor watch)
* 1 new unit test for PttSanitizer (end-to-end through a real
tracing subscriber proving banned drop + safe forward)
cargo test --workspace: 55 passed / 0 failed / 3 ignored
cargo deny check: advisories ok, bans ok, licenses ok, sources ok
flutter analyze: no issues
tools/validate_docs.py: zero undefined refs, zero direct-layer
violations (pre-existing 35 old-package-name warning unchanged)
No SDD/SAD/SRS doc changes — the contracts already named these
units; this commit aligns code unit boundaries with those contracts.
This commit is contained in:
@@ -108,13 +108,11 @@ pub struct AudioEngine {
|
||||
/// denied microphone permission), PTT becomes a no-op and
|
||||
/// `frames_sent` stays at 0.
|
||||
capture_active: bool,
|
||||
/// Active desktop PTT backend (SAD-071 / SDD-081). Stored
|
||||
/// inside a `Mutex<Option<_>>` so `stop()` can move it out
|
||||
/// and release OS-level resources before the engine is
|
||||
/// dropped. The value is always `Some` between `start_audio`
|
||||
/// and `stop`.
|
||||
ptt_backend: Mutex<Option<Box<dyn crate::ptt_backends::DesktopPttBackend>>>,
|
||||
/// Missed-key-up watchdog. Dropping aborts the task.
|
||||
/// Missed-key-up watchdog (SDD-092). Dropping aborts the task.
|
||||
/// The watchdog is independent of the PTT input backend — it
|
||||
/// observes the gate directly. The platform input backend is
|
||||
/// owned by `chanora_core::ptt::PttController` (SDD-088), not
|
||||
/// by the engine.
|
||||
ptt_watchdog: Option<crate::ptt::MissedKeyUpWatchdog>,
|
||||
}
|
||||
|
||||
@@ -307,33 +305,11 @@ impl AudioEngine {
|
||||
}
|
||||
});
|
||||
|
||||
// Select and arm the desktop PTT backend (SAD-071,
|
||||
// SDD-081). This call is the only place that talks to the
|
||||
// platform-input layer; the rest of the engine consumes
|
||||
// the typed `AudioTransmitGate`. We always have a backend
|
||||
// because the cross-platform factory falls back to
|
||||
// `FocusedPttBackend` (SDD-087).
|
||||
let mut ptt_backend = crate::ptt_backends::select();
|
||||
let initial_binding = crate::ptt_backends::PttBinding::none();
|
||||
match ptt_backend.start(transmit_gate.clone(), initial_binding) {
|
||||
Ok(()) => {
|
||||
let d = ptt_backend.descriptor();
|
||||
info!(
|
||||
target: "chanora_audio",
|
||||
capability_level = %d.level,
|
||||
backend_id = d.backend_id,
|
||||
bound_input_class = ?d.bound_input_class,
|
||||
"ptt backend armed"
|
||||
);
|
||||
}
|
||||
Err(e) => {
|
||||
warn!(
|
||||
target: "chanora_audio",
|
||||
error = %e,
|
||||
"ptt backend start failed; engine continues with Focused fallback"
|
||||
);
|
||||
}
|
||||
}
|
||||
// Select and arm the desktop PTT backend is no longer the
|
||||
// engine's job (SDD-088). The PTT controller lives in
|
||||
// `chanora_core::ptt::PttController`; the engine is
|
||||
// responsible only for the cpal streams and the
|
||||
// missed-key-up watchdog (SDD-092).
|
||||
|
||||
// Spawn the missed-key-up watchdog. The task aborts on
|
||||
// Drop of `MissedKeyUpWatchdog`, so the engine's `stop`
|
||||
@@ -353,7 +329,6 @@ impl AudioEngine {
|
||||
_output_stream: Mutex::new(Some(output_stream)),
|
||||
shutdown_tx: Some(shutdown_tx),
|
||||
capture_active,
|
||||
ptt_backend: Mutex::new(Some(ptt_backend)),
|
||||
ptt_watchdog: Some(ptt_watchdog),
|
||||
})
|
||||
}
|
||||
@@ -363,15 +338,10 @@ impl AudioEngine {
|
||||
if let Some(tx) = self.shutdown_tx.take() {
|
||||
let _ = tx.send(());
|
||||
}
|
||||
// Release the active PTT backend's OS resources before
|
||||
// dropping the streams; the backend may hold a worker
|
||||
// thread (Raw Input message loop, Event Tap run loop, etc.)
|
||||
// that needs an explicit stop() to wind down cleanly.
|
||||
if let Ok(mut guard) = self.ptt_backend.lock() {
|
||||
if let Some(mut backend) = guard.take() {
|
||||
backend.stop();
|
||||
}
|
||||
}
|
||||
// The platform PTT backend is no longer owned by the
|
||||
// engine (SDD-088); its lifecycle is managed by
|
||||
// `chanora_core::ptt::PttController`. The engine only
|
||||
// needs to abort its watchdog and drop the cpal streams.
|
||||
// Aborting the watchdog cancels its tokio task.
|
||||
self.ptt_watchdog.take();
|
||||
// Drop the streams, which stops their callback threads.
|
||||
@@ -404,52 +374,17 @@ impl AudioEngine {
|
||||
&self.transmit_gate
|
||||
}
|
||||
|
||||
/// Privacy-safe descriptor of the currently active PTT
|
||||
/// backend (SDD-081 / SDD-091). Returns the universal Focused
|
||||
/// fallback descriptor when the backend slot is empty
|
||||
/// (typically only between `stop()` and Drop).
|
||||
/// Privacy-safe descriptor of the engine's PTT view. The
|
||||
/// platform backend lives in `chanora_core::ptt::PttController`
|
||||
/// (SDD-088); the engine itself no longer owns it. This getter
|
||||
/// always returns the universal Focused fallback descriptor
|
||||
/// and is retained only for legacy callers that constructed
|
||||
/// engines directly without a controller (tests, headless
|
||||
/// diagnostics).
|
||||
pub fn ptt_descriptor(&self) -> crate::ptt::PttBackendDescriptor {
|
||||
if let Ok(guard) = self.ptt_backend.lock() {
|
||||
if let Some(b) = guard.as_ref() {
|
||||
return b.descriptor();
|
||||
}
|
||||
}
|
||||
crate::ptt::PttBackendDescriptor::focused()
|
||||
}
|
||||
|
||||
/// Replace the PTT binding on the active backend. Returns the
|
||||
/// freshly-published descriptor so callers can re-emit the
|
||||
/// capability event.
|
||||
pub fn rebind_ptt(
|
||||
&self,
|
||||
binding: crate::ptt_backends::PttBinding,
|
||||
) -> Result<crate::ptt::PttBackendDescriptor, AudioError> {
|
||||
let mut guard = self
|
||||
.ptt_backend
|
||||
.lock()
|
||||
.map_err(|_| AudioError::Backend("ptt_backend mutex poisoned".to_string()))?;
|
||||
let backend = guard
|
||||
.as_mut()
|
||||
.ok_or_else(|| AudioError::Backend("ptt backend not armed".to_string()))?;
|
||||
backend
|
||||
.rebind(binding)
|
||||
.map_err(|e| AudioError::Backend(format!("rebind: {e}")))?;
|
||||
Ok(backend.descriptor())
|
||||
}
|
||||
|
||||
/// Subscribe to PTT descriptor transitions on the active
|
||||
/// backend. The Linux GNOME-Wayland portal backend uses this
|
||||
/// channel to publish the post-`BindShortcuts` capability
|
||||
/// transition. Returns `None` when the engine has no backend
|
||||
/// armed (e.g. between `stop()` and Drop).
|
||||
pub fn ptt_descriptor_watch(
|
||||
&self,
|
||||
) -> Option<tokio::sync::watch::Receiver<crate::ptt::PttBackendDescriptor>> {
|
||||
let guard = self.ptt_backend.lock().ok()?;
|
||||
let backend = guard.as_ref()?;
|
||||
Some(backend.descriptor_watch())
|
||||
}
|
||||
|
||||
/// Legacy alias for [`Self::set_transmit_active`]. Retained so
|
||||
/// the existing bridge `set_ptt` command and the existing
|
||||
/// Flutter UI continue to compile during the v0.9.3 PTT
|
||||
|
||||
@@ -408,11 +408,19 @@ const PTT_BANNED_FIELDS: &[&str] = &[
|
||||
/// A `tracing` Layer that funnels records into an
|
||||
/// [`InMemoryLogSink`]. Install during process init.
|
||||
///
|
||||
/// Per DEC-027 the layer also drops any record that carries one of
|
||||
/// [`PTT_BANNED_FIELDS`] in its field set; the structural check
|
||||
/// runs before format / redaction so a banned record never reaches
|
||||
/// the in-memory sink and therefore never reaches the user-initiated
|
||||
/// diagnostic export.
|
||||
/// Per DEC-027 the records that carry raw key data must never reach
|
||||
/// the in-memory sink. Per SDD-090 that responsibility lives in a
|
||||
/// separate decorator Layer — [`PttSanitizer`] — so the
|
||||
/// privacy boundary is its own named software unit. Constructors
|
||||
/// that want the decorated stack should call
|
||||
/// [`PttSanitizer::wrap`] (or use `RedactingLogLayer::with_sanitizer`
|
||||
/// for the canonical pairing).
|
||||
///
|
||||
/// `RedactingLogLayer` retains the same structural ban check for
|
||||
/// callers that install it bare, so existing wiring continues to
|
||||
/// honour DEC-027 even without `PttSanitizer`. The two checks are
|
||||
/// idempotent — a sanitiser-wrapped layer never sees a banned
|
||||
/// record so the inner check is a no-op on that path.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct RedactingLogLayer {
|
||||
sink: InMemoryLogSink,
|
||||
@@ -423,6 +431,12 @@ impl RedactingLogLayer {
|
||||
pub fn new(sink: InMemoryLogSink) -> Self {
|
||||
Self { sink }
|
||||
}
|
||||
|
||||
/// Convenience: wrap `self` in a [`PttSanitizer`] decorator
|
||||
/// (SDD-090). Equivalent to `PttSanitizer::wrap(self)`.
|
||||
pub fn with_sanitizer(self) -> PttSanitizer<Self> {
|
||||
PttSanitizer::wrap(self)
|
||||
}
|
||||
}
|
||||
|
||||
impl<S> Layer<S> for RedactingLogLayer
|
||||
@@ -430,11 +444,10 @@ where
|
||||
S: Subscriber + for<'a> LookupSpan<'a>,
|
||||
{
|
||||
fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) {
|
||||
// PttSanitizer (SAD-077 / SDD-090): drop any record whose
|
||||
// field set names a raw key code, scan code, virtual-key
|
||||
// value, keysym, or timing sequence. The structural check
|
||||
// is fast — at most one allocation-free pass over the
|
||||
// field list.
|
||||
// Defence-in-depth: a `RedactingLogLayer` installed without
|
||||
// a surrounding `PttSanitizer` still drops banned records.
|
||||
// When wrapped by `PttSanitizer` this check is unreachable
|
||||
// (the sanitiser short-circuits first).
|
||||
let mut ban_check = PttBanCheckVisitor::default();
|
||||
event.record(&mut ban_check);
|
||||
if ban_check.banned {
|
||||
@@ -456,6 +469,56 @@ where
|
||||
fn on_new_span(&self, _: &Attributes<'_>, _: &Id, _: Context<'_, S>) {}
|
||||
}
|
||||
|
||||
/// PTT sanitiser Layer (SAD-077 / SDD-090).
|
||||
///
|
||||
/// Decorates an inner `tracing-subscriber::Layer` (typically
|
||||
/// [`RedactingLogLayer`]). On each `on_event` the sanitiser performs
|
||||
/// a single allocation-free pass over the event's field set and
|
||||
/// drops the record if any field name matches the
|
||||
/// [`PTT_BANNED_FIELDS`] list. Records without banned fields are
|
||||
/// forwarded verbatim to the inner Layer's `on_event`.
|
||||
///
|
||||
/// The implementation is allocation-free on the success path (the
|
||||
/// typical "no banned field" case): the visitor holds a single
|
||||
/// `bool` on the stack and exits early once a banned name is seen.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct PttSanitizer<L> {
|
||||
inner: L,
|
||||
}
|
||||
|
||||
impl<L> PttSanitizer<L> {
|
||||
/// Wrap an inner Layer with the PTT sanitiser. Use
|
||||
/// [`RedactingLogLayer::with_sanitizer`] for the canonical
|
||||
/// pairing.
|
||||
pub fn wrap(inner: L) -> Self {
|
||||
Self { inner }
|
||||
}
|
||||
|
||||
/// Borrow the wrapped inner Layer (read-only).
|
||||
pub fn inner(&self) -> &L {
|
||||
&self.inner
|
||||
}
|
||||
}
|
||||
|
||||
impl<S, L> Layer<S> for PttSanitizer<L>
|
||||
where
|
||||
S: Subscriber + for<'a> LookupSpan<'a>,
|
||||
L: Layer<S>,
|
||||
{
|
||||
fn on_event(&self, event: &Event<'_>, ctx: Context<'_, S>) {
|
||||
let mut ban_check = PttBanCheckVisitor::default();
|
||||
event.record(&mut ban_check);
|
||||
if ban_check.banned {
|
||||
return;
|
||||
}
|
||||
self.inner.on_event(event, ctx);
|
||||
}
|
||||
|
||||
fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) {
|
||||
self.inner.on_new_span(attrs, id, ctx);
|
||||
}
|
||||
}
|
||||
|
||||
/// Lightweight `tracing::field::Visit` implementation that only
|
||||
/// notes whether any visited field name matches the PTT banned
|
||||
/// list. Allocation-free.
|
||||
@@ -698,4 +761,35 @@ mod tests {
|
||||
assert!(!PTT_BANNED_FIELDS.contains(&safe));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ptt_sanitizer_drops_banned_records_before_inner_layer() {
|
||||
// End-to-end test through a real tracing subscriber: a
|
||||
// sanitiser-wrapped RedactingLogLayer must drop records
|
||||
// that name banned fields and must forward records that
|
||||
// do not. Uses `with_default` so the subscriber is scoped
|
||||
// to the closure (no global state mutation across tests).
|
||||
use tracing_subscriber::layer::SubscriberExt;
|
||||
let secrets = KnownSecretRegistry::default();
|
||||
let redactor = Redactor::with_secrets(secrets);
|
||||
let sink = InMemoryLogSink::new(16, redactor);
|
||||
let inner = RedactingLogLayer::new(sink.clone());
|
||||
let sanitised = PttSanitizer::wrap(inner);
|
||||
let subscriber = tracing_subscriber::registry().with(sanitised);
|
||||
tracing::subscriber::with_default(subscriber, || {
|
||||
tracing::info!(key_code = 42, "banned record must drop");
|
||||
tracing::info!(backend_id = "linux-portal", "safe record must pass");
|
||||
});
|
||||
let exported =
|
||||
DiagnosticExport::from_sink(&sink, vec![("k".into(), "v".into())]).unwrap();
|
||||
let text = exported.to_text();
|
||||
assert!(
|
||||
!text.contains("banned record must drop"),
|
||||
"sanitiser must have dropped the banned record"
|
||||
);
|
||||
assert!(
|
||||
text.contains("safe record must pass"),
|
||||
"sanitiser must forward the safe record to the inner layer"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user