From 85a77f7e4dec977be023c62725bcbdce0266435e Mon Sep 17 00:00:00 2001 From: Tomer Weller Date: Thu, 27 Aug 2026 00:06:03 +0000 Subject: [PATCH 1/3] =?UTF-8?q?Regression=20test=20for=20#3844=20=E2=80=94?= =?UTF-8?q?=20fails=20on=20current=20main?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds test_startup_peak_phase_extraction_quote_tolerant to scripts/test-monitor-skill-snippets.sh: extracts the check-(7) phase- extraction bash block from monitor-tick/SKILL.md and runs it against the real tracing-quoted summary line (phase="cache-scan"). On main the unquoted phase=[a-z_-]+ class matches nothing, so STARTUP_PEAK_PHASE is empty and both the behavioral and doc-consistency assertions fail. Refs #3844 Co-authored-by: Claude Code --- scripts/test-monitor-skill-snippets.sh | 58 +++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/scripts/test-monitor-skill-snippets.sh b/scripts/test-monitor-skill-snippets.sh index c0d7d74d..84fc0ee5 100755 --- a/scripts/test-monitor-skill-snippets.sh +++ b/scripts/test-monitor-skill-snippets.sh @@ -55,7 +55,7 @@ cleanup() { trap cleanup EXIT # ── TAP state ──────────────────────────────────────────────────────────────── -TAP_PLAN=486 +TAP_PLAN=489 TAP_CURRENT=0 TAP_FAILURES=0 @@ -11391,6 +11391,62 @@ Cargo.toml" tap_not_ok "structural: Test 6 boundary check is deterministic and guarded (#3766)" \ "Test 6 must mock 7199 (not the exact 7200 boundary) and guard its check_session_wiped call via exit_code6" fi + + # ── Startup-peak phase extraction is quote-tolerant (#3844) ───────────────── + # `tracing` quotes string fields, so the check-(7) summary line the sampler + # emits reads `startup_peak_anon_rss_mb=21047 phase="cache-scan"`. The pre-fix + # snippet used `phase=[a-z_-]+`, whose class matched neither the leading `"` + # nor — because of the `+` — the numeric prefix, silently yielding an empty + # STARTUP_PEAK_PHASE and dropping the `(phase=…)` suffix from WATCH_ITEMS. + # This block sources the extraction snippet *from the skill itself* so the + # test fails on origin/main and passes only once the pattern is fixed. + local phase_tick_md="$REPO_ROOT/.claude/skills/monitor-tick/SKILL.md" + # Two nearby ```bash fences exist; select the one that actually assigns + # STARTUP_PEAK_PHASE by content, not position. + local phase_block + phase_block=$(awk ' + /^```bash$/ { inblk=1; buf=""; next } + /^```$/ { if (inblk && buf ~ /STARTUP_PEAK_PHASE=/) printf "%s", buf; inblk=0; next } + inblk { buf = buf $0 ORS } + ' "$phase_tick_md") + + if [[ -n "$phase_block" && "$phase_block" == *STARTUP_PEAK_PHASE=* ]]; then + tap_ok "startup-peak phase: extracted the STARTUP_PEAK_PHASE bash block from SKILL.md" + else + tap_not_ok "startup-peak phase: extracted the STARTUP_PEAK_PHASE bash block from SKILL.md" \ + "block empty or missing STARTUP_PEAK_PHASE= assignment" + fi + + # Behavioral: run the extracted block against the REAL emitted (quoted) line. + # The block hardcodes /home/tomer/data/$MONITOR_SESSION_ID; rewrite that base + # to a portable temp session dir under TEST_ROOT so the test does not depend + # on the operator's home path (keeps CI green after the fix). + local phase_session phase_block_run phase_result + phase_session="$TEST_ROOT/phase3844" + mkdir -p "$phase_session/logs" + printf '%s\n' '2026-08-09T09:53:13.843959Z INFO henyey_ledger::peak_rss_sampler: Startup peak RSS summary startup_peak_anon_rss_mb=21047 phase="cache-scan"' \ + > "$phase_session/logs/monitor.log" + phase_block_run=$(sed "s#/home/tomer/data/\$MONITOR_SESSION_ID#$phase_session#g" <<<"$phase_block") + phase_result=$( + set +e + WATCH_ITEMS=() + eval "$phase_block_run" + printf '%s' "${STARTUP_PEAK_PHASE:-}" + ) + if [[ "$phase_result" == "cache-scan" ]]; then + tap_ok "startup-peak phase: quote-tolerant extraction yields 'cache-scan' (#3844)" + else + tap_not_ok "startup-peak phase: quote-tolerant extraction yields 'cache-scan' (#3844)" \ + "got STARTUP_PEAK_PHASE='$phase_result' (pre-fix phase=[a-z_-]+ cannot match phase=\"cache-scan\")" + fi + + # Doc-consistency: the pattern must be quote-tolerant AND strip captured quotes. + if grep -qF 'phase="?[a-z_-]+"?' <<<"$phase_block" && grep -qF "tr -d '\"'" <<<"$phase_block"; then + tap_ok "startup-peak phase: SKILL.md pattern is quote-tolerant and strips quotes (#3844)" + else + tap_not_ok "startup-peak phase: SKILL.md pattern is quote-tolerant and strips quotes (#3844)" \ + "expected phase=\"?[a-z_-]+\"? and a tr -d '\"' strip in the extracted block" + fi } check_skill_structure run_tests From 3d72368b87793dd17f6b2387bfd1e858c2643f6b Mon Sep 17 00:00:00 2001 From: Tomer Weller Date: Thu, 27 Aug 2026 00:09:35 +0000 Subject: [PATCH 2/3] Make check-(7) startup-peak phase grep quote-tolerant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tracing's default field formatter quotes string values, so the sampler's summary line is `startup_peak_anon_rss_mb= phase=""`. The extraction used `phase=[a-z_-]+`, whose class matched neither the leading `"` nor (because `+` requires a class member) the numeric prefix, so the whole alternation failed and STARTUP_PEAK_PHASE was silently empty on every tick — the `(phase=…)` suffix never reached WATCH_ITEMS. Add an optional quote `"?` on both sides of the class in both grep stages and strip captured quotes with `tr -d '"'`. Using `"?` (not a mandatory `"`) keeps the pattern robust to both the quoted Text form and any unquoted rendering, matching the sampler's own unit-test tolerance. Also correct the misleading format comment to show the quoted form. Refs #3844 Co-authored-by: Claude Code --- .claude/skills/monitor-tick/SKILL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.claude/skills/monitor-tick/SKILL.md b/.claude/skills/monitor-tick/SKILL.md index e45d36a8..da2ebf50 100644 --- a/.claude/skills/monitor-tick/SKILL.md +++ b/.claude/skills/monitor-tick/SKILL.md @@ -1097,10 +1097,10 @@ attributed `phase` from the greppable log summary line: PROM=/home/tomer/data/$MONITOR_SESSION_ID/metrics/current.prom STARTUP_PEAK_MB=$(awk '/^henyey_startup_peak_anon_rss_mb /{printf "%d", $2}' "$PROM" 2>/dev/null) # phase label lives in the log summary line, not the gauge: -# "...startup_peak_anon_rss_mb= phase=..." -STARTUP_PEAK_PHASE=$(grep -oE 'startup_peak_anon_rss_mb=[0-9]+ phase=[a-z_-]+' \ +# '...startup_peak_anon_rss_mb= phase=""...' (tracing quotes string fields) +STARTUP_PEAK_PHASE=$(grep -oE 'startup_peak_anon_rss_mb=[0-9]+ phase="?[a-z_-]+"?' \ /home/tomer/data/$MONITOR_SESSION_ID/logs/monitor.log 2>/dev/null \ - | tail -1 | grep -oE 'phase=[a-z_-]+' | cut -d= -f2) + | tail -1 | grep -oE 'phase="?[a-z_-]+"?' | cut -d= -f2 | tr -d '"') if [ -n "$STARTUP_PEAK_MB" ] && [ "$STARTUP_PEAK_MB" -gt 0 ]; then WATCH_ITEMS+=("startup_peak_mb=$STARTUP_PEAK_MB${STARTUP_PEAK_PHASE:+ (phase=$STARTUP_PEAK_PHASE)}") fi From 301508abe36cce9c1499695f35be02d762eba424 Mon Sep 17 00:00:00 2001 From: Tomer Weller Date: Thu, 27 Aug 2026 01:13:11 +0000 Subject: [PATCH 3/3] Re-trigger CI after clean rebase on main No code change: branch is already based on the latest origin/main and both adversarial reviewers APPROVED. The prior review bounce was caused solely by unrelated flaky Quickstart external-service (rpc/horizon/friendbot) container bring-up timeouts, not this docs-only diff. Empty commit to trigger a fresh CI run. Refs #3844 Co-authored-by: Claude Code