Skip to content

Commit 93f77d2

Browse files
fiskusclaude
andcommitted
Make autosync visible
The background engine emitted nothing — no events, no fault reports — which made it the least observable part of the app and the only part that runs with no user present. Per-deployment counts existed but were manual-action counts only. **The seam already existed.** Every outcome the engine produces already funnels through `StatusReporter`, so telemetry is a decorating reporter rather than `track()` calls threaded into the tick: one place to keep in step instead of two, and the UI reporter keeps working untouched behind it. Emission is spawned, not awaited — the trait is sync, telemetry is not, and the engine must not wait on a network call to finish a tick. Three events, under their own names: - `autosync_published` — deliberately *not* `package_published`. Folding unattended work into a series already read as user actions would redefine it silently, which is the thing the name-continuity rule exists to prevent. - `autosync_paused`, carrying the reason *category* only. - `autosync_login_required` — the silent-failure signal: background sync stops and nothing asks the user anything. Two methods report nothing, on purpose. `report_status` fires per package per tick and carries a fingerprint precisely so consumers can discard repeats — a progress signal, not a countable act. A subscriber error becomes a **fault** rather than an event, because the vocabulary carries no error events and the filesystem watcher concerns no deployment; that path is only assertable because of the seam #823 added. **The host is required, not optional** — the tightening the host change left open, available here for free. The loop already declines to work on a package whose lineage has no origin, so that guard is the proof: every outcome it reports concerns a known deployment. `report_published` and `report_paused` now take `&Host`, with one call site each, no degradation anywhere. `PausedKind` maps the engine's reason to a category and drops its contents. The engine's own type carries conflicting file names, a role name and a free-text message for the UI banner; none of it crosses into the vocabulary. A test feeds all three through and asserts nothing leaked. Attribution is proven through the real tick, not just the type: `RecordingReporter` now records hosts, and the publish test asserts the reported host is the package's own origin. just lint 0, cargo fmt --check 0, 310 quilt-sync tests, workspace green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 323e951 commit 93f77d2

6 files changed

Lines changed: 308 additions & 17 deletions

File tree

quilt-sync/src-tauri/src/autopull/reporter.rs

Lines changed: 109 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
use std::fmt::Write;
2+
use std::sync::Arc;
23

34
use serde::Serialize;
45

56
use quilt_uri::Host;
67
use quilt_uri::Namespace;
7-
use tauri::Emitter;
8+
use tauri::{Emitter, Manager};
89

910
use crate::autopull::PausedReason;
1011
use crate::quilt;
12+
use crate::telemetry::Telemetry;
13+
use crate::telemetry::event::{
14+
AutosyncAuthEvent, AutosyncEvent, AutosyncPausedEvent, MixpanelEvent, PausedKind,
15+
};
1116
use crate::telemetry::prelude::*;
1217

1318
/// Event names. Kept in lockstep with the UI's `listen(...)` calls.
@@ -192,7 +197,10 @@ pub struct SubscriberErrorEvent {
192197
/// tests and a hypothetical headless daemon wire a logger.
193198
pub trait StatusReporter: Send + Sync + 'static {
194199
fn report_status(&self, namespace: &Namespace, event: PackageStatusEvent);
195-
fn report_paused(&self, namespace: &Namespace, reason: PausedReason);
200+
/// `host` is required rather than optional: the loop skips any package whose
201+
/// lineage has no origin before doing work, so an outcome it reports always
202+
/// concerns a known deployment.
203+
fn report_paused(&self, namespace: &Namespace, host: &Host, reason: PausedReason);
196204
fn report_login_required(&self, host: Option<&Host>);
197205
fn report_subscriber_error(&self, event: SubscriberErrorEvent) {
198206
warn!(
@@ -202,8 +210,8 @@ pub trait StatusReporter: Send + Sync + 'static {
202210
}
203211
/// Surface a successful autosync publish. Default implementation
204212
/// logs only; `TauriEventReporter` also emits `PUBLISHED_EVENT`.
205-
fn report_published(&self, namespace: &Namespace, message: &str) {
206-
info!("autosync: published namespace={namespace} message={message}");
213+
fn report_published(&self, namespace: &Namespace, host: &Host, message: &str) {
214+
info!("autosync: published namespace={namespace} host={host} message={message}");
207215
}
208216
}
209217

@@ -221,8 +229,8 @@ impl StatusReporter for LogReporter {
221229
);
222230
}
223231

224-
fn report_paused(&self, namespace: &Namespace, reason: PausedReason) {
225-
info!("autosync: paused namespace={namespace} reason={reason:?}");
232+
fn report_paused(&self, namespace: &Namespace, host: &Host, reason: PausedReason) {
233+
info!("autosync: paused namespace={namespace} host={host} reason={reason:?}");
226234
}
227235

228236
fn report_login_required(&self, host: Option<&Host>) {
@@ -236,6 +244,89 @@ impl StatusReporter for LogReporter {
236244

237245
/// Production reporter: emits typed events on the Tauri event bus and
238246
/// also logs so file-tail-style debugging still works.
247+
/// Wraps another reporter and, in passing, tells telemetry what the engine just
248+
/// did.
249+
///
250+
/// A decorator rather than calls threaded into the tick: the engine already
251+
/// funnels every outcome through [`StatusReporter`], so that trait *is* the seam,
252+
/// and adding a second one inside the loop would mean two places to keep in step.
253+
/// The inner reporter still does its job — this only observes.
254+
///
255+
/// **Not every method reports.** `report_status` fires per package per tick and
256+
/// carries a fingerprint precisely so consumers can discard repeats; it is a
257+
/// progress signal, not a countable act, and sending it would swamp the
258+
/// vocabulary. A subscriber error is reported as a *fault* rather than an event,
259+
/// because the analytics vocabulary carries no error events by design.
260+
///
261+
/// Emission is spawned rather than awaited, because the trait is synchronous and
262+
/// telemetry is not. That is the right shape regardless: the engine must not wait
263+
/// on a network call to finish its tick.
264+
pub struct TelemetryReporter {
265+
handle: tauri::AppHandle,
266+
inner: Arc<dyn StatusReporter>,
267+
}
268+
269+
impl TelemetryReporter {
270+
pub fn wrapping(inner: Arc<dyn StatusReporter>, handle: tauri::AppHandle) -> Self {
271+
Self { handle, inner }
272+
}
273+
274+
/// Hand `event` to telemetry without blocking the caller.
275+
///
276+
/// The `Telemetry` lives in Tauri state rather than being held here, so a
277+
/// reporter constructed before it is managed still works — the same reason
278+
/// [`TauriEventReporter`] holds a handle instead of a window.
279+
fn emit(&self, event: MixpanelEvent) {
280+
let handle = self.handle.clone();
281+
tauri::async_runtime::spawn(async move {
282+
handle.state::<Telemetry>().track(event).await;
283+
});
284+
}
285+
}
286+
287+
impl StatusReporter for TelemetryReporter {
288+
fn report_status(&self, namespace: &Namespace, event: PackageStatusEvent) {
289+
// Deliberately not reported — see the type's note on volume.
290+
self.inner.report_status(namespace, event);
291+
}
292+
293+
fn report_paused(&self, namespace: &Namespace, host: &Host, reason: PausedReason) {
294+
self.emit(MixpanelEvent::AutosyncPaused(AutosyncPausedEvent {
295+
host: host.clone(),
296+
reason: PausedKind::from(&reason),
297+
}));
298+
self.inner.report_paused(namespace, host, reason);
299+
}
300+
301+
fn report_login_required(&self, host: Option<&Host>) {
302+
self.emit(MixpanelEvent::AutosyncLoginRequired(AutosyncAuthEvent {
303+
host: host.cloned(),
304+
}));
305+
self.inner.report_login_required(host);
306+
}
307+
308+
fn report_subscriber_error(&self, event: SubscriberErrorEvent) {
309+
// A fault, not an event: the vocabulary has no error events, and the
310+
// filesystem watcher concerns no deployment, so there would be nothing to
311+
// attribute one to. The message is a constant so the reporter groups it as
312+
// one issue rather than one per path.
313+
let handle = self.handle.clone();
314+
tauri::async_runtime::spawn(async move {
315+
handle
316+
.state::<Telemetry>()
317+
.report_anomaly("Autosync filesystem watcher reported an error");
318+
});
319+
self.inner.report_subscriber_error(event);
320+
}
321+
322+
fn report_published(&self, namespace: &Namespace, host: &Host, message: &str) {
323+
self.emit(MixpanelEvent::AutosyncPublished(AutosyncEvent {
324+
host: host.clone(),
325+
}));
326+
self.inner.report_published(namespace, host, message);
327+
}
328+
}
329+
239330
pub struct TauriEventReporter {
240331
handle: tauri::AppHandle,
241332
}
@@ -257,8 +348,8 @@ impl StatusReporter for TauriEventReporter {
257348
}
258349
}
259350

260-
fn report_paused(&self, namespace: &Namespace, reason: PausedReason) {
261-
info!("autosync: paused namespace={namespace} reason={reason:?}");
351+
fn report_paused(&self, namespace: &Namespace, host: &Host, reason: PausedReason) {
352+
info!("autosync: paused namespace={namespace} host={host} reason={reason:?}");
262353
let payload = PausedEvent::from_reason(namespace, &reason);
263354
if let Err(err) = self.handle.emit(PAUSED_EVENT, &payload) {
264355
warn!("autosync: failed to emit {PAUSED_EVENT}: {err}");
@@ -290,8 +381,8 @@ impl StatusReporter for TauriEventReporter {
290381
}
291382
}
292383

293-
fn report_published(&self, namespace: &Namespace, message: &str) {
294-
info!("autosync: published namespace={namespace} message={message}");
384+
fn report_published(&self, namespace: &Namespace, host: &Host, message: &str) {
385+
info!("autosync: published namespace={namespace} host={host} message={message}");
295386
let payload = PublishedEvent {
296387
namespace: namespace.to_string(),
297388
message: message.to_string(),
@@ -522,6 +613,10 @@ pub(crate) mod test_support {
522613
pub logins: Mutex<Vec<Option<Host>>>,
523614
pub subscriber_errors: Mutex<Vec<SubscriberErrorEvent>>,
524615
pub published: Mutex<Vec<(Namespace, String)>>,
616+
/// Hosts seen on the outcomes that carry one, so a test can assert the
617+
/// engine attributed a report to the package's own deployment rather than
618+
/// merely compiling against a `&Host`.
619+
pub hosts: Mutex<Vec<Host>>,
525620
}
526621

527622
impl StatusReporter for RecordingReporter {
@@ -532,7 +627,8 @@ pub(crate) mod test_support {
532627
.push((namespace.clone(), event));
533628
}
534629

535-
fn report_paused(&self, namespace: &Namespace, reason: PausedReason) {
630+
fn report_paused(&self, namespace: &Namespace, host: &Host, reason: PausedReason) {
631+
self.hosts.lock().unwrap().push(host.clone());
536632
self.paused
537633
.lock()
538634
.unwrap()
@@ -547,7 +643,8 @@ pub(crate) mod test_support {
547643
self.subscriber_errors.lock().unwrap().push(event);
548644
}
549645

550-
fn report_published(&self, namespace: &Namespace, message: &str) {
646+
fn report_published(&self, namespace: &Namespace, host: &Host, message: &str) {
647+
self.hosts.lock().unwrap().push(host.clone());
551648
self.published
552649
.lock()
553650
.unwrap()

quilt-sync/src-tauri/src/autopull/tick.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,13 @@ pub(crate) async fn run_once(
464464
let Some(remote) = lineage.remote_uri.as_ref() else {
465465
continue;
466466
};
467-
if remote.origin.is_none() || remote.bucket.is_empty() {
467+
// The origin is what makes every outcome below attributable: the loop
468+
// declines to work on a package without one, so `report_*` can require a
469+
// host rather than accept an absent one.
470+
let Some(origin) = remote.origin.as_ref() else {
471+
continue;
472+
};
473+
if remote.bucket.is_empty() {
468474
continue;
469475
}
470476

@@ -490,7 +496,7 @@ pub(crate) async fn run_once(
490496
inner.backoff.write().await.remove(&namespace);
491497
inner.login_blocked.write().await.remove(&namespace);
492498
if let Some(message) = outcome.published.as_deref() {
493-
inner.reporter.report_published(&namespace, message);
499+
inner.reporter.report_published(&namespace, origin, message);
494500
}
495501
inner.reporter.report_status(
496502
&namespace,
@@ -522,13 +528,15 @@ pub(crate) async fn run_once(
522528
// the lookup is cached per host and the namespace is about
523529
// to be paused, so a denied host costs at most one `/me`
524530
// per role switch — not one per tick.
525-
let reason = name_denied_role(model, roles, remote.origin.as_ref(), reason).await;
531+
let reason = name_denied_role(model, roles, Some(origin), reason).await;
526532
inner
527533
.paused
528534
.write()
529535
.await
530536
.insert(namespace.clone(), reason.clone());
531-
inner.reporter.report_paused(&namespace, reason.clone());
537+
inner
538+
.reporter
539+
.report_paused(&namespace, origin, reason.clone());
532540
// Heuristic status from the refusal reason — flow::pull /
533541
// flow::publish don't expose the post-attempt state
534542
// directly. The string `"error"` is **reserved** for "we

quilt-sync/src-tauri/src/autopull/tick/tests/publish.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,14 @@ async fn run_once_publishes_on_changes() -> Result<(), Error> {
156156
// Default message_template is None → falls back to summary.
157157
assert_eq!(published[0].1, "Add file.txt");
158158
}
159+
{
160+
// Attribution, not just compilation: the host reported is the package's
161+
// own origin, which is what makes a per-deployment autosync count mean
162+
// anything.
163+
let hosts = reporter.hosts.lock().unwrap();
164+
assert_eq!(hosts.len(), 1);
165+
assert_eq!(hosts[0].to_string(), "catalog.dev");
166+
}
159167
{
160168
let statuses = reporter.statuses.lock().unwrap();
161169
assert_eq!(statuses.len(), 1);

quilt-sync/src-tauri/src/main.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,14 @@ fn main() {
131131
app.manage(oauth::OAuthState::default());
132132
// The watcher reads `Model` via `app_handle.state::<Model>()`
133133
// so it can spawn after `Model` is registered above.
134+
// Telemetry wraps the UI reporter rather than replacing it: the
135+
// engine's outcomes reach the window exactly as before, and telemetry
136+
// observes them on the way past.
134137
let reporter: Arc<dyn StatusReporter> =
135-
Arc::new(TauriEventReporter::new(app.handle().clone()));
138+
Arc::new(autopull::reporter::TelemetryReporter::wrapping(
139+
Arc::new(TauriEventReporter::new(app.handle().clone())),
140+
app.handle().clone(),
141+
));
136142
let (watcher, status_rx) = Watcher::spawn(
137143
app.handle().clone(),
138144
autosync_settings.clone(),

quilt-sync/src-tauri/src/telemetry/event.rs

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,75 @@ pub struct LoginEvent {
6969
pub flow: LoginFlow,
7070
}
7171

72+
/// Something autosync did on its own, against a deployment it is **known** to
73+
/// have.
74+
///
75+
/// The host is required here where a manual package operation's is optional, and
76+
/// the loop is why: it skips any package whose lineage carries no origin before
77+
/// doing any work, so every outcome it reports is about a package with a remote.
78+
/// That is the proof a required host needs — the tightening the previous change
79+
/// left open for the manual paths, available here for free.
80+
#[derive(Debug, Clone, Serialize)]
81+
pub struct AutosyncEvent {
82+
pub host: Host,
83+
}
84+
85+
/// Autosync stopping on a package, and the kind of thing that stopped it.
86+
#[derive(Debug, Clone, Serialize)]
87+
pub struct AutosyncPausedEvent {
88+
pub host: Host,
89+
pub reason: PausedKind,
90+
}
91+
92+
/// Autosync stopping because the session expired.
93+
///
94+
/// The one autosync payload whose host is optional, and not for the usual
95+
/// reason: the refusal itself carries the host only when the failing call knew
96+
/// which one it was talking to, so an absent host here means the engine could not
97+
/// name the deployment, not that there wasn't one.
98+
#[derive(Debug, Clone, Serialize)]
99+
pub struct AutosyncAuthEvent {
100+
pub host: Option<Host>,
101+
}
102+
103+
/// Why autosync paused, coarsely — the variant, never its contents.
104+
///
105+
/// The engine's own reason type carries a conflicting-file list, a role name and
106+
/// a free-text message for the UI banner. None of that crosses into telemetry:
107+
/// the vocabulary admits no free text, and a file name is exactly the kind of
108+
/// detail [the module's rule](self) exists to keep out. The category is what a
109+
/// report can act on anyway — "how often does a role denial stop background
110+
/// sync" does not need to know which file.
111+
#[derive(Debug, Clone, Copy, Serialize)]
112+
#[serde(rename_all = "snake_case")]
113+
pub enum PausedKind {
114+
PendingChanges,
115+
PendingCommit,
116+
Diverged,
117+
PullConflict,
118+
RoleDenied,
119+
/// The engine's own catch-all for non-transient errors it has not
120+
/// enumerated. Coarse by inheritance rather than by choice: sharpening it
121+
/// means sharpening `PausedReason` upstream first.
122+
Other,
123+
}
124+
125+
impl From<&crate::autopull::PausedReason> for PausedKind {
126+
fn from(reason: &crate::autopull::PausedReason) -> Self {
127+
use crate::autopull::PausedReason as R;
128+
// Exhaustive rather than wildcarded, so a new pause reason cannot be
129+
// added without deciding how a report should see it.
130+
match reason {
131+
R::PendingChanges => Self::PendingChanges,
132+
R::PendingCommit => Self::PendingCommit,
133+
R::Diverged => Self::Diverged,
134+
R::PullConflict(_) => Self::PullConflict,
135+
R::RoleDenied { .. } => Self::RoleDenied,
136+
R::Other(_) => Self::Other,
137+
}
138+
}
139+
}
140+
72141
impl RemotePackageEvent {
73142
pub fn for_uri(uri: Option<&S3PackageUri>) -> Self {
74143
Self {
@@ -149,6 +218,18 @@ pub enum MixpanelEvent {
149218
FileRevealed(PackageFileEvent),
150219
DefaultApplicationOpened(PackageFileEvent),
151220

221+
// ── autosync: the engine acting with no user present ──
222+
/// A publish the loop completed on its own. Deliberately *not*
223+
/// `package_published`: folding unattended work into a series read as
224+
/// user actions would redefine it silently, which is the thing the
225+
/// name-continuity rule exists to prevent.
226+
AutosyncPublished(AutosyncEvent),
227+
/// The loop stopped on a package, with the category that stopped it.
228+
AutosyncPaused(AutosyncPausedEvent),
229+
/// The loop stopped because the session expired — the silent-failure signal:
230+
/// background sync ceases working and nothing asked the user anything.
231+
AutosyncLoginRequired(AutosyncAuthEvent),
232+
152233
// ── auth: always names the deployment it acts on ──
153234
UserLoggedIn(LoginEvent),
154235
RoleSwitched(AuthEvent),
@@ -181,6 +262,10 @@ impl MixpanelEvent {
181262
| Self::FileRevealed(e)
182263
| Self::DefaultApplicationOpened(e) => e.host.as_ref(),
183264

265+
Self::AutosyncPublished(e) => Some(&e.host),
266+
Self::AutosyncPaused(e) => Some(&e.host),
267+
Self::AutosyncLoginRequired(e) => e.host.as_ref(),
268+
184269
Self::RoleSwitched(e) | Self::OAuthLoginInitiated(e) | Self::AuthErased(e) => {
185270
Some(&e.host)
186271
}

0 commit comments

Comments
 (0)