Skip to content

Commit 0466d15

Browse files
Lykhoydaclaude
andauthored
fix(#266): report telemetry staleness honestly in /send-feedback collector (#284)
* fix(#266): report telemetry staleness honestly in /send-feedback collector Telemetry capture didn't break — it was removed by design: commit 3beb8e5 (PR #203, closes #200) deleted the Experience Engine including src/experience/telemetry.ts, the per-tool-call writer of ~/.claude/rn-agent/telemetry/*.jsonl. collect-feedback.sh was never updated and kept tailing the orphaned files, presenting weeks-old events as "Recent Tool Activity" in filed issues (disk evidence: newest file 2026-05-31 20:44, the writer's last day alive). Fix (collector honesty, NOT resurrecting the removed writer): - collect-feedback.sh cross-checks the newest event's age. <24h → telemetry_status "ok" + events as before (legacy versions still writing). Otherwise events are omitted and telemetry_status says "stale (last event N days ago — ...)" or "none" explicitly. - Empty-telemetry edge no longer emits a single bogus {} event (the "[]" placeholder used to pass through the dict filter). - send-feedback.md template renders the status line when no fresh events exist — never an empty/misleading activity table. - New end-to-end test scripts/test/telemetry-staleness.test.sh (stale / none / fresh), registered in CI. Verified live: against this machine's real orphaned files the collector now reports "stale (last event 11 days ago ...)" with 0 events. redact.test.sh still passes. Closes #266 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(#266): harden collector against empty telemetry dir + future mtime — review findings Multi-LLM review round (both SHIP) surfaced two verified edges: - Empty telemetry dir (manual cleanup) made the unmatched glob fail `ls`, and under `set -euo pipefail` that killed the WHOLE collector — /send-feedback got zero JSON (verified live: exit=1, 0 bytes). Pre-existing, but same block and same bug family; `|| true` on the pipeline degrades it to telemetry_status "none". - Future file mtime (clock skew, fs restore) yielded a negative age that passed `-lt 1` and shipped possibly-stale events as fresh. Fresh now requires 0 <= age < 1; negative/unparseable ages report "unknown". - Test cleanup: trap on parent-scope home vars (array registration from $(...) subshells never reached the parent — caught by the run itself); "day(s)" grammar. telemetry-staleness.test.sh: 10/10. redact.test.sh: pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 016afde commit 0466d15

5 files changed

Lines changed: 164 additions & 7 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"rn-dev-agent-plugin": patch
3+
---
4+
5+
`/send-feedback` no longer presents weeks-old telemetry as "recent" (GH #266).
6+
7+
Root cause: the per-tool-call telemetry writer was removed with the Experience Engine (GH #200, v0.49 era), but `collect-feedback.sh` kept reading the orphaned `~/.claude/rn-agent/telemetry/*.jsonl` files and shipped their tail as "Recent Tool Activity" in filed issues. The collector now cross-checks the newest event's age: fresh events (<24h, legacy plugin versions still writing) ship as before with `telemetry_status: "ok"`; otherwise events are omitted and `telemetry_status` reports `stale (last event N days ago — …)` or `none` explicitly. The `/send-feedback` issue template renders the status line instead of an empty/misleading activity table, and the empty-telemetry edge no longer emits a single bogus `{}` event.

.github/workflows/ci.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ jobs:
5353
- name: SessionStart bounded-startup guard
5454
run: bash scripts/test/session-start-bounded.test.sh
5555

56+
# GH#266: feedback collector must not present stale/orphaned telemetry as recent.
57+
- name: Feedback telemetry staleness guard
58+
run: bash scripts/test/telemetry-staleness.test.sh
59+
5660
version-sync:
5761
name: Version sync check
5862
runs-on: ubuntu-latest

commands/send-feedback.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ This collects (all redacted):
4646
- OS, Node.js, npm versions
4747
- Simulator/emulator status, Metro status
4848
- agent-device and maestro-runner versions
49-
- Last 20 telemetry events (tool name, result, latency — no params or paths)
49+
- Last 20 telemetry events ONLY when fresh (<24h; tool name, result, latency — no params or paths), plus `telemetry_status` (`ok` / `stale (...)` / `none`). On current plugin versions `stale`/`none` is expected — per-tool-call telemetry capture was removed with the Experience Engine (GH #200); only legacy versions still write it.
5050

5151
Also call `cdp_status` to get current CDP connection state (if available).
5252

@@ -116,7 +116,7 @@ gh issue create \
116116
117117
## Recent Tool Activity
118118
119-
<table of last 5 tool calls with result and latency>
119+
<ONLY if recent_telemetry_lines is non-empty: table of last 5 tool calls with result and latency. Otherwise render the telemetry_status value verbatim on one line (e.g. "Telemetry: none" or the stale explanation) — never present old events as recent (GH #266).>
120120
121121
## CDP State at Time of Report
122122

scripts/collect-feedback.sh

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,34 @@ elif curl -s --max-time 2 http://localhost:8082/status 2>/dev/null | grep -q "pa
8787
fi
8888

8989
# --- Collect recent telemetry (last 20 events, redacted) ---
90+
# GH #266: the per-tool-call telemetry writer was removed with the Experience
91+
# Engine (GH #200), so on current plugin versions these files stop updating.
92+
# Cross-check the newest event's age against now and only ship events that are
93+
# actually recent (<24h, e.g. a legacy plugin version still writing); report
94+
# staleness explicitly instead of presenting weeks-old events as current.
9095

9196
recent_telemetry="[]"
97+
telemetry_status="none"
9298
if [ -d "$TELEMETRY_DIR" ]; then
93-
latest_log=$(ls -t "$TELEMETRY_DIR"/*.jsonl 2>/dev/null | head -1)
99+
# `|| true`: an empty dir makes the unmatched glob fail `ls`, and under
100+
# `set -euo pipefail` that used to kill the WHOLE collector (zero JSON).
101+
latest_log=$(ls -t "$TELEMETRY_DIR"/*.jsonl 2>/dev/null | head -1 || true)
94102
if [ -n "$latest_log" ]; then
95-
raw=$(tail -20 "$latest_log" 2>/dev/null || echo "")
96-
if [ -n "$raw" ]; then
97-
recent_telemetry=$(redact "$raw")
103+
age_days=$(python3 -c "import os,sys,time; print(int((time.time()-os.path.getmtime(sys.argv[1]))//86400))" "$latest_log" 2>/dev/null || echo "")
104+
# `-ge 0`: a future mtime (clock skew, fs restore) yields a negative age
105+
# and must not count as fresh.
106+
if [ -n "$age_days" ] && [ "$age_days" -ge 0 ] 2>/dev/null && [ "$age_days" -lt 1 ] 2>/dev/null; then
107+
telemetry_status="ok"
108+
raw=$(tail -20 "$latest_log" 2>/dev/null || echo "")
109+
if [ -n "$raw" ]; then
110+
recent_telemetry=$(redact "$raw")
111+
fi
112+
else
113+
case "$age_days" in
114+
''|-*) age_label="unknown";;
115+
*) age_label="$age_days";;
116+
esac
117+
telemetry_status="stale (last event ${age_label} day(s) ago — telemetry capture is not active in this plugin version; it was removed with the Experience Engine, GH #200. Old events omitted.)"
98118
fi
99119
fi
100120
fi
@@ -147,6 +167,8 @@ for line in lines:
147167
continue
148168
try:
149169
e = json.loads(line)
170+
if not isinstance(e, dict):
171+
continue
150172
safe = {k: e[k] for k in ['ts','event','tool','result','latency_ms','phase'] if k in e}
151173
events.append(safe)
152174
except:
@@ -171,6 +193,7 @@ data = {
171193
'maestro_runner': sys.argv[11],
172194
},
173195
'recent_telemetry_lines': json.loads(sys.argv[12]),
196+
'telemetry_status': sys.argv[14],
174197
}
175198
log_tail = sys.argv[13].strip()
176199
if log_tail:
@@ -189,4 +212,5 @@ print(json.dumps(data, indent=2))
189212
"$agent_device_version" \
190213
"$maestro_runner_version" \
191214
"$telemetry_json" \
192-
"$cdp_log_tail"
215+
"$cdp_log_tail" \
216+
"$telemetry_status"
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
#!/usr/bin/env bash
2+
# Regression test for collect-feedback.sh telemetry staleness (GH #266).
3+
#
4+
# The per-tool-call telemetry writer was removed with the Experience Engine
5+
# (GH #200, commit 3beb8e52, 2026-06-01), but collect-feedback.sh kept reading
6+
# the orphaned ~/.claude/rn-agent/telemetry/*.jsonl files and presented
7+
# weeks-old events as "recent" in filed feedback issues. The collector must
8+
# cross-check the newest event's age against now and report honestly:
9+
# - fresh (<24h, legacy writer still active) → telemetry_status "ok" + events
10+
# - stale (>=24h) → "stale (...)" + NO events
11+
# - absent (no dir / no files) → "none" + NO events
12+
#
13+
# End-to-end: fake $HOME with planted telemetry files, run the real script,
14+
# assert on the emitted JSON.
15+
#
16+
# Run: bash scripts/test/telemetry-staleness.test.sh
17+
18+
set -uo pipefail
19+
20+
SCRIPT_DIR="$(cd "$(dirname "$0")/.." && pwd)"
21+
COLLECT="$SCRIPT_DIR/collect-feedback.sh"
22+
23+
fail=0
24+
25+
# jq-free JSON assertions via python3 (already a hard dependency of the script).
26+
get_field() { # $1=json $2=python expr over `d`
27+
printf '%s' "$1" | python3 -c "import json,sys; d=json.load(sys.stdin); print($2)" 2>/dev/null
28+
}
29+
30+
run_collect() { # $1=fake home
31+
HOME="$1" CLAUDE_PLUGIN_DATA="$1/plugin-data" bash "$COLLECT" 2>/dev/null
32+
}
33+
34+
# make_home runs inside $(...) subshells, so cleanup state can't be
35+
# accumulated there — the trap references the parent-scope home vars instead
36+
# (":-" keeps `set -u` happy for homes not yet created at failure time).
37+
trap 'rm -rf "${home1:-}" "${home2:-}" "${home3:-}" "${home4:-}" "${home5:-}"' EXIT
38+
39+
make_home() {
40+
local h
41+
h="$(mktemp -d)"
42+
mkdir -p "$h/plugin-data"
43+
printf '%s' "$h"
44+
}
45+
46+
check() { # $1=label $2=actual $3=expected-prefix
47+
case "$2" in
48+
"$3"*) echo "ok: $1 ($2)";;
49+
*) echo "FAIL: $1 — expected prefix '$3', got '$2'"; fail=1;;
50+
esac
51+
}
52+
53+
# ── Case 1: stale telemetry (the GH #266 repro) ─────────────────────
54+
home1="$(make_home)"
55+
tdir1="$home1/.claude/rn-agent/telemetry"
56+
mkdir -p "$tdir1"
57+
cat > "$tdir1/2026-05-31-session-1234.jsonl" <<'EOF'
58+
{"ts":"2026-05-31T18:44:32.560Z","event":"tool_call","tool":"cdp_status","result":"PASS","latency_ms":1,"phase":"tool"}
59+
EOF
60+
# Backdate mtime far past the 24h threshold (portable across BSD/GNU touch).
61+
touch -t 202605311844 "$tdir1/2026-05-31-session-1234.jsonl"
62+
63+
out1="$(run_collect "$home1")"
64+
status1="$(get_field "$out1" "d.get('telemetry_status','<missing>')")"
65+
count1="$(get_field "$out1" "len(d.get('recent_telemetry_lines',['sentinel']))")"
66+
check "stale: telemetry_status flags staleness" "$status1" "stale"
67+
check "stale: status names the age in days" "$status1" "stale (last event"
68+
check "stale: old events are NOT shipped as recent" "$count1" "0"
69+
70+
# ── Case 2: no telemetry at all (fresh install post-#200) ───────────
71+
home2="$(make_home)"
72+
out2="$(run_collect "$home2")"
73+
status2="$(get_field "$out2" "d.get('telemetry_status','<missing>')")"
74+
count2="$(get_field "$out2" "len(d.get('recent_telemetry_lines',['sentinel']))")"
75+
check "none: telemetry_status reports none" "$status2" "none"
76+
check "none: no events shipped" "$count2" "0"
77+
78+
# ── Case 3: fresh telemetry (legacy writer still active) ────────────
79+
home3="$(make_home)"
80+
tdir3="$home3/.claude/rn-agent/telemetry"
81+
mkdir -p "$tdir3"
82+
cat > "$tdir3/current-session.jsonl" <<'EOF'
83+
{"ts":"2026-06-12T12:00:00.000Z","event":"tool_call","tool":"cdp_status","result":"PASS","latency_ms":2,"phase":"tool"}
84+
EOF
85+
# mtime = now (just written) → fresh.
86+
87+
out3="$(run_collect "$home3")"
88+
status3="$(get_field "$out3" "d.get('telemetry_status','<missing>')")"
89+
count3="$(get_field "$out3" "len(d.get('recent_telemetry_lines',[]))")"
90+
check "fresh: telemetry_status ok" "$status3" "ok"
91+
check "fresh: events ARE shipped" "$count3" "1"
92+
93+
# ── Case 4: telemetry dir exists but is EMPTY (manual cleanup) ──────
94+
# Pre-existing hazard surfaced in the #266 review: the unmatched glob made
95+
# `ls` fail and, under `set -euo pipefail`, killed the WHOLE collector —
96+
# /send-feedback got zero JSON. Must degrade to status "none" instead.
97+
home4="$(make_home)"
98+
mkdir -p "$home4/.claude/rn-agent/telemetry"
99+
out4="$(run_collect "$home4")"
100+
status4="$(get_field "$out4" "d.get('telemetry_status','<missing>')")"
101+
check "empty dir: collector survives and reports none" "$status4" "none"
102+
103+
# ── Case 5: future mtime (clock skew / filesystem restore) ──────────
104+
# A negative age must never count as fresh (it would ship stale events).
105+
home5="$(make_home)"
106+
tdir5="$home5/.claude/rn-agent/telemetry"
107+
mkdir -p "$tdir5"
108+
cat > "$tdir5/skewed-session.jsonl" <<'EOF'
109+
{"ts":"2031-01-01T00:00:00.000Z","event":"tool_call","tool":"cdp_status","result":"PASS","latency_ms":3,"phase":"tool"}
110+
EOF
111+
touch -t 203101010000 "$tdir5/skewed-session.jsonl"
112+
out5="$(run_collect "$home5")"
113+
status5="$(get_field "$out5" "d.get('telemetry_status','<missing>')")"
114+
count5="$(get_field "$out5" "len(d.get('recent_telemetry_lines',['sentinel']))")"
115+
check "future mtime: not treated as fresh" "$status5" "stale"
116+
check "future mtime: no events shipped" "$count5" "0"
117+
118+
if [ "$fail" -ne 0 ]; then
119+
echo "telemetry-staleness.test.sh: FAILURES"
120+
exit 1
121+
fi
122+
echo "telemetry-staleness.test.sh: all assertions passed"

0 commit comments

Comments
 (0)