From 29237ff9a81f628ce378f02adc0d04f703c4d3d8 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 4 Sep 2026 17:28:00 +0000 Subject: [PATCH] fix(telemetry): one-shot setup events never sweep other sessions' pending markers gstack-telemetry-log finalizes every .pending- marker that is not the caller's own as outcome:unknown and deletes it. setup's onboarding events (_setup_welcome, _setup_playwright) have no session of their own, so a Chromium bootstrap failure during a live skill session recorded a false unknown for that session and removed its marker. New --no-sweep flag skips the stale-marker pass; both setup call sites use it (the synthetic --session-id did not prevent the sweep). Surfaced by the Codex adversarial pass. Co-Authored-By: Claude Fable 5.1 --- bin/gstack-telemetry-log | 10 ++++++++++ setup | 15 +++++++++++---- test/telemetry.test.ts | 14 ++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/bin/gstack-telemetry-log b/bin/gstack-telemetry-log index 05d9c9866..4426f9f98 100755 --- a/bin/gstack-telemetry-log +++ b/bin/gstack-telemetry-log @@ -10,6 +10,11 @@ # gstack-telemetry-log --skill qa --duration 142 --outcome success \ # --used-browse true --session-id "12345-1710756600" # +# --no-sweep one-shot events with no session of their own (setup's +# onboarding events) must not finalize OTHER sessions' in-flight +# .pending markers as outcome:unknown — only a skill's own +# epilogue may sweep. +# # Env overrides (for testing): # GSTACK_STATE_DIR — override ~/.gstack state directory # GSTACK_DIR — override auto-detected gstack root @@ -48,9 +53,11 @@ SEC_PAYLOAD_HASH="" SEC_CONFIDENCE="" SEC_LAYER="" SEC_VERDICT="" +NO_SWEEP="" while [ $# -gt 0 ]; do case "$1" in + --no-sweep) NO_SWEEP=1; shift ;; --skill) SKILL="$2"; shift 2 ;; --duration) DURATION="$2"; shift 2 ;; --outcome) OUTCOME="$2"; shift 2 ;; @@ -93,7 +100,10 @@ fi # ─── Finalize stale .pending markers ──────────────────────── # Each session gets its own .pending-$SESSION_ID file to avoid races # between concurrent sessions. Finalize any that don't match our session. +# --no-sweep (one-shot events from setup) skips this: a marker that is not +# ours is another live session's, not a stale one. for PFILE in "$PENDING_DIR"/.pending-*; do + [ -z "$NO_SWEEP" ] || break [ -f "$PFILE" ] || continue # Skip our own session's marker (it's still in-flight) PFILE_BASE="$(basename "$PFILE")" diff --git a/setup b/setup index 52594e72f..d1cfebb06 100755 --- a/setup +++ b/setup @@ -2021,7 +2021,7 @@ if [ ! -f "$HOME/.gstack/.welcome-seen" ]; then log "" # Best-effort onboarding telemetry (respects telemetry!=off; never blocks setup). if [ -x "$SOURCE_GSTACK_DIR/bin/gstack-telemetry-log" ]; then - "$SOURCE_GSTACK_DIR/bin/gstack-telemetry-log" --event-type onboarding --skill _setup_welcome --outcome shown >/dev/null 2>&1 || true + "$SOURCE_GSTACK_DIR/bin/gstack-telemetry-log" --event-type onboarding --skill _setup_welcome --outcome shown --no-sweep >/dev/null 2>&1 || true fi touch "$HOME/.gstack/.welcome-seen" fi @@ -2641,9 +2641,16 @@ if [ -n "${_PW_FAIL_REASON:-}" ]; then case "$_PW_FAIL_REASON" in *post-install-launch*) log " Ubuntu 24.04+ (AppArmor user namespaces): GSTACK_CHROMIUM_NO_SANDBOX=1 ./setup (#2157)" ;; esac - # Reason code only — never a path or command line — so a support thread can - # be matched to a code. Telemetry-gated inside gstack-telemetry-log. + # Reason code only — never a path, hostname, or the installer's output. The + # event is a one-shot with no session of its own, so --no-sweep keeps it + # from finalizing other live sessions' in-flight .pending markers as + # outcome:unknown. Telemetry-gated inside gstack-telemetry-log. if [ -x "$SOURCE_GSTACK_DIR/bin/gstack-telemetry-log" ]; then - "$SOURCE_GSTACK_DIR/bin/gstack-telemetry-log" --event-type onboarding --skill _setup_playwright --outcome "$_PW_FAIL_REASON" >/dev/null 2>&1 || true + "$SOURCE_GSTACK_DIR/bin/gstack-telemetry-log" --event-type onboarding --skill _setup_playwright --outcome "$_PW_FAIL_REASON" --no-sweep >/dev/null 2>&1 || true fi fi +if [ ${#_FOREIGN_SKIPPED_ENTRIES[@]} -gt 0 ]; then + log "" + log "Not registered (a skill you own already uses the name; left untouched): ${_FOREIGN_SKIPPED_ENTRIES[*]}" + log " Rename or move yours, or switch modes (./setup --prefix / --no-prefix) so the names no longer collide." +fi diff --git a/test/telemetry.test.ts b/test/telemetry.test.ts index 668349481..06dd43221 100644 --- a/test/telemetry.test.ts +++ b/test/telemetry.test.ts @@ -332,6 +332,20 @@ describe('gstack-telemetry-log', () => { }); describe('.pending marker', () => { + test('--no-sweep (one-shot setup events) leaves other sessions\' in-flight markers untouched', () => { + setConfig('telemetry', 'anonymous'); + const analyticsDir = path.join(tmpDir, 'analytics'); + fs.mkdirSync(analyticsDir, { recursive: true }); + const marker = path.join(analyticsDir, '.pending-live-456'); + fs.writeFileSync(marker, '{"skill":"ship","ts":"2026-09-04T00:00:00Z","session_id":"live-456","gstack_version":"1.79.0.0"}'); + run(`${BIN}/gstack-telemetry-log --event-type onboarding --skill _setup_playwright --outcome chromium-install --no-sweep`); + expect(fs.existsSync(marker)).toBe(true); + const events = parseJsonl(); + expect(events).toHaveLength(1); + expect(events[0].event_type).toBe('onboarding'); + expect(events[0].outcome).toBe('chromium-install'); + }); + test('finalizes stale .pending from another session as outcome:unknown', () => { setConfig('telemetry', 'anonymous');