feat(telemetry): one outcome event per install, with nowhere to put a path (backend#1907)

[LukasWodka]

Parent epic: tracebloc/backend#1872 · Ticket: tracebloc/backend#1907 · Contract: the telemetry contract · Sibling: tracebloc/cli#527

The installer is the highest-variance, least-observed step in the product: it
runs on machines we have never seen, under package managers, proxies and shells
we do not control, and it reports to nobody. Each of the backend#736 failures
was invisible until a customer happened to mention it.

scripts/lib/telemetry.sh emits one contract-shaped event per install, from
install_cleanup — the EXIT trap — so it fires on every path including the
interrupted one and the one that dies under errexit. That is where §6.5's
"terminal event on every path" is actually honoured; anywhere else and the
failure rate is uncomputable.

What is emitted

install.run.succeeded / .failed / .cancelled, with:

layer	attribute	notes
record	tracebloc.install.phase	preflight…connect, from the closed a–f set
record	tracebloc.install.phase_<name>_ms	six fixed keys, one per phase
record	tracebloc.install.duration_ms / .exit_code	ints
record	tracebloc.install.client_state	summary.sh's closed diagnosis set
record	tracebloc.install.cli_on_path	the #736 PATH case, as a 1/0
record	error.type	closed vocabulary, failures only (§8.4)
record	tracebloc.install.source / .source_line	basename + line, never the path
resource	service.name / tracebloc.component	installer / install (§10.1)
resource	service.version, os.type, host.arch, deployment.environment, service.instance.id

Ctrl-C (130/143) is cancelled and carries no error.type.

"No arguments, no paths, no data" is a shape, not a rule

Every value goes through one writer, _telemetry_attr, which admits a string
only if it matches ^[A-Za-z0-9._-]{1,64}$ and an integer only if it is one. A
filesystem path contains /. A proxy credential contains : and @. A token
is longer than 64 characters. A person's name contains a space. Values that
fail are dropped, never trimmed or escaped — a value that had to be repaired
to be safe is a value we did not understand, and shipping our guess about it is
how a redactor leaks.

The phase and the client state are additionally checked against their closed
sets at the render boundary. That line exists because of a test failure: a
canary assigned straight to TB_TELEMETRY_PHASE reached the record, and it was
shaped exactly like a legal value, so the token regex waved it through.
telemetry_phase_begin is the only writer and already closes the set — but
"the only writer is careful" is a property that stops being true silently.

TB_ERR_CMD is deliberately never emitted (unexpanded command text is still
free text), and service.instance.id is a fresh per-run value rather than the
hostname — field hostnames here are overwhelmingly <firstname>-macbook, which
§7.3 forbids outright.

The vocabularies are derived, and a required check proves it

scripts/tests/telemetry-vocabulary-agreement.sh holds no copy of any
vocabulary. It parses:

1. install-k8s.sh's step_header calls → the phase letters
2. summary.sh's CLIENT_STATE writers → the client states
3. gen-manifest.sh's FILES array → the source basenames
4. install.sh's immutable-release-tag regex → the service.version shape
5. common.sh's print_help → the documented opt-out variables

and for the error classes — which have no second declaration to parse — it
exercises telemetry_error_class over the full cross-product of its two
closed input sets, checking both that every answer is registered and that
every registered class is reachable. Comparing a declaration to itself would be
self-consistent and therefore blind.

It runs in drift-checks.yaml's Source-of-truth drift job, which is a
required check — a guard in a job nobody must wait for is advice, not a gate.

It found something on its first run:summary.sh's own CLIENT_STATE
docstring listed five states while _diagnose_not_ready produces six
(image_pull_ca, added by #424, never made it into the comment). Fixed here.

Two real bugs the tests caught — both fatal, both invisible to unit tests

Same shape twice: grep -q closes the pipe at its first hit, so under the
installer's set -o pipefail a producer takes SIGPIPE and the pipeline returns
141 — on a successful match. This is the backend#1778 defect, already
documented in summary.sh's own comments.

1. printf '%s' "$key" | grep -qE … in the shape checks.
2. tr -dc 'a-f0-9' < /dev/urandom | head -c 16 for the instance id — this one
   at source time, so it killed the entire installer under set -e before
   it printed a line.

Every unit-level test passed throughout both. The only thing that went red was
the test that drives install_cleanup for real, under the installer's own shell
options. That test stays, and a second one now exercises the whole surface under
set -euo pipefail explicitly.

Mutation proofs

Every guard broken, watched go red, restored — and every anchor asserted to
have applied, because an inert mutation and real coverage are identical in a
log.

mutation	reddened
the phase is trusted at the render boundary	2 tests
CLIENT_STATE passed through unchecked	2 tests
_telemetry_attr sanitises instead of dropping	1 test
the source attribute emits the whole location	1 test
the version shape loosens to the generic token	1 test
the urandom | head SIGPIPE pipeline restored	2 tests
the opt-out is ignored	1 test
the emit-once latch removed	1 test
cancel folded into failed	3 tests
an unknown CLIENT_ENV exported under a guess	1 test
the spool made unbounded	1 test
step_header stops driving the phase clock	2 tests
a 7th install step added with no phase name	agreement guard
summary.sh gains an unknown client state	agreement guard
a script dropped from the source vocabulary	agreement guard
the classifier returns an unregistered answer	agreement guard
a class registered that nothing produces	agreement guard
the version regex drifts from install.sh	agreement guard
the phase parse made inert	exit 2, fail closed
opt-out doc/code drift, both directions	agreement guard
the bootstrap fixture's FILES parse made inert	setup fails closed

Two mutations were rerun after strengthening a test that had stayed green:
CLIENT_STATE and the phase were being tested with a value containing /,
which _telemetry_attr's shape refuses regardless — so the test proved the
shape guard and said nothing about the vocabulary guard. It now uses
degraded / verifying, which are shape-legal and exactly what a future edit
to summary.sh would produce.

One mutation came back inert and is reported as such rather than counted:
putting _telemetry_attr's key check back through a pipe reddens nothing,
because for a short key printf completes before grep -q exits and no SIGPIPE
occurs. The here-string is still the correct idiom (and is what fixes the value
checks, where the input can be long), but this PR does not claim coverage it
does not have.

What remains to be connected

The transport, and only the transport. The ticket said "rides the gateway
and token"; the 17 Aug decision replaced the Collector gateway with an ingest
endpoint on the backend (rfcs#28),
which is tracebloc/backend#1905, in flight and not yet available.

_telemetry_deliver therefore writes the install log and a bounded, 0600
local spool at $HOST_DATA_DIR/telemetry/pending.jsonl (last 50 events, in a
0700 directory) — which tracebloc/backend#1906's forwarder can drain — and posts
nothing. That one function is the whole change when #1905 lands.

The ticket body has been corrected so it no longer says "blocked by the gateway".

Would the three named field failures now be visible?

#736 case	visible?
CLI on ~/.local/bin, PATH advice only printed	Yes.tracebloc.install.cli_on_path=0 rides every event. install-cli.sh has always computed this — it asks whether a fresh login shell resolves tracebloc — and only ever printed advice about it. It becomes a rate.
apt-get hung on the dpkg lock	Yes.tracebloc.install.phase_prerequisites_ms rides the success event too — that failure produces no non-zero exit at all, it just takes twenty minutes. It also rides the cancel and failure events, for the phase that was still running when the process ended, which is the likelier real shape: stuck twenty minutes and then killed. (That second half was broken in the first two commits and is Bugbot finding 3 — see the comments; the phase durations now sum exactly to duration_ms, which is asserted.)
cluster info on the wrong kubeconfig context	Not here — that is the CLI's, and it is covered in tracebloc/cli#527 (with an honest limit stated there: a run that succeeds against a wrong-but-valid context still looks like a success).

Two of the three are this PR's; the third is the sibling's.

Also here

• install-bootstrap.bats held a hand-written second copy of install.sh's
  FILES array, in two places — so adding one lib turned ten unrelated
  supply-chain tests red. It now derives the list from the array (the same parse
  gen-manifest.sh uses) and fails closed on an inert parse. That is a landmine
  removed, not a workaround for this PR.
• scripts/manifest.sha256 regenerated; telemetry.sh added to both FILES
  arrays, which gen-manifest.sh cross-checks.
• --help gains a "Usage reporting" section; the copy golden is regenerated.

Review history

Bugbot found 5 issues across 4 rounds: 4 fixed, 1 demoted with measured
evidence (the mechanism does not exist on bash 3.2 or 5.x). In order of what they
would have cost:

1. the spool's mkdir disarmed the NFS guard — _telemetry_deliver created
   HOST_DATA_DIR from the EXIT trap on the very path where
   early_data_dir_guard had just refused it, and that guard skips an existing
   directory by design. Run 1 refused and created it; run 2 installed MySQL onto
   NFS. Customer database corruption, reintroducing client#432.
2. --help emitted install.run.succeeded — a free success in the denominator
   of the failure rate this ticket exists to produce.
3. the still-running phase had no duration — the dpkg-lock case invisible in
   its likeliest form, and phase_connect_ms absent on every success. This
   undercut the DoD claim in the table above.
4. pre-log failures were reported nowhere — fallout from fixing (1): log is a
   no-op until setup_log_file, which runs after the guards, so the NFS refusal
   itself became permanently invisible. Events now fall back to a mktemp'd file
   in $TMPDIR. My own NFS test had been masking this by setting
   LOG_FILE=/dev/null.
5. source lookup aborts the whole event — demoted; a command substitution in an
   argument position does not propagate its status, measured on bash 3.2 and 5.x.

All four real ones were invisible to my own tests, for the same reason: they
were about which code paths reach the emitter, not about what the emitter does
with its inputs — and one of my tests was actively masking a bug. No unit test of telemetry_render_event can see any of them.
Every replacement test now drives the real install-k8s.sh, install_cleanup,
_assess_handoff, or early_data_dir_guard. Full reproductions and mutation
tables are in the PR comments.

Two of my own mutations came back inert and were fixed by moving the tests end
to end rather than by counting them; a third is reported as inert rather than
counted. Details in the comments.

Test plan

make check green (lint, drift incl. the new guard, helm-lint, helm-vocab);
bats scripts/tests/*.bats green (31 new tests); shellcheck -S warning -x clean on every new file; bash -n clean; gen-manifest.sh --check
current; the mutations above plus 10 more from the review rounds (see comments).

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes installer EXIT-trap behavior and data-dir/spool logic on failure paths (including NFS refusal); mistakes could skew failure metrics or recreate dirs the guard rejects, though transport is local-only and heavily tested.

Overview
Adds scripts/lib/telemetry.sh and wires it through the bash installer so install_cleanup emits one contract-shaped outcome (install.run.succeeded / .failed / .cancelled / .skipped) per committed run, with phase timings, exit code, closed vocabularies for errors/client state, and cli_on_path — values pass a strict shape gate so paths and credentials cannot be recorded. Delivery is log + bounded pending.jsonl spool only until backend ingest exists; opt-out via TRACEBLOC_NO_TELEMETRY / DO_NOT_TRACK.

Integration:telemetry.sh is fetched and manifest-signed; step_header drives the phase clock; telemetry_run_started skips --help/terminal exits; telemetry_run_skipped / telemetry_rerun_handoff distinguish healthy handoffs from real failures vs stray exit 2; GPU reboot exit 2 declares the handoff. --help documents usage reporting.

Guards & tests:telemetry-vocabulary-agreement.sh (required drift job, make drift, shellcheck) derives phases, client states, sources, version regex, and event names from producers; telemetry.bats exercises trap paths, NFS/pre-log/bootstrap TMPDIR fallbacks, and privacy. install-bootstrap.bats parses install.sh’s FILES instead of duplicating the list; summary.sh comment aligns with image_pull_ca; .gitleaks.toml allowlists telemetry canary fixtures.

^{Reviewed by Cursor Bugbot for commit 9a8d62c. Bugbot is set up for automated code reviews on this repo. Configure here.}

[LukasWodka]

bugbot run

[LukasWodka]

Sibling PR for the CLI half of backend#1907: tracebloc/cli#527. Independent — neither depends on the other, and the transport for both is tracebloc/backend#1905.

[LukasWodka]

bugbot run

[LukasWodka]

⚠️quality / gitleaks is red, and it needs one command from a human

What it found: the curl-auth-user rule, on a test fixture in
scripts/tests/telemetry.bats that spelled out curl -u $CANARY:hunter2 …. It
was a canary — the whole point of that test is to prove TB_ERR_CMD never
reaches the record — but the rule is right: a source file containing that
spelling is a finding whatever the surrounding test claims, and a reviewer
scanning a diff cannot tell a canary from the real thing at a glance.

Already fixed in the code (f495186): the fixture now carries a path instead
of a credential, and the credential half of the same assertion is covered by
HTTPS_PROXY, which encodes user:pass in a proxy URL — what a hospital
network actually configures, and what gitleaks does not flag.

Why the check is still red: gitleaks scans the commit range
(3a618e2..f495186, "2 commits scanned"), so the line is still found in the
first commit's diff even though it no longer exists in the tree.

The fix, and why I have not applied it. The remedy is to squash the branch so
the blob never appears in any commit:

git rebase -i --autosquash origin/develop # or: git reset --soft origin/develop && git commit
git push --force-with-lease

I do not force-push without being asked to — that is an explicit rule for AI
sessions in this repo, and it applies to my own branches too.

What I deliberately did NOT do: add this to .gitleaks-baseline.json. That
file is for findings that predate the gate; putting a brand-new one in it to
make my own PR green would be weakening a security check to avoid a rebase.

Everything else on this PR is green.

[LukasWodka]

bugbot run

[LukasWodka]

Bugbot round 1 — one fixed, one demoted with evidence

✅ "Non-install exits report success" (High) — real, fixed

Reproduced exactly as described:

$ HOST_DATA_DIR=$T bash scripts/install-k8s.sh --help
$ cat $T/telemetry/pending.jsonl
{"resource":{…},"attributes":{"event.name":"install.run.succeeded",
"tracebloc.install.phase":"bootstrap","tracebloc.install.exit_code":0,…}}

This is the worst possible bug in this PR and the severity is right: --help is
the command people run most while a real install is broken, and a free success
in the denominator moves the failure rate — the one number the whole ticket
exists to produce — in the direction that hides the problem.

Fix: a run-started latch, not a phase test. main() calls
telemetry_run_started once the terminal commands (--help, --diagnose,
prepare-host) have had their chance to dispatch; telemetry_emit_outcome
returns early without it. A latch rather than "phase != bootstrap" because a
genuine failure in the bootstrap phase — the leftover-data guard,
validate_config — is an install attempt and must still be reported.

Second, related thing found while fixing it: the assess handoff exits 0
having run no step, so counting it as install.run.succeeded would make the
success count grow with re-runs on machines nothing happened to. It now emits
install.run.skipped — a registered outcome verb (contract §6.4), so no new
vocabulary — which also makes "how often do people re-run an installer that was
already done" a question we can ask.

Not covered, deliberately:prepare-host. It is a different command with its
own registry component (§10.1 gives installer the components
install/preflight/upgrade), and reporting it as tracebloc.component=install
would be mislabelling it rather than measuring it.

⚠️ "Source lookup aborts whole event" (Medium) — mechanism does not exist

The predicted mechanism is that the unprotected command substitutions feeding
tracebloc.install.source abort telemetry_render_event under set -e and take
the whole outcome with them. A command substitution in an argument position
does not propagate its status to the enclosing command — only an assignment
(x=$(false)) does. Constructed and measured on bash 3.2.57 (the system
bash on macOS, the version that actually matters here) and on 5.x:

$ TB_ERR_LOC='/home/someone/evil.sh:9' # deliberately not one of our scripts
$ telemetry_emit_outcome 1 # under set -euo pipefail
REACHED_END rc=0 spooled: 1

The event is rendered, the process survives, one line is spooled. Reported as
unreachable rather than fixed — a fix for a path that cannot be taken costs more
than the finding.

But the invariant is worth having, so it is now pinned: an unrecognised
source location drops the field, never the event — asserted through a subshell
under set -euo pipefail, checking the event still renders, still carries
error.type, and carries neither tracebloc.install.source nor the rejected
filename.

Mutation proofs for the new code

Every new guard broken, watched go red, restored, anchor asserted to have
applied:

mutation	reddened
the run-started latch removed	2 tests
the latch set at source time (always on)	1 test
main stops setting the latch (feature dead)	1 test
the skipped handoff reported as succeeded	1 test
assess.sh stops marking the handoff skipped	1 test
a bootstrap failure classified as something else	1 test

Two of those came back inert on the first pass — "main stops setting the
latch" and "assess.sh stops marking the handoff" — because the tests set the
flags themselves and so could not see the wiring disappear. That is the same
class as the bug you found: a unit test of the emitter cannot observe which exits
reach the trap. Both are now driven end to end through the real entrypoint (a
validate_config rejection, which lands after the latch and before step a) and
through the real _assess_handoff with tracebloc mocked. Both mutations redden
now.

26 tests in telemetry.bats, make check green.

[LukasWodka]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 29495f7. Configure here.}

[LukasWodka]

Bugbot round 2 — the third finding was real too, and it hit the DoD claim

I missed this one in my first reply: it came from Bugbot's earlier review pass
and my query only fetched the later one. It is the most consequential of the three.

✅ "Final phase duration never recorded" (Medium) — real, fixed

telemetry_phase_begin only closes a phase when the next one starts, and
nothing closed the active phase before render. Reproduced — a run cancelled after
22 minutes stuck in prerequisites:

"tracebloc.install.phase":"prerequisites"
"tracebloc.install.duration_ms":1330000
"tracebloc.install.phase_preflight_ms":0
← no prerequisites duration at all

This undercut the DoD claim in the PR body. I wrote that per-phase durations
make the dpkg-lock case visible. They did — but only in the version where the
install goes on to succeed. In its likeliest real form the user is stuck twenty
minutes in prerequisites and then kills it or gives up, never reaching step c,
and the number was silently absent. Likewise phase_connect_ms was missing on
every successful install: the readiness wait, up to READY_TIMEOUT (600s),
the single longest phase in the run.

And as you noted, the remainder trick could not recover it, because bootstrap
had no key at all — the loop iterated the letter map, and bootstrap has no step
letter. So download + verify + leftover-guard + assess time was an unnamed
remainder.

My own test passed throughout. "A slow phase is visible on a run that
SUCCEEDED" only ever measured a phase that a laterstep_header had closed —
the exact shape of a test that proves the easy half of its own claim.

Fix: the live delta is added at read time, not by a "close the phase" call
in the emit path, so telemetry_render_event stays idempotent (the tests call it
repeatedly; a render that mutated the accumulators would report different numbers
each time). The loop now iterates _telemetry_phase_names, so bootstrap and
unknown get keys too. The clock is read once per event, so the per-phase
numbers and the total are exactly consistent.

New invariant, asserted:the phase durations sum EXACTLY to duration_ms —
every millisecond of the run attributed to exactly one phase, with the individual
attributions also pinned so a compensating pair of errors cannot pass, and a
counted == 4 anchor so an inert loop cannot.

That test needed a fake clock, and the reason is worth recording: the obvious
fixture is wrong. Winding _TB_TELEMETRY_PHASE_STARTED_MS backwards after the
step_headers have already attributed that time invents milliseconds that never
elapsed. The first version of the test failed for exactly that reason — sum
1200000 against a total of 900000 — and the fixture was the bug, not the code.

Mutation proofs

mutation	reddened
the active phase's live delta dropped (the original bug)	3 tests
the loop reverts to the letter map (bootstrap unattributed)	2 tests
the live delta counted for every phase (double counting)	1 test
the clock read per attribute instead of once	inert — see below

The fourth is reported as inert rather than counted: _telemetry_now_ms has
second resolution (BSD date has no %N, and half the install base is macOS),
so two reads inside the same second are identical and the guard only matters
across a second boundary. It is kept because it is free, not because a test
proves it.

---

Running total across both rounds

3 findings, 2 fixed, 1 demoted with measured evidence. Two of my own
mutations came back inert along the way and were fixed by moving the tests end to
end rather than by counting them.

29 tests in telemetry.bats (up from 20), make check green, the required
Source-of-truth drift guard green.

quality / gitleaks remains red for the history reason documented above and
needs one force-push from a human.

[LukasWodka]

bugbot run

[LukasWodka]

Bugbot round 3 — the most serious one yet, fixed

✅ "Spool mkdir undoes NFS guard" (High) — real, and worse than the first two

The first two findings corrupted a metric. This one corrupted a customer's
database. Reproduced exactly as described:

guard exit=1
HOST_DATA_DIR created on the REJECTED volume? YES
./nfs-volume/.tracebloc/telemetry

_telemetry_deliver ran from the EXIT trap and mkdir -p'd
$HOST_DATA_DIR/telemetry unconditionally — including on the path where
early_data_dir_guard had just refused that directory for being on a network
filesystem and called error. That guard skips an existing directory on
purpose ([[ -d "$target" ]] && return 0 — "an EXISTING data dir has no at-risk
mkdir here", client#441), so:

run 1: guard refuses (dir absent) → error → EXIT trap → telemetry creates it
run 2: guard sees the dir, returns 0 → MySQL installs onto NFS

which is the InnoDB corruption client#432 exists to prevent, reintroduced by the
telemetry that was only supposed to watch.

The principle I got wrong: an observer that changes the install's own
preconditions is not an observer. Adding a write to the EXIT trap put telemetry
upstream of a safety gate, and nothing in the design made me look for that.

Fix: the spool only writes into a data dir that already exists. A run that
dies before then still reports through the install log — which is what a support
bundle collects, and which _choose_log_file has already placed somewhere safe
(falling back to $TMPDIR). The forwarder (#1906) picks up every later run.

Both directions mutation-proven, because "don't write" is trivially satisfied
by "never write":

mutation	reddened
the HOST_DATA_DIR existence check removed (guard disarmed again)	the new test
delivery disabled outright (a "fix" that kills the feature)	4 other tests

The regression test carries an anchor asserting the guard actually refused
(non-zero exit + the "network filesystem" message), so "no directory" cannot
silently mean "the fixture never ran the guard".

---

Final tally across three rounds

4 findings, 3 fixed, 1 demoted with measured evidence. In order of what they
would have cost:

1. spool mkdir disarms the NFS guard → customer database corruption. Fixed.
2. --help emits install.run.succeeded → the failure rate the whole ticket
   exists to produce, moved in the direction that hides problems. Fixed.
3. the still-running phase had no duration → the dpkg-lock case invisible in
   its likeliest real form; phase_connect_ms absent on every success. Fixed.
4. source lookup aborts the whole event → mechanism does not exist on bash
   3.2 or 5.x; demoted with a reproduction attempt, invariant pinned anyway.

Three of those were invisible to my own tests, and the pattern is consistent: all
three were about which code paths reach the emitter, not about what the
emitter does with its inputs. Unit tests of telemetry_render_event cannot see
any of them. Every replacement test now drives the real install-k8s.sh, the real
install_cleanup, the real _assess_handoff, or the real
early_data_dir_guard.

30 tests in telemetry.bats (up from 20). make check green, Source-of-truth drift green, full bats scripts/tests/*.bats green locally.

quality / gitleaks remains red for the commit-history reason documented earlier
and needs one force-push from a human.

[LukasWodka]

bugbot run

[LukasWodka]

Bugbot round 4 — right again, and it was fallout from my own round-3 fix

✅ "Early failures drop outcome events" (High) — real, fixed

This is the sharpest of the four, because it is a consequence of the previous
fix: stopping telemetry from disarming the NFS guard also made the NFS refusal
itself invisible.

Reproduced:

$ early_data_dir_guard # target reads as nfs, LOG_FILE unset as on the real path
guard exit=1
any telemetry written anywhere? 0

_telemetry_deliver's comment claimed "the install log always gets it." It does
not. log is a no-op until setup_log_file sets LOG_FILE, and setup_log_file
runs aftervalidate_config and early_data_dir_guard — deliberately,
because client#432 refuses a network data dir before logging starts. So on
exactly those paths there was no log and (correctly, since the last commit) no
data dir, and the rendered event was dropped on the floor.

Those pre-log failures are also precisely the class the run-started latch was
built to preserve, so losing them quietly undid that work too.

And you were right about the test. My NFS test set LOG_FILE=/dev/null, which
made log appear to succeed and hid the whole thing. It now leaves LOG_FILE
unset, as the real path does, and asserts the refusal is reported. A second
test covers the general pre-log case.

Fix: when there is no data dir, spool to a mktemp'd file in $TMPDIR.
mktemp and not a fixed name — /tmp is world-writable on Linux and the
installer runs privileged steps, so a predictable path is a symlink target for an
append that may be running under sudo; mktemp creates with O_EXCL. This
mirrors _choose_log_file's own fallback, so an early-failure run leaves one
small file beside the install log it already leaves there. #1906's forwarder
reads both locations.

The false comment is replaced with what actually holds and why — a comment that
claims something untrue is itself the defect.

mutation	reddened
the fallback removed (early failures dropped again)	2 tests
the fallback given a predictable shared path	1 test
the fallback's 0600 mode dropped	1 test
the data-dir existence test dropped (NFS guard disarmed again)	2 tests

---

Final tally — 5 findings across 4 rounds

4 fixed, 1 demoted with measured evidence. Every single real one was about
which code paths reach the emitter, never about what the emitter does with its
inputs:

#	finding	would have cost
1	spool mkdir disarms the NFS guard	customer database corruption (client#432 reintroduced)
2	--help emits install.run.succeeded	the failure rate, moved the way that hides problems
3	the still-running phase has no duration	the dpkg-lock case, invisible in its likeliest form
4	pre-log failures reported nowhere	the NFS refusal, permanently invisible
5	source lookup aborts the event	demoted — mechanism absent on bash 3.2 and 5.x

No unit test of telemetry_render_event can see any of 1–4. Every replacement
test now drives the real install-k8s.sh, install_cleanup, _assess_handoff,
or early_data_dir_guard — and one of my own tests had been actively masking a
bug, which is the thing worth remembering out of all of this.

31 tests in telemetry.bats (up from 20). make check green, Source-of-truth drift green.

quality / gitleaks remains red for the commit-history reason documented earlier
and needs one force-push from a human.

[LukasWodka]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 46a33de. Configure here.}

[saadqbal]

Four findings, all about which runs reach the emitter or what gets past the shape guard — nothing wrong with the render logic itself. The $TMPDIR one is the significant one: it quietly undoes the round-4 fix on the primary macOS path.

Two things that don't fit on a line in the diff:

• install.run.skipped will almost never fire on the curl | bash path. install.sh:144exec traceblocs on a healthy machine before install-k8s.sh is fetched at all, so assess.sh's gate only sees direct ./install-k8s.sh re-runs. Fine, just much narrower than "how often do people re-run an installer that was already done".
• quality / gitleaks is still red. Your comment explains it and what it needs; noting it's the one thing blocking merge.

Vocabulary guard and the 31 bats tests are green here on 46a33de.

[LukasWodka]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit eaa848a. Configure here.}

[saadqbal]

All five addressed, and the fixes are better than what I suggested in two places — deriving "is the installer running inside this directory?" instead of asking install.sh to export a second name is the right call for a bootstrap users may have curl'd months ago, and the pwd -P catch on both sides is the kind of thing only re-running the reproduction finds.

Verified on this head rather than taken: the agreement check is green including the new behavioural version corpus and the exit-2 branch; bats scripts/tests/telemetry.bats is 36/36; and I re-checked the four shape guards under /bin/bash 3.2.57 — [[ =~ ]] refuses every embedded-newline input and still admits the legal spellings, and the {1,15} interval works there.

One non-blocking note below on exit 2. quality / gitleaks is still the only red check and still needs the squash from a human.

[LukasWodka]

bugbot run

[saadqbal]

Re-approving — the marker is the right shape and goes further than the note asked for. Declaring at the exit 2 site, clearing at source time so an inherited _TB_TELEMETRY_RERUN_HANDOFF=1 can't pose as a handoff, and failing closed toward failed for an undeclared 2 all land the way round I'd want. unexpected_exit_2 as its own row rather than the phase bucket is better than what I suggested: a stray 2 stays distinguishable from a real prerequisite failure instead of just being counted.

(d4) deriving the site list from grep -n 'exit 2' over the runtime, and failing closed on zero sites with the TRACEBLOC_DOCKER_FIRST_RUN_EXIT precedent named, is the part that will still be true in a year.

Verified on a00b1c9: agreement check green (13 checks, including both exit-2 directions and the site sweep), bats scripts/tests/telemetry.bats 40/40, gpu-nvidia.bats green, gen-manifest.sh --check current, shellcheck -S warning -x clean on the three touched files apart from the pre-existing SC2034 at gpu-nvidia.sh:253. dispatch_gpu_setup is a plain call chain from install_linux, no subshell, so the marker really does survive to the trap — test 30 pins it.

Two follow-ups, neither for this PR: install_cleanup still prints "Re-run required. Complete the step above" for an undeclared 2, so the user gets the message telemetry now correctly refuses to believe — it can key on the same marker. And the dead TRACEBLOC_DOCKER_FIRST_RUN_EXIT read at common.sh:1022 deserves its own ticket now that you've traced its producer to 8c3a3d4.

quality / gitleaks is still the only red check, still needs the squash.

[LukasWodka]

bugbot run

[LukasWodka]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit ccbbbbf. Configure here.}

[saadqbal]

Post-hoc read (this is already on develop as c564a15), and a privacy-focused one: every field the record can carry, traced to its source.

The short version is that the shape gate holds. Nine attributes, all of them either a closed vocabulary checked at the render boundary (event.name, phase, client_state, error.type, source) or a bounded integer (exit_code, duration_ms, cli_on_path, source_line, the eight phase_*_ms). _telemetry_attr is the only writer for all of them, it drops rather than repairs, and [[ =~ ]] closes the newline hole a grep would have left. I could not find a free-text channel: TB_ERR_CMD is never read, TB_ERR_LOC is reduced to a basename that must be in the source vocabulary, and there is no stderr capture anywhere in the file. The bats "derived guard" walks the whole rendered object and requires every value to be int-or-token, which is a much better test than greping for the absence of a canary.

Two things about enforcement rather than shape, neither of them a defect here:

The seven resource fields are printf'd straight into the JSON and do not pass through _telemetry_attr — so the "one writer" argument in the header covers attributes only. Each of the seven is independently constrained today (installer/install constants, three closed case sets, the version regex, a $RANDOM id), so nothing gets past. But there is no allowlist of field names, and the token shape ^[A-Za-z0-9._-]{1,64}$ admits an identifier as readily as it refuses a path: alice, db-01.internal, hospital-vm-03.example.com all match it. What is enforced is "no path, no credential, no whitespace", which is what the title claims; "no identifier" is currently a property of the code's shape rather than of a gate. backend#2165's allowlist is the stronger form if this grows.

Opt-out is honoured on every path that can emit, because telemetry_emit_outcome is the only one and the check sits ahead of both render and deliver. It is documented in --help and the golden — but not in install-k8s.sh's own "Environment variable overrides" header block (where TRACEBLOC_ALLOW_NETWORK_FS, TRACEBLOC_SKIP_LEFTOVER_GUARD etc. live), not in docs/SECURITY.md, and install-k8s.ps1 has no equivalent. Worth a follow-up rather than a change here. The help copy also doesn't mention that a file appears at $HOST_DATA_DIR/telemetry/pending.jsonl and keeps 50 runs; a customer reading "records ONE outcome event per run" wouldn't expect that.

On the parts of the brief that turned out to be moot: there is no transport. No curl, no POST, no endpoint, no token, nothing with a timeout — delivery is log plus a local 0600 spool in a 0700 dir. So the installer cannot block, cannot hang on an air-gapped site, and cannot fail an install on a telemetry error, and there is no -k/--insecure question to ask. Nothing leaves the customer's machine in this PR at all; the review that actually decides "does customer data leave the environment" is #1905/#1906, not this one.

Exactly-once holds: the _TB_TELEMETRY_EMITTED latch is set before any other work in telemetry_emit_outcome, and the run-started latch keeps --help out. Zero-times cases are all deliberate and documented in the code — install.sh's own fetch/manifest/cosign failures (different process, different EXIT trap), --diagnose (clears the trap), prepare-host (swaps it). The outcome is right on the paths that matter, including the two that were wrong earlier in the branch (declared exit 2, and Ctrl-C on the skipped handoff).

Manifest: regenerated correctly. I recomputed all 19 entries against the tree at this head rather than trusting the diff — every one matches, including the three files whose content changed and the new telemetry.sh.

Coordination note for backend#2123, not a defect here: its metric filters key on service.name and deployment.environment, and both live inside the nested resource object in this record, so a filter written as $.service.name won't match — it needs $.resource.service.name. Worth settling before the filters are cut, since nothing emits to CloudWatch until #1905 anyway.

Split is roughly 1050 lines of implementation and wiring against 1830 of tests and drift guards, plus 10 of help copy. The vocabulary-agreement script is the best thing in the PR — deriving the sets from their producers and failing closed on an inert parse is the right shape, and exercising the classifier over its cross-product instead of restating it is genuinely better than a second declaration.

Two nits inline.

(Process note: I did not run /code-review on this one — this session's working directory is the tracebloc-engine checkout, not client, and that skill resolves its target against the cwd, so it would have reviewed the wrong repository's diff. Everything above is from a direct read of origin/pull/747/head.)

[saadqbal]

nit: this is the one field in the record with no length bound — ([.-][A-Za-z0-9.]+)? is open-ended where the token shape is {1,64} — and service.version is also one of the seven resource fields that go straight into the printf rather than through _telemetry_attr, so the "one writer" argument doesn't cover it.

The "install.sh refuses anything that isn't a release tag" reasoning doesn't constrain it either. common.sh:1128 is TB_VERSION="${TB_VERSION:-${TRACEBLOC_INSTALL_REF:-}}", so the caller's own TB_VERSION wins over the ref the bootstrap exported — and on the direct ./install-k8s.sh path (or under TRACEBLOC_ALLOW_UNVERIFIED=1) install.sh's gate never ran at all. TB_VERSION=v1.0.0-alice.hospital.internal ./install-k8s.sh lands verbatim in the version column.

Not a leak — the charset has no /, :, @ or space, so a path or a proxy credential still can't fit — but it's an unbounded, environment-supplied string in a record where everything else is capped. A {1,32} on the tail would close it.

[saadqbal]

nit: this row doesn't test what the comment says. ([.-][A-Za-z0-9.]+)? matches 64 zeros, so v1.9.3-000…0 (71 chars) is admitted by both gates, not refused:

$ RE='^v[0-9]+\.[0-9]+\.[0-9]+([.-][A-Za-z0-9.]+)?$'
$ c="v1.9.3-$(printf '%064d' 0)"; [[ $c =~ $RE ]] && echo admitted
admitted

The check still passes, because it only asserts the two gates agree — but the row is proving "both admit" while the comment reads as a length bound being exercised. Same underlying point as the note on TB_TELEMETRY_VERSION_RE.

[LukasWodka]

/fr-pass

Best-effort functional review passed (triage: merged clean, non-interactive; behavioral evidence limited while e2e journey is red — backend#2206). Advancing to Ready for prod.