From 01d5c612c0fbfa6efb33c6c4bca2083a40fdeb25 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 1 Oct 2026 20:59:23 +1000 Subject: [PATCH 1/4] Contact email: withdrawal covers earlier reports; consent is a real true - A report with follow-up ticked carries this copy's contact id, and the inbox marks its permission withdrawn when a later choice from that copy no longer agrees to follow-up questions at that address. - The one-time prompt never appears in a visit that showed the privacy notice, even if the Frame connects just after it's dismissed. - Saving contact details isn't headset work: it can't hold up switching headsets or be refused after a switch. - Consent flags must be JSON true/false; "false" is no longer consent. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/privacy.md | 13 +++++-- tests/test_contact.py | 83 +++++++++++++++++++++++++++++++++++++++++ tests/test_telemetry.py | 4 +- ui/frame_contact.py | 20 +++++++++- ui/frame_report.py | 45 ++++++++++++++++++---- ui/index.html | 11 ++++-- ui/server.py | 4 +- 7 files changed, 160 insertions(+), 20 deletions(-) diff --git a/docs/privacy.md b/docs/privacy.md index 1ec1755..132f4bf 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -107,7 +107,9 @@ wrote, a short reference shown after sending, and the diagnostics below. Your email address goes with it only if you tick **The maintainer may contact me with follow-up questions** (the report then carries `contact_followup: true`); it's filled in from **Contact email** below when you've agreed there. It has its own random id, so it isn't linked to -your analytics events. +your analytics events. With that box ticked it also carries this copy's contact +id (`contact_id`, see below), so removing or changing the address later takes +back the follow-up permission given with the report too. With **Include diagnostics** ticked (the default), the report adds: @@ -141,8 +143,8 @@ are two separate choices, both off until you tick them: | **The maintainer may contact me with follow-up questions** | Questions about problem reports you send, mostly | You're asked once, in a bar at the top of the page, after the Frame has -connected for the first time, and never while or straight after the -first-run privacy notice is showing. **No thanks** hides it for good, and it isn't +connected for the first time, and never in the same visit as the first-run +privacy notice. **No thanks** hides it for good, and it isn't shown again even if you ignore it. **Contact email** in **Privacy & updates** is where you add, change or remove the address and either choice at any time. @@ -164,7 +166,10 @@ deletes it from this computer, including from the **Show what's been sent** log (in earlier contact events and problem reports), and sends a `withdraw` event with no address in it. The maintainer's list only uses the newest event from each copy, so from then on the address isn't listed for either choice. Unticking one choice -works the same way for that choice. If you're offline, the change waits on +works the same way for that choice. This also covers problem reports you sent +from this copy with follow-up questions ticked: if your newest choice no longer +agrees to follow-up questions at that address, the maintainer's inbox shows the +permission as withdrawn and leaves the address out. If you're offline, the change waits on this computer and is sent when PostHog can be reached. The earlier event stays in PostHog until its data retention removes it; to have it deleted sooner, ask the maintainer (for example in a problem report). diff --git a/tests/test_contact.py b/tests/test_contact.py index 8c2234a..e759f00 100644 --- a/tests/test_contact.py +++ b/tests/test_contact.py @@ -59,6 +59,16 @@ class Contact(Base): self.assertEqual(fc.load()["email"], "") self.assertEqual(self.got, []) + def test_only_a_real_true_counts_as_consent(self): + for wrong in ("false", "true", 1, 0, [], {}): + with self.assertRaisesRegex(ValueError, "true or false"): + fc.save({"email": "me@example.com", "updates": wrong, "followup": True}) + with self.assertRaisesRegex(ValueError, "true or false"): + fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": wrong}) + self.assertEqual((fc.load()["email"], self.got), ("", [])) + fc.save({"email": "me@example.com", "updates": True}) # left out is no + self.assertEqual((fc.load()["updates"], fc.load()["followup"]), (True, False)) + def test_each_choice_is_sent_privately_on_its_own(self): fc.save({"email": " me@example.com ", "updates": True}) fc.save({"email": "me@example.com", "updates": False, "followup": True}) @@ -248,6 +258,63 @@ class Contact(Base): with self.assertRaisesRegex(ValueError, "email address"): fr.send({**REPORT, "contact": "discord:me", "contactFollowup": True}) + def test_a_report_with_follow_up_carries_the_kept_contact_id(self): + fr.send({**REPORT, "contact": "me@example.com"}) + self.assertFalse(fc.FILE.exists()) # no follow-up, nothing kept or linked + fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": True}) + without, with_ = (e["properties"] for e in self.events()) + self.assertEqual(without["contact_id"], "") + self.assertEqual(with_["contact_id"], fc.load()["id"]) + self.assertNotEqual(with_["contact_id"], tm.settings()["id"]) # not the analytics id + fc.save({"email": "me@example.com", "updates": True}) + self.assertEqual(self.events()[-1]["distinct_id"], with_["contact_id"]) # same copy, same id + + def report_row(self, ts, contact="me@example.com", followup=True, cid="copy"): + return [ts, "AB12CD34", "bug", "RDP", "It never connects.", contact, "0.4.0", "Windows", "", "", followup, cid] + + def test_a_later_withdrawal_takes_back_a_reports_follow_up_permission(self): + reports = [self.report_row("2026-09-10T10:00:00Z"), # removed later + self.report_row("2026-09-12T10:00:00Z", cid="other"), # another copy + self.report_row("2026-09-20T10:00:00Z"), # after the withdrawal + self.report_row("2026-09-11T10:00:00Z", cid="", followup=True), # sent before contact_id + self.report_row("2026-09-15T10:00:00Z", cid="same-second")] + consents = [["copy", "me@example.com", True, 1, "2026-09-01T10:00:00Z"], + ["copy", "", False, 2, "2026-09-14T10:00:00Z"], + ["other", "me@example.com", True, 1, "2026-09-13T10:00:00Z"], # still agrees + ["same-second", "", False, 1, "2026-09-15T10:00:00Z"], ["short"]] + fr.mark_withdrawn(reports, consents) + self.assertEqual([r[10] for r in reports], ["withdrawn", True, True, True, "withdrawn"]) + + def test_changing_the_address_or_unticking_follow_up_takes_it_back_too(self): + reports = [self.report_row("2026-09-10T10:00:00Z", cid="moved"), + self.report_row("2026-09-10T10:00:00Z", cid="news-only"), + self.report_row("2026-09-10T10:00:00Z", contact="Me@Example.com", cid="case")] + consents = [["moved", "new@example.com", True, 1, "2026-09-11T10:00:00Z"], + ["news-only", "me@example.com", False, 1, "2026-09-11T10:00:00Z"], + ["case", "me@example.com", True, 1, "2026-09-11T10:00:00Z"], + # rev, not the clock, decides which later choice is newest + ["case", "", False, 2, "2026-09-11T09:59:00Z"]] + fr.mark_withdrawn(reports, consents) + self.assertEqual([r[10] for r in reports], ["withdrawn", "withdrawn", "withdrawn"]) + reports[2][10] = True + fr.mark_withdrawn(reports[2:], consents[2:3]) + self.assertIs(reports[2][10], True) # same address, any case, still agrees + + def test_the_inbox_shows_withdrawn_follow_up_without_the_address(self): + reports = [self.report_row("2026-09-10T10:00:00Z"), ["short"]] + consents = [["copy", "", False, 2, "2026-09-14T10:00:00Z"]] + with mock.patch.object(db, "_posthog_query", side_effect=[{"results": reports}, {"results": consents}]) as q, \ + mock.patch.object(sys, "argv", ["frame_report.py", "inbox", "30"]), \ + mock.patch("builtins.print") as out: + fr.main() + self.assertIn("event = 'contact_consent'", q.call_args_list[1].args[0]) + printed = " ".join(str(c.args[0]) for c in out.call_args_list if c.args) + self.assertIn("follow-up permission since withdrawn", printed) + self.assertNotIn("me@example.com", printed) + with mock.patch.object(db, "_posthog_query", return_value={"results": [self.report_row("x", followup=False)]}) as q: + fr.inbox() + self.assertEqual(q.call_count, 1) # nothing to reconcile, no second query + def test_contacts_lists_the_newest_choice_per_copy_by_consent(self): rows = [["a", "both@example.com", True, "true", "2026-09-01T10:00:00Z"], ["b", "news@example.com", "true", False, "2026-09-02T10:00:00Z"], @@ -272,6 +339,22 @@ class Contact(Base): self.assertIs(server.POST["/api/contact"], fc.save) self.assertIs(server.POST["/api/contact/prompt"], fc.prompt) + def test_saving_is_not_headset_work(self): + """A slow send mustn't hold up switching headsets, nor be refused after a switch.""" + import io + import server + seen = [] + for path in ("/api/contact", "/api/contact/prompt"): + h = server.Handler.__new__(server.Handler) + body = b'{"prompt": "shown"}' if path.endswith("prompt") else b'{"email": "me@example.com", "updates": true}' + h.path, h.rfile = path, io.BytesIO(body) + h.headers = {"Content-Length": str(len(body)), "X-Frame-Device": "a-headset-switched-away-from"} + h.local_request = lambda: True + h.send_json = lambda obj, status=200: seen.append((status, server._work[0])) + with mock.patch.object(fc, "_send_pending", side_effect=lambda block=True: seen.append(("send", server._work[0]))): + h.do_POST() + self.assertEqual(seen, [("send", 0), (200, 0), (200, 0)]) + # Run these once, in test_telemetry, not again through the import above. del Base, ReportProblem diff --git a/tests/test_telemetry.py b/tests/test_telemetry.py index 71c9f7e..3cd50cc 100644 --- a/tests/test_telemetry.py +++ b/tests/test_telemetry.py @@ -422,8 +422,8 @@ class ReportProblem(Base): def test_the_inbox_skips_malformed_reports(self): good = ["2026-09-28T09:50:00Z", "AB12CD34", "bug", "Live view stops", "It stops.", None, - "0.4.0", "macOS", "", "", None] - rows = [["2026-09-28T10:00:00Z", "X", "bug", "Hand-made", None, None, None, None, None, None, None], + "0.4.0", "macOS", "", "", None, None] + rows = [["2026-09-28T10:00:00Z", "X", "bug", "Hand-made", None, None, None, None, None, None, None, None], ["short"], good] with mock.patch.object(db, "_posthog_query", return_value={"results": rows}), \ mock.patch.object(sys, "argv", ["frame_report.py", "inbox"]), \ diff --git a/ui/frame_contact.py b/ui/frame_contact.py index c79e2c1..f64ca2c 100644 --- a/ui/frame_contact.py +++ b/ui/frame_contact.py @@ -71,6 +71,24 @@ def valid_email(email): return len(email) <= EMAIL_MAX and bool(EMAIL_RE.fullmatch(email)) +def flag(body, key): + """A consent choice: true only when it really is true (not "false" or 1), left out is no.""" + v = body.get(key) + if v is not None and not isinstance(v, bool): + raise ValueError(f'{key} must be true or false') + return v is True + + +def contact_id(): + """This copy's contact id, kept from now on. A report with follow-up consent carries it, so + removing the address later takes back the follow-up permission given with the report too.""" + with _lock: + s = load() + if not FILE.exists(): + _save(s) + return s['id'] + + def state(): """What the page shows. showPrompt: the one-time prompt hasn't been shown or answered yet, and the Frame has connected at least once (setup worked), so it never greets a new install.""" @@ -156,7 +174,7 @@ def save(body): """Set, change or remove the address and the two choices. An address needs at least one choice ticked; an empty address (or neither ticked) removes it and withdraws both.""" email = str(body.get('email') or '').strip() - updates, followup = bool(body.get('updates')), bool(body.get('followup')) + updates, followup = flag(body, 'updates'), flag(body, 'followup') if email and not valid_email(email): raise ValueError("that doesn't look like an email address") if email and not (updates or followup): diff --git a/ui/frame_report.py b/ui/frame_report.py index 25b887c..043f9b1 100644 --- a/ui/frame_report.py +++ b/ui/frame_report.py @@ -112,13 +112,15 @@ def send(body): """Send the report to PostHog. Returns {"id", "message"}; raises ReportError.""" kind = body.get('kind') if body.get('kind') in KINDS else 'bug' title, text, diag = compose(body) - followup = bool(body.get('contactFollowup')) + followup = frame_contact.flag(body, 'contactFollowup') contact = str(body.get('contact') or '').strip() if followup else '' if followup and not frame_contact.valid_email(contact): raise ValueError('add your email address for follow-up questions, or untick that box') ref = uuid.uuid4().hex[:8].upper() props = {**frame_telemetry.common(), 'kind': kind, 'title': title, 'message': text, 'contact': contact, 'contact_followup': followup, 'diagnostics': diag, + # Only with an address: so removing it later (Settings) also takes this permission back. + 'contact_id': frame_contact.contact_id() if followup else '', 'report_id': ref, 'steamos': str(frame.get('build') or '')[:120], 'level': 'report'} # Its own random id: a report can carry contact details, so it isn't linked to this copy's analytics. event = {'event': 'problem_report', 'distinct_id': str(uuid.uuid4()), 'uuid': str(uuid.uuid4()), @@ -143,15 +145,43 @@ class ReportError(RuntimeError): def inbox(days=30): """The maintainer's recent reports from PostHog, newest first (needs the personal API key - frame_compat_db.sync uses).""" + frame_compat_db.sync uses). Column 10 is whether the person may be asked follow-up + questions now: 'withdrawn' when a later choice from the same copy took it back.""" import frame_compat_db + days = int(days) res = frame_compat_db._posthog_query( "SELECT timestamp, properties.report_id, properties.kind, properties.title, properties.message, " "properties.contact, properties.app_version, properties.os, properties.steamos, properties.diagnostics, " - "properties.contact_followup " - f"FROM events WHERE event = 'problem_report' AND timestamp > now() - INTERVAL {int(days)} DAY " + "properties.contact_followup, properties.contact_id " + f"FROM events WHERE event = 'problem_report' AND timestamp > now() - INTERVAL {days} DAY " "ORDER BY timestamp DESC LIMIT 200") - return res.get('results') or [] + rows = [r for r in res.get('results') or [] if isinstance(r, list) and len(r) == 12] + if any(r[11] and _yes(r[10]) for r in rows): + later = frame_compat_db._posthog_query( + "SELECT distinct_id, properties.email, properties.followup, ifNull(toInt(properties.rev), 0), timestamp " + f"FROM events WHERE event = 'contact_consent' AND timestamp > now() - INTERVAL {days} DAY LIMIT 100000") + mark_withdrawn(rows, later.get('results') or []) + return rows + + +def mark_withdrawn(reports, consents): + """Mark reports whose follow-up permission was taken back: the newest contact choice from + the same copy made at or after the report (same second counts, so a withdrawal wins) no + longer agrees to follow-up questions at that address.""" + newest = {} + for c in consents: + if not isinstance(c, list) or len(c) != 5: + continue + cid, email, followup, rev, ts = c + newest.setdefault(str(cid), []).append(((int(rev or 0), str(ts or '')), str(email or ''), followup)) + for r in reports: + if not (r[11] and _yes(r[10])): + continue + after = [c for c in newest.get(str(r[11]), []) if c[0][1] >= str(r[0] or '')] + if after: + _, email, followup = max(after, key=lambda c: c[0]) + if not (_yes(followup) and email.strip().lower() == str(r[5] or '').strip().lower()): + r[10] = 'withdrawn' def _yes(v): @@ -204,14 +234,13 @@ def main(): if cmd != 'inbox': sys.exit(USAGE) for row in inbox(*(args[:1] or [30])): - if not isinstance(row, list) or len(row) != 11: - continue ts, ref, kind, title, text, contact, version, osname, steamos, diag = (str(v or '') for v in row[:10]) # Reports from before contact_followup existed only carried an address given for a reply. reply = contact and (row[10] is None or _yes(row[10])) print(f"== {ts[:16].replace('T', ' ')} {ref} [{kind}] {title}") print(f" {version} on {osname}, SteamOS {steamos or 'unknown'}" - f"{', may follow up at ' + contact if reply else ''}") + f"{', may follow up at ' + contact if reply else ''}" + f"{', follow-up permission since withdrawn' if row[10] == 'withdrawn' else ''}") print(' ' + text.replace('\n', '\n ')) if diag: print(' --- diagnostics\n ' + diag.replace('\n', '\n ')) diff --git a/ui/index.html b/ui/index.html index 972a867..586e14e 100644 --- a/ui/index.html +++ b/ui/index.html @@ -3963,6 +3963,7 @@ async function offerTest(m) { // ---- privacy: anonymous analytics levels (ui/frame_telemetry.py, docs/privacy.md) ---- const telemetry = { usage: false, compat: false, blocked: "not loaded" }; +let privacyNoticeShown = false; // this visit: then the contact prompt waits for another one function renderTelemetry(s) { Object.assign(telemetry, s); setRepHint(); @@ -3973,6 +3974,7 @@ function renderTelemetry(s) { : "Nothing sent yet."; const showNotice = !s.blocked && !s.noticeShown && s.usage; $("privacyNotice").hidden = !showNotice; + if (showNotice) privacyNoticeShown = true; if (showNotice) api("/api/telemetry", { noticeShown: true }).catch(() => {}); } async function loadTelemetry() { @@ -4015,13 +4017,14 @@ async function loadContact() { try { renderContact(await api("/api/contact")); } catch { return; } checkContactPrompt(); } -// One time only, only once the Frame has connected, and never on top of the privacy notice or -// straight after it (two asks in a row is nagging): checked at load and whenever the Frame connects. +// One time only, only once the Frame has connected, and never in a visit that showed the privacy +// notice (two asks in a row is nagging): checked at load and whenever the Frame connects. async function checkContactPrompt() { - if (!$("contactNotice").hidden || !$("privacyNotice").hidden) return; + await telemetryLoaded; + if (privacyNoticeShown || !$("contactNotice").hidden) return; let s; try { s = await api("/api/contact"); } catch { return; } - if (!s.showPrompt || !$("contactNotice").hidden || !$("privacyNotice").hidden) return; + if (!s.showPrompt || privacyNoticeShown || !$("contactNotice").hidden) return; $("contactNotice").hidden = false; api("/api/contact/prompt", { prompt: "shown" }).catch(() => {}); } diff --git a/ui/server.py b/ui/server.py index 5dfa19f..6ea78cf 100755 --- a/ui/server.py +++ b/ui/server.py @@ -134,6 +134,7 @@ LINK = None # the connector (frame_link.Link); None on the Frame itself # install's clean-up) to the other headset. _work_lock = threading.Lock() _work = [0] +NOT_HEADSET_WORK = {"/api/devices", "/api/contact", "/api/contact/prompt"} @contextlib.contextmanager @@ -2440,7 +2441,8 @@ class Handler(BaseHTTPRequestHandler): body = json.loads(self.rfile.read(length) or b"{}") if not isinstance(body, dict): raise Failure("request body must be a JSON object", 400) - with (contextlib.nullcontext() if path == "/api/devices" else working(meant)): + # Not headset work: switching headsets mustn't wait for (or refuse) these. + with (contextlib.nullcontext() if path in NOT_HEADSET_WORK else working(meant)): result = handler(body) self.send_json(result) except Failure as e: From 08d75e3ffbd7be5005b05bb2be497d4ddf9b61d4 Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:08:16 +1000 Subject: [PATCH 2/4] Contact email: follow-up given with a report is kept and removable; match by rev - Ticking follow-up questions on a report makes that address the contact email (follow-up ticked, update choice unchanged), so Settings shows it and Remove my email withdraws it like any other. - Reports carry contact_rev; the inbox takes a report's follow-up permission back when a later change from that copy (higher rev) no longer agrees, whatever the clocks say. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/privacy.md | 16 +++--- tests/test_contact.py | 105 ++++++++++++++++++++++++---------------- tests/test_telemetry.py | 11 +++-- ui/frame_contact.py | 18 ++++--- ui/frame_report.py | 39 +++++++++------ ui/index.html | 3 +- 6 files changed, 117 insertions(+), 75 deletions(-) diff --git a/docs/privacy.md b/docs/privacy.md index 132f4bf..e866419 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -107,9 +107,12 @@ wrote, a short reference shown after sending, and the diagnostics below. Your email address goes with it only if you tick **The maintainer may contact me with follow-up questions** (the report then carries `contact_followup: true`); it's filled in from **Contact email** below when you've agreed there. It has its own random id, so it isn't linked to -your analytics events. With that box ticked it also carries this copy's contact -id (`contact_id`, see below), so removing or changing the address later takes -back the follow-up permission given with the report too. +your analytics events. With that box ticked, the address also becomes your +**Contact email** below with follow-up questions ticked (your update choice +stays as it was), so you remove it there like any other. The report then also +carries this copy's contact id and change number (`contact_id`, `contact_rev`, +see below), so removing or changing the address later takes back the +follow-up permission given with the report too. With **Include diagnostics** ticked (the default), the report adds: @@ -167,9 +170,10 @@ log (in earlier contact events and problem reports), and sends a `withdraw` event with no address in it. The maintainer's list only uses the newest event from each copy, so from then on the address isn't listed for either choice. Unticking one choice works the same way for that choice. This also covers problem reports you sent -from this copy with follow-up questions ticked: if your newest choice no longer -agrees to follow-up questions at that address, the maintainer's inbox shows the -permission as withdrawn and leaves the address out. If you're offline, the change waits on +from this copy with follow-up questions ticked: if your newest choice since the +report (by change number, not the clock) no longer agrees to follow-up +questions at that address, the maintainer's inbox shows the permission as +withdrawn and leaves the address out. If you're offline, the change waits on this computer and is sent when PostHog can be reached. The earlier event stays in PostHog until its data retention removes it; to have it deleted sooner, ask the maintainer (for example in a problem report). diff --git a/tests/test_contact.py b/tests/test_contact.py index e759f00..7a00f03 100644 --- a/tests/test_contact.py +++ b/tests/test_contact.py @@ -249,69 +249,92 @@ class Contact(Base): # ---- reports and the maintainer's list + def reports(self): + return [e["properties"] for e in self.events() if e["event"] == "problem_report"] + def test_a_report_carries_the_address_only_with_follow_up_consent(self): fr.send({**REPORT, "contact": "me@example.com"}) + self.assertFalse(fc.FILE.exists()) # no follow-up: nothing kept, nothing linked fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": True}) - without, with_ = (e["properties"] for e in self.events()) - self.assertEqual((without["contact"], without["contact_followup"]), ("", False)) + without, with_ = self.reports() + self.assertEqual((without["contact"], without["contact_followup"], without["contact_id"]), ("", False, "")) self.assertEqual((with_["contact"], with_["contact_followup"]), ("me@example.com", True)) + self.assertEqual((with_["contact_id"], with_["contact_rev"]), (fc.load()["id"], fc.load()["rev"])) + self.assertNotEqual(with_["contact_id"], tm.settings()["id"]) # not the analytics id with self.assertRaisesRegex(ValueError, "email address"): fr.send({**REPORT, "contact": "discord:me", "contactFollowup": True}) - def test_a_report_with_follow_up_carries_the_kept_contact_id(self): - fr.send({**REPORT, "contact": "me@example.com"}) - self.assertFalse(fc.FILE.exists()) # no follow-up, nothing kept or linked + def test_follow_up_given_with_a_report_is_kept_and_removed_in_settings(self): fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": True}) - without, with_ = (e["properties"] for e in self.events()) - self.assertEqual(without["contact_id"], "") - self.assertEqual(with_["contact_id"], fc.load()["id"]) - self.assertNotEqual(with_["contact_id"], tm.settings()["id"]) # not the analytics id - fc.save({"email": "me@example.com", "updates": True}) - self.assertEqual(self.events()[-1]["distinct_id"], with_["contact_id"]) # same copy, same id + s = fc.state() + self.assertEqual((s["email"], s["updates"], s["followup"]), ("me@example.com", False, True)) + consent = [e for e in self.events() if e["event"] == "contact_consent"] + self.assertEqual([(e["properties"]["action"], e["properties"]["rev"]) for e in consent], [("set", 1)]) + self.assertEqual(consent[0]["distinct_id"], self.reports()[0]["contact_id"]) + fr.send({**REPORT, "contact": "ME@example.com", "contactFollowup": True}) # already agreed + self.assertEqual(len([e for e in self.events() if e["event"] == "contact_consent"]), 1) + self.assertEqual(self.reports()[1]["contact_rev"], 1) + fc.save({"email": ""}) # Remove my email + last = self.events()[-1] + self.assertEqual((last["properties"]["action"], last["properties"]["email"], last["properties"]["rev"]), + ("withdraw", "", 2)) + logged = [e["properties"].get("contact") for e in tm._read_lines(tm.SENT) if e["event"] == "problem_report"] + self.assertEqual(logged, ["", ""]) - def report_row(self, ts, contact="me@example.com", followup=True, cid="copy"): - return [ts, "AB12CD34", "bug", "RDP", "It never connects.", contact, "0.4.0", "Windows", "", "", followup, cid] + def test_a_report_to_another_address_keeps_the_update_choice(self): + fc.save({"email": "old@example.com", "updates": True}) + fr.send({**REPORT, "contact": "new@example.com", "contactFollowup": True}) + s = fc.state() + self.assertEqual((s["email"], s["updates"], s["followup"]), ("new@example.com", True, True)) + self.assertEqual(self.reports()[0]["contact_rev"], 2) - def test_a_later_withdrawal_takes_back_a_reports_follow_up_permission(self): - reports = [self.report_row("2026-09-10T10:00:00Z"), # removed later - self.report_row("2026-09-12T10:00:00Z", cid="other"), # another copy - self.report_row("2026-09-20T10:00:00Z"), # after the withdrawal - self.report_row("2026-09-11T10:00:00Z", cid="", followup=True), # sent before contact_id - self.report_row("2026-09-15T10:00:00Z", cid="same-second")] - consents = [["copy", "me@example.com", True, 1, "2026-09-01T10:00:00Z"], - ["copy", "", False, 2, "2026-09-14T10:00:00Z"], - ["other", "me@example.com", True, 1, "2026-09-13T10:00:00Z"], # still agrees - ["same-second", "", False, 1, "2026-09-15T10:00:00Z"], ["short"]] + def report_row(self, contact="me@example.com", followup=True, cid="copy", rev=1): + return ["2026-09-10T10:00:00Z", "AB12CD34", "bug", "RDP", "It never connects.", contact, + "0.4.0", "Windows", "", "", followup, cid, rev] + + def test_a_later_change_takes_back_a_reports_follow_up_permission(self): + reports = [self.report_row(), # removed later + self.report_row(cid="other"), # another copy, still agrees + self.report_row(rev=3), # sent after the removal + self.report_row(cid="moved"), # address changed later + self.report_row(cid="news-only"), # follow-up unticked later + self.report_row(contact="Me@Example.com", cid="case"), # same address, any case + self.report_row(cid="", followup=True), # no contact id: left alone + self.report_row(cid="bad", rev="x")] # malformed rev: treated as 0 + consents = [["copy", "me@example.com", True, 1], ["copy", "", False, 2], + ["other", "me@example.com", True, 1], ["other", "me@example.com", True, 2], + ["moved", "new@example.com", True, 2], ["news-only", "me@example.com", False, 2], + ["case", "me@example.com", True, 2], ["bad", "", False, 1], ["short"], ["x", "", False, "?"]] fr.mark_withdrawn(reports, consents) - self.assertEqual([r[10] for r in reports], ["withdrawn", True, True, True, "withdrawn"]) + self.assertEqual([r[10] for r in reports], + ["withdrawn", True, True, "withdrawn", "withdrawn", True, True, "withdrawn"]) - def test_changing_the_address_or_unticking_follow_up_takes_it_back_too(self): - reports = [self.report_row("2026-09-10T10:00:00Z", cid="moved"), - self.report_row("2026-09-10T10:00:00Z", cid="news-only"), - self.report_row("2026-09-10T10:00:00Z", contact="Me@Example.com", cid="case")] - consents = [["moved", "new@example.com", True, 1, "2026-09-11T10:00:00Z"], - ["news-only", "me@example.com", False, 1, "2026-09-11T10:00:00Z"], - ["case", "me@example.com", True, 1, "2026-09-11T10:00:00Z"], - # rev, not the clock, decides which later choice is newest - ["case", "", False, 2, "2026-09-11T09:59:00Z"]] - fr.mark_withdrawn(reports, consents) - self.assertEqual([r[10] for r in reports], ["withdrawn", "withdrawn", "withdrawn"]) - reports[2][10] = True - fr.mark_withdrawn(reports[2:], consents[2:3]) - self.assertIs(reports[2][10], True) # same address, any case, still agrees + def test_the_change_number_decides_not_the_clock(self): + """The clock went back between the report and the removal: the removal still counts.""" + fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": True}) + with mock.patch.object(fc.time, "gmtime", return_value=time.gmtime(0)): + fc.save({"email": ""}) + report = self.reports()[0] + row = self.report_row(cid=report["contact_id"], rev=report["contact_rev"]) + consents = [[e["distinct_id"], e["properties"]["email"], e["properties"]["followup"], e["properties"]["rev"]] + for e in self.events() if e["event"] == "contact_consent"] + self.assertEqual(self.events()[-1]["timestamp"], "1970-01-01T00:00:00Z") + fr.mark_withdrawn([row], consents) + self.assertEqual(row[10], "withdrawn") def test_the_inbox_shows_withdrawn_follow_up_without_the_address(self): - reports = [self.report_row("2026-09-10T10:00:00Z"), ["short"]] - consents = [["copy", "", False, 2, "2026-09-14T10:00:00Z"]] + reports = [self.report_row(), ["short"]] + consents = [["copy", "", False, 2]] with mock.patch.object(db, "_posthog_query", side_effect=[{"results": reports}, {"results": consents}]) as q, \ mock.patch.object(sys, "argv", ["frame_report.py", "inbox", "30"]), \ mock.patch("builtins.print") as out: fr.main() + self.assertIn("properties.contact_rev", q.call_args_list[0].args[0]) self.assertIn("event = 'contact_consent'", q.call_args_list[1].args[0]) printed = " ".join(str(c.args[0]) for c in out.call_args_list if c.args) self.assertIn("follow-up permission since withdrawn", printed) self.assertNotIn("me@example.com", printed) - with mock.patch.object(db, "_posthog_query", return_value={"results": [self.report_row("x", followup=False)]}) as q: + with mock.patch.object(db, "_posthog_query", return_value={"results": [self.report_row(followup=False)]}) as q: fr.inbox() self.assertEqual(q.call_count, 1) # nothing to reconcile, no second query diff --git a/tests/test_telemetry.py b/tests/test_telemetry.py index 3cd50cc..1d249e8 100644 --- a/tests/test_telemetry.py +++ b/tests/test_telemetry.py @@ -391,15 +391,16 @@ class ReportProblem(Base): def test_send_is_a_private_posthog_event_whatever_the_settings(self): got = self.serve() tm.update_settings({"usage": False}) # analytics off: a deliberate report still goes - res = fr.send({"kind": "idea", "title": "Live view stops", "message": "It stops after a minute.", - "contact": "me@example.com", "contactFollowup": True}) + with mock.patch.object(fr.frame_contact, "from_report", return_value=("contact-id", 1)): # test_contact + res = fr.send({"kind": "idea", "title": "Live view stops", "message": "It stops after a minute.", + "contact": "me@example.com", "contactFollowup": True}) path, body = got[0] event = body["batch"][0] self.assertEqual((path, body["api_key"], event["event"]), ("/batch/", "phc_test", "problem_report")) props = event["properties"] self.assertEqual((props["kind"], props["title"], props["message"], props["contact"], props["report_id"]), ("idea", "Live view stops", "It stops after a minute.", "me@example.com", res["id"])) - self.assertIs(props["contact_followup"], True) + self.assertEqual((props["contact_followup"], props["contact_id"], props["contact_rev"]), (True, "contact-id", 1)) self.assertEqual((props["$process_person_profile"], props["$geoip_disable"]), (False, True)) self.assertNotEqual(event["distinct_id"], tm.settings()["id"]) # not linked to the analytics self.assertIn(res["id"], res["message"]) @@ -422,8 +423,8 @@ class ReportProblem(Base): def test_the_inbox_skips_malformed_reports(self): good = ["2026-09-28T09:50:00Z", "AB12CD34", "bug", "Live view stops", "It stops.", None, - "0.4.0", "macOS", "", "", None, None] - rows = [["2026-09-28T10:00:00Z", "X", "bug", "Hand-made", None, None, None, None, None, None, None, None], + "0.4.0", "macOS", "", "", None, None, None] + rows = [["2026-09-28T10:00:00Z", "X", "bug", "Hand-made", None, None, None, None, None, None, None, None, None], ["short"], good] with mock.patch.object(db, "_posthog_query", return_value={"results": rows}), \ mock.patch.object(sys, "argv", ["frame_report.py", "inbox"]), \ diff --git a/ui/frame_contact.py b/ui/frame_contact.py index f64ca2c..5ba3d36 100644 --- a/ui/frame_contact.py +++ b/ui/frame_contact.py @@ -79,14 +79,18 @@ def flag(body, key): return v is True -def contact_id(): - """This copy's contact id, kept from now on. A report with follow-up consent carries it, so - removing the address later takes back the follow-up permission given with the report too.""" - with _lock: +def from_report(email): + """Follow-up questions agreed to with a problem report: the address becomes the contact + email with that choice ticked (the update choice stays as it was), so it shows in Settings + and is removed the same way. Returns (contact id, rev) for the report to carry: a later + change from this copy has a higher rev, and the newest such change decides whether the + report's follow-up permission still stands, whatever the clocks say.""" + s = load() + if s['email'].lower() != email.lower() or not s['followup']: + save({'email': s['email'] if s['email'].lower() == email.lower() else email, + 'updates': s['updates'], 'followup': True}) s = load() - if not FILE.exists(): - _save(s) - return s['id'] + return s['id'], s['rev'] def state(): diff --git a/ui/frame_report.py b/ui/frame_report.py index 043f9b1..c04289a 100644 --- a/ui/frame_report.py +++ b/ui/frame_report.py @@ -116,11 +116,13 @@ def send(body): contact = str(body.get('contact') or '').strip() if followup else '' if followup and not frame_contact.valid_email(contact): raise ValueError('add your email address for follow-up questions, or untick that box') + # It becomes the contact email in Settings, where it's changed or removed like any other. + contact_id, contact_rev = frame_contact.from_report(contact) if followup else ('', 0) ref = uuid.uuid4().hex[:8].upper() props = {**frame_telemetry.common(), 'kind': kind, 'title': title, 'message': text, 'contact': contact, 'contact_followup': followup, 'diagnostics': diag, - # Only with an address: so removing it later (Settings) also takes this permission back. - 'contact_id': frame_contact.contact_id() if followup else '', + # Only with an address: a later change from this copy (higher rev) can take it back. + 'contact_id': contact_id, 'contact_rev': contact_rev, 'report_id': ref, 'steamos': str(frame.get('build') or '')[:120], 'level': 'report'} # Its own random id: a report can carry contact details, so it isn't linked to this copy's analytics. event = {'event': 'problem_report', 'distinct_id': str(uuid.uuid4()), 'uuid': str(uuid.uuid4()), @@ -152,36 +154,43 @@ def inbox(days=30): res = frame_compat_db._posthog_query( "SELECT timestamp, properties.report_id, properties.kind, properties.title, properties.message, " "properties.contact, properties.app_version, properties.os, properties.steamos, properties.diagnostics, " - "properties.contact_followup, properties.contact_id " + "properties.contact_followup, properties.contact_id, properties.contact_rev " f"FROM events WHERE event = 'problem_report' AND timestamp > now() - INTERVAL {days} DAY " "ORDER BY timestamp DESC LIMIT 200") - rows = [r for r in res.get('results') or [] if isinstance(r, list) and len(r) == 12] + rows = [r for r in res.get('results') or [] if isinstance(r, list) and len(r) == 13] if any(r[11] and _yes(r[10]) for r in rows): later = frame_compat_db._posthog_query( - "SELECT distinct_id, properties.email, properties.followup, ifNull(toInt(properties.rev), 0), timestamp " - f"FROM events WHERE event = 'contact_consent' AND timestamp > now() - INTERVAL {days} DAY LIMIT 100000") + "SELECT distinct_id, properties.email, properties.followup, ifNull(toInt(properties.rev), 0) " + "FROM events WHERE event = 'contact_consent' LIMIT 100000") mark_withdrawn(rows, later.get('results') or []) return rows def mark_withdrawn(reports, consents): """Mark reports whose follow-up permission was taken back: the newest contact choice from - the same copy made at or after the report (same second counts, so a withdrawal wins) no + the same copy made after the report (a higher rev than it carries, not a later clock) no longer agrees to follow-up questions at that address.""" newest = {} for c in consents: - if not isinstance(c, list) or len(c) != 5: + if not isinstance(c, list) or len(c) != 4: continue - cid, email, followup, rev, ts = c - newest.setdefault(str(cid), []).append(((int(rev or 0), str(ts or '')), str(email or ''), followup)) + cid, email, followup, rev = c + try: + rev = int(rev or 0) + except (TypeError, ValueError): + continue + if rev > newest.get(str(cid), (-1,))[0]: + newest[str(cid)] = (rev, str(email or ''), followup) for r in reports: if not (r[11] and _yes(r[10])): continue - after = [c for c in newest.get(str(r[11]), []) if c[0][1] >= str(r[0] or '')] - if after: - _, email, followup = max(after, key=lambda c: c[0]) - if not (_yes(followup) and email.strip().lower() == str(r[5] or '').strip().lower()): - r[10] = 'withdrawn' + try: + sent_at = int(r[12] or 0) + except (TypeError, ValueError): + sent_at = 0 + rev, email, followup = newest.get(str(r[11]), (-1, '', None)) + if rev > sent_at and not (_yes(followup) and email.strip().lower() == str(r[5] or '').strip().lower()): + r[10] = 'withdrawn' def _yes(v): diff --git a/ui/index.html b/ui/index.html index 586e14e..ce97499 100644 --- a/ui/index.html +++ b/ui/index.html @@ -1262,7 +1262,7 @@ + Optional. Your email address goes with this report only when this is ticked, and is kept as your contact email in Privacy & updates, where you can remove it. @@ -4125,6 +4125,7 @@ $("bugForm").onsubmit = async e => { $("bugMsg").textContent = `Couldn't send it: ${err.message}. Try again later, or use Copy report.`; $("bugSend").disabled = false; } + if ($("bugFollowup").checked) api("/api/contact").then(renderContact).catch(() => {}); }; if (window.frameApp && window.frameApp.onReportProblem) window.frameApp.onReportProblem(openBugReport); From 989962aecc70a4b312c828843df5b08e8c384c9d Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Thu, 1 Oct 2026 21:25:22 +1000 Subject: [PATCH 3/4] Contact email: a report's rev is its own change; another address starts fresh - from_report applies its change and reads the id and rev together, so a removal made while that change is sending is newer than the report; the report's redaction window now starts before the address is saved. - A report with a different address replaces the saved one with follow-up questions only: update notices aren't carried over to an address nobody agreed them for, and the form says so before sending. - Settings refreshes after every report send, whatever the box shows by then. - privacy.md: a report with follow-up ticked also saves and sends the address. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/privacy.md | 11 ++++++++--- tests/test_contact.py | 30 ++++++++++++++++++++++++++++-- ui/frame_contact.py | 32 ++++++++++++++++++++++---------- ui/frame_report.py | 2 +- ui/index.html | 14 +++++++++++++- 5 files changed, 72 insertions(+), 17 deletions(-) diff --git a/docs/privacy.md b/docs/privacy.md index e866419..726105e 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -108,8 +108,10 @@ email address goes with it only if you tick **The maintainer may contact me with follow-up questions** (the report then carries `contact_followup: true`); it's filled in from **Contact email** below when you've agreed there. It has its own random id, so it isn't linked to your analytics events. With that box ticked, the address also becomes your -**Contact email** below with follow-up questions ticked (your update choice -stays as it was), so you remove it there like any other. The report then also +**Contact email** below with follow-up questions ticked, so you remove it there +like any other. If it's a different address from the one saved there, it +replaces it, and update notices stop until you turn them on again (they were +agreed for the old address); the form says so before you send. The report then also carries this copy's contact id and change number (`contact_id`, `contact_rev`, see below), so removing or changing the address later takes back the follow-up permission given with the report too. @@ -156,7 +158,10 @@ Frame Control's PostHog project, the same place as problem reports, as a `contact_consent` event with `email`, `updates`, `followup`, `action` (`set` or `withdraw`) and the common properties above. Only the maintainer can read that project, and nothing in it is published or shared. It's sent only when -you save, whatever the analytics settings are, because you chose to. It +you save, or when you send a problem report with follow-up questions ticked, +whatever the analytics settings are, because you chose to. With a report, the +address is saved and sent first, so it stands even if the report itself then +fails to send. It carries its own random contact id, not the analytics id, so it isn't linked to your usage events, and a `rev` number that goes up with each change, so the newest choice always wins. Like everything else sent, it's listed under diff --git a/tests/test_contact.py b/tests/test_contact.py index 7a00f03..882a596 100644 --- a/tests/test_contact.py +++ b/tests/test_contact.py @@ -281,12 +281,38 @@ class Contact(Base): logged = [e["properties"].get("contact") for e in tm._read_lines(tm.SENT) if e["event"] == "problem_report"] self.assertEqual(logged, ["", ""]) - def test_a_report_to_another_address_keeps_the_update_choice(self): + def test_a_report_to_another_address_replaces_it_with_follow_up_only(self): + """Update notices were agreed for the old address, not the new one (the form says so).""" fc.save({"email": "old@example.com", "updates": True}) fr.send({**REPORT, "contact": "new@example.com", "contactFollowup": True}) s = fc.state() - self.assertEqual((s["email"], s["updates"], s["followup"]), ("new@example.com", True, True)) + self.assertEqual((s["email"], s["updates"], s["followup"]), ("new@example.com", False, True)) self.assertEqual(self.reports()[0]["contact_rev"], 2) + fc.save({"email": "new@example.com", "updates": True, "followup": False}) + fr.send({**REPORT, "contact": "NEW@example.com", "contactFollowup": True}) # same address: kept + s = fc.state() + self.assertEqual((s["email"], s["updates"], s["followup"]), ("new@example.com", True, True)) + + def test_a_removal_while_the_report_saves_its_address_still_counts(self): + """Removed while the report's own consent is on its way: the report keeps that consent's + rev (so the removal is newer) and is logged without the address.""" + post, removed = tm.post, [] + + def slow_post(events, **kw): + post(events, **kw) + if not removed and events[0]["event"] == "contact_consent": + removed.append(fc.save({"email": ""})) # Remove my email, mid-send + with mock.patch.object(tm, "post", side_effect=slow_post): + fr.send({**REPORT, "contact": "me@example.com", "contactFollowup": True}) + report = self.reports()[0] + self.assertEqual((report["contact_rev"], fc.load()["rev"], fc.state()["email"]), (1, 2, "")) + consents = [[e["distinct_id"], e["properties"]["email"], e["properties"]["followup"], e["properties"]["rev"]] + for e in self.events() if e["event"] == "contact_consent"] + row = self.report_row(cid=report["contact_id"], rev=report["contact_rev"]) + fr.mark_withdrawn([row], consents) + self.assertEqual(row[10], "withdrawn") + logged = [e["properties"]["contact"] for e in tm._read_lines(tm.SENT) if e["event"] == "problem_report"] + self.assertEqual(logged, [""]) def report_row(self, contact="me@example.com", followup=True, cid="copy", rev=1): return ["2026-09-10T10:00:00Z", "AB12CD34", "bug", "RDP", "It never connects.", contact, diff --git a/ui/frame_contact.py b/ui/frame_contact.py index 5ba3d36..8dca387 100644 --- a/ui/frame_contact.py +++ b/ui/frame_contact.py @@ -81,16 +81,19 @@ def flag(body, key): def from_report(email): """Follow-up questions agreed to with a problem report: the address becomes the contact - email with that choice ticked (the update choice stays as it was), so it shows in Settings - and is removed the same way. Returns (contact id, rev) for the report to carry: a later - change from this copy has a higher rev, and the newest such change decides whether the - report's follow-up permission still stands, whatever the clocks say.""" - s = load() - if s['email'].lower() != email.lower() or not s['followup']: - save({'email': s['email'] if s['email'].lower() == email.lower() else email, - 'updates': s['updates'], 'followup': True}) + email with that choice ticked, so it shows in Settings and is removed the same way. Update + notices stay on only for the same address: a different one replaces the old address with + follow-up questions only (the report form says so before sending). Returns (contact id, + rev) for the report to carry, read together with the change itself: a later change from + this copy has a higher rev, and the newest such change decides whether the report's + follow-up permission still stands, whatever the clocks say.""" + with _lock: s = load() - return s['id'], s['rev'] + same = s['email'].lower() == email.lower() + changed, cid, rev = _apply({'email': s['email'] if same else email, + 'updates': s['updates'] and same, 'followup': True}) + _deliver(changed) + return cid, rev def state(): @@ -177,6 +180,12 @@ def redact_removed(event, started): def save(body): """Set, change or remove the address and the two choices. An address needs at least one choice ticked; an empty address (or neither ticked) removes it and withdraws both.""" + _deliver(_apply(body)[0]) + return state() + + +def _apply(body): + """save()'s change, kept here and waiting to send. Returns (changed, contact id, rev).""" email = str(body.get('email') or '').strip() updates, followup = flag(body, 'updates'), flag(body, 'followup') if email and not valid_email(email): @@ -203,9 +212,12 @@ def save(body): _forget_locally(old) except OSError: pass + return changed, s['id'], s['rev'] + + +def _deliver(changed): if changed and not _send_pending(block=False): _wake.set() # offline, or a send under way that will take this change with it - return state() def prompt(body): diff --git a/ui/frame_report.py b/ui/frame_report.py index c04289a..1073e29 100644 --- a/ui/frame_report.py +++ b/ui/frame_report.py @@ -116,6 +116,7 @@ def send(body): contact = str(body.get('contact') or '').strip() if followup else '' if followup and not frame_contact.valid_email(contact): raise ValueError('add your email address for follow-up questions, or untick that box') + started = time.time() # a removal from now on (even while saving the address) is redacted from the log # It becomes the contact email in Settings, where it's changed or removed like any other. contact_id, contact_rev = frame_contact.from_report(contact) if followup else ('', 0) ref = uuid.uuid4().hex[:8].upper() @@ -127,7 +128,6 @@ def send(body): # Its own random id: a report can carry contact details, so it isn't linked to this copy's analytics. event = {'event': 'problem_report', 'distinct_id': str(uuid.uuid4()), 'uuid': str(uuid.uuid4()), 'timestamp': time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime()), 'properties': props} - started = time.time() try: frame_telemetry.post([event], timeout=30) except frame_telemetry.SendError as e: diff --git a/ui/index.html b/ui/index.html index ce97499..9809512 100644 --- a/ui/index.html +++ b/ui/index.html @@ -1264,6 +1264,7 @@ +