Conversation
…, D10) Records the InboxWriter/GrafanaBox file restructuring (T00-D9, with exact T01/T02 handoff instructions), the runtime-only outcome validation finding (T00-D10) and its T02 warning, the three post-review correctness fixes (double1 default, hot.kind, stack-frame query stripping) with real revert-check evidence for each (including a genuine control run for the probe-methodology claim this file previously stated without one), the faro-web-sdk dependency now actually added, and the final verify numbers (1236 tests, 1233 pass, both dry-runs and the presence gate still green). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ault, harden index check
Review-round fixes on top of the T01 phase 1 spike:
- Use vertamedia-clickhouse-datasource ("the Altinity plugin" per ADR-0041
SA), not grafana-clickhouse-datasource: Cloudflare's own Analytics Engine
+ Grafana docs name this exact plugin id and its single-custom-header
auth shape (verified: a real ClickHouse HTTP endpoint 403s a bare
`Bearer` scheme, so the two plugins are not interchangeable here). Header
name AND value are now both env-driven (local: X-ClickHouse-User/-Key;
production, phase 2: a single Authorization header) since local
ClickHouse and the AE SQL API need genuinely different auth shapes from
the same static image.
- Disable [auth.basic] and [security] disable_initial_admin_creation:
admin:admin basic auth was a second, untested way into a box meant to
trust only the auth.proxy header. Verified 401 where it was 200 before.
- compose.yml WAKE_ID no longer defaults to "local-dev": a later SIGKILL
under the same convenience default would find a previous clean run's
marker and read as clean. Every caller must set O11Y_WAKE_ID explicitly
now; stop_grace_period raised to stay ahead of O11Y_STOP_GRACE_SECONDS.
- shutdown.sh's index-upload check no longer diffs the bare `index/`
prefix: R2's ListObjectsV2 caps a listing at 1000 keys in lexicographic
order, and this bucket accumulates index objects for up to 90 days, so an
unpaginated whole-prefix listing would eventually stop seeing new
(highest-sorting) keys and every stop would read as unclean. It now reads
this instance's own uploader name from /loki/tsdb-index/uploader/name
before stopping Loki and searches only today's/yesterday's single-day
table prefix for an object bearing it.
- loki-config.filesystem.yaml is now actually COPYd into the image
(STORAGE=filesystem previously failed on a missing file); boot-verified
to /ready 200 and pinned by the same §B.4 assertions as the S3 config.
- stop-roundtrip.mjs adds a SIGKILL data-loss negative control (recreate
the box, assert the canary line the SIGKILL'd wake pushed is genuinely
absent) and fixes a real bug where every `up -d --force-recreate` call
was missing the `box` service argument and recreated the whole stack.
- pipeline/o11y-box-config.test.mjs: added mutation-evidence coverage for
19 more pinned keys (25 total; full table in the report), fixed two tests
that had a same-substring-elsewhere blind spot (compose.yml's Grafana
port default, the Dockerfile filesystem-config COPY pin), pinned the new
auth.basic/disable_initial_admin_creation keys and the
r2-lifecycle-rules.json shape (also stripped its `_comment` field — an
unknown top-level key risks the real R2 lifecycle API rejecting the
document).
- Dropped the redundant chown -R on /var/lib/grafana (it was
copy-on-write-duplicating the whole plugin tree, ~865 MB -> 1.22 GB
uncompressed for one RUN layer); final image is 989 MB / 203.3 MB
compressed.
Full details, the 25-case mutation table and re-verified measurements are
in .superpowers/sdd/README/T01-report.md (outside this repo's tracked
tree).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ask checks (T00) - scrub.ts: fix ScrubbableFaroItem/Payload/Meta/StackFrame and ScrubbableOtlpRecord against the real @grafana/faro-web-sdk types (now a real dependency in apps/authoring) via a temporary typecheck probe, not just source reading. Two real mismatches found and fixed: `type` must be `string`, not a literal union (Faro's own `type` is the string enum TransportItemType, which TS does not consider assignable to an unrelated literal union); none of the nested shapes may carry a `[key: string]: unknown` index signature (a real TransportItem's nested types have none, and TS requires the source type to also have a matching index signature when the target declares one). A concrete TransportItem<LogEvent> now passes into and back out of scrubTelemetry with zero casts; the one remaining friction (Faro's fully-generic BeforeSendHook, TraceEvent included — traces are never exported, ADR §C.4) needs one documented cast at that boundary, which is expected, documented in scrub.ts for T06. - convert.ts: faroItemToRecord now throws on an item.type outside attrs.ts's new HOT_KINDS closed set (exception/log/event/measurement) — "trace" is a real TransportItemType value Faro allows but this contract does not. Reachable from client-controlled /telemetry/collect input, the same T00-D10 catch obligation toAePoint already puts on T02. - sink.ts: clickhouseSink's timestamp column, cross-checked read-only against T01's already-committed containers/o11y/local/clickhouse-init.sql (T01 is running in parallel). Column names matched exactly (T00-D2 held), but T01's `timestamp` is DateTime64(3) while this sink was sending a bare Unix-seconds integer, which DateTime64 reads as whole seconds — not malformed, but silently truncating the millisecond precision the column exists to hold. Fixed: sends 'YYYY-MM-DD HH:MM:SS.sss' via the new exported clickhouseTimestamp, ClickHouse's own default DateTime64 text format. - Ran `pnpm install --frozen-lockfile` (CI's install mode) — clean. Five new/updated pipeline test cases cover all of the above, each with a real revert-check. Task file's Outcome (T00-D9 extended, T00-D10 extended, new T00-D11) and the T00 report updated separately. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Records the scrubTelemetry real-Faro-type probe findings, the T01 cross-check on the ClickHouse timestamp column, the extended HOT_KINDS guard on faroItemToRecord, the explicit files-outside-Owns-row justification for box.ts/inbox/writer.ts, the DurableObjectNamespace typing deviation from the controller's literal wording (with the brand- constraint reasoning), frozen-lockfile verification, and final numbers (1240 tests, 1237 pass, both dry-runs and the presence gate still green). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…use fix, brand-constraint proof (T00) - scrub.ts: apply redactPreviewHosts to every string in a scrubbed record, not only the five fields the targeted rules named. The Scope line says "redactPreviewHosts on every string" — an allowlisted attribute value (session.id, the client-supplied hot.framework) or payload.type could still carry a preview host through untouched. Added a final, idempotent deep-walk over every string leaf on both branches, after the targeted rules (order does not matter since redactPreviewHosts is a pure regex replace). Two new isolated cases prove it on fields no targeted rule names. - sink.ts: clickhouseTimestamp fixed a second time, this time against measurement instead of another guess. Spun up a throwaway clickhouse/clickhouse-server:24.10-alpine container (T01's own pinned tag) on port 4123, applied T01's actual DDL unmodified, and inserted test rows through the same HTTP path this sink uses. Found: a bare Unix-seconds integer is not read as seconds at all — ClickHouse reads a plain integer into DateTime64(3) as raw milliseconds at the column's declared scale, landing in 1970 (the v1→v2 fix's "read as whole seconds" comment was itself a wrong guess). The v2 fix (a formatted 'YYYY-MM-DD HH:MM:SS.sss' string) round-tripped exactly under the container's UTC timezone, but the same string under a session_timezone=Asia/Tokyo override came back 9 hours off — not safe without pinning a server timezone nothing here pins. v3, a raw epoch-millisecond integer, round-tripped byte-for-byte identical via toUnixTimestamp64Milli and is timezone-independent by construction (a pure tick count). clickhouseTimestamp now sends v3. Container torn down after. - attrs.ts: added HOT_KINDS, and telemetry-contract.test.mjs now pins it against §3's "hot.kind (the Faro item kind: exception, log, event, measurement)" parenthetical, the same doc-parses-itself rule as every other closed set. - Verified the DurableObjectNamespace<InboxWriterApi> deviation (T00-D9) with a real probe instead of asserting it: TS2344, "Type 'InboxWriterApi' does not satisfy the constraint 'DurableObjectBranded'" — the exact compiler error is now in the task file's Outcome instead of a claim that a probe was "unnecessary." Five new/updated pipeline test cases, each with a real revert-check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Records the blanket redactPreviewHosts fix, the three-iteration measured ClickHouse timestamp fix (with the real toUnixTimestamp64Milli round-trip numbers and the timezone-dependence finding), the real TS2344 compiler error backing the DurableObjectNamespace deviation, the HOT_KINDS contract-test pin, and final verify numbers (1242 tests, 1239 pass, all 79 telemetry cases green, both dry-runs and the presence gate still green, no leftover docker containers). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…order (T00) - sink.ts: clickhouseSink authenticates and rejects on non-2xx. Measured against a real container (T01's compose.yml requires CLICKHOUSE_PASSWORD): with no credentials sent, every insert answered 403, but the sink only checked whether fetch() itself threw, so writeDataPoint resolved anyway — SELECT count() on the table read back 0, a silent local-mode metrics blackout. Added user/password options sent as X-ClickHouse-User/-Key (T02 passes env.AE_SQL_TOKEN); every non-2xx response now rejects with the status and the first 200 bytes of the body. Verified end to end against the real container: with credentials, the row lands with the exact blob/double values built. - convert.ts: corrected T00-D6 — the scrub/convert order is not symmetric. A LiteBeaconPayload does not typecheck as scrubTelemetry's argument at all (confirmed with a probe: TS2345, neither Faro- nor OTLP-shaped), so a beacon must convert first, then scrub the resulting record — the opposite order from a Faro item, which scrubs first because scrubTelemetry only sees the richer Faro shape that way. - scrub.ts: redactStringsDeep now redacts object keys too, not only values — a MeasurementEvent's metric-name keys reach the OTLP body verbatim via faroBody's JSON.stringify(payload.values), a value-only walk never touches them. Six new/updated pipeline test cases, each with a real revert-check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Records the corrected T00-D6 (asymmetric scrub/convert order, beacon does not typecheck as scrubTelemetry's argument), the clickhouseSink auth/rejection fix measured against a real container, the key-redaction fix, and final numbers (1248 tests, 1245 pass, all 85 telemetry cases green). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tlp label set T01 phase 1 review, fix round 1 (reviewer: opus). Addresses: C1 (critical, fails open): the index-upload check only asked "does an uploader-named object exist after Loki exits?" The TSDB shipper also uploads on its own ~15-minute schedule while Loki runs, so on any wake at least that long, an object from an earlier, successful mid-wake upload can already be there — that check would pass even if the FINAL, shutdown-time upload silently failed. Fixed: shutdown.sh now snapshots uploader-named keys under both day prefixes BEFORE sending SIGTERM and requires a genuinely NEW key after Loki exits. Proven both ways: a new negative control in stop-roundtrip.mjs seeds a fake uploader-named "earlier upload" and runs the box against a MinIO user whose policy denies PutObject on index/* only (approximating the production bucket-scoped token) -- the fixed check correctly refuses the marker; the same scenario run by hand against the reverted pre-fix logic incorrectly WRITES one (revert evidence, full log in the report). I1: withdrew the T01-D1 claim only where re-verification contradicted it. `[live] max_connections=0` DOES set `liveEnabled: false` in /api/frontend/settings, confirmed on 11.4.0 -- Grafana's own frontend never opens a Live socket, so the T03 idle-tab retry concern is dropped. But a direct Centrifuge protocol connect (not just the HTTP upgrade) still fully succeeds regardless of this setting, reproduced again over both Basic auth and a real cookie session -- so the phase-2 box.ts /api/live/* 404 stays required, not optional. grafana.ini and the test comment now say both things precisely instead of one. I2: pinned auth_enabled: true on both Loki configs (flipping it merges both tenants into `fake`) and the datasource uids / X-Scope-OrgID pairing in datasources.yaml (T09 depends on the uids surviving every wake's disposable sqlite state). P1: Loki has a built-in default OTLP resource-attribute promotion list (service.namespace, deployment.environment, k8s.*, ...) that applies on top of attributes_config unless disabled -- confirmed via `loki -help` and a real push carrying two of those defaults. Added resource_attributes.ignore_defaults: true and switched the test from subset-plus-exclusions to an exact-set assertion; added hot.kind to the never-a-label list. P2: dropped the standalone AE_SQL_TOKEN env var from compose.yml's box service -- nothing ever read it directly, only O11Y_CLICKHOUSE_HEADER2_VALUE (which already carries the same value) is wired into the datasource. Minors fixed while in these files: [auth.anonymous] was already pinned (M3); GF_* check is now an exact-allowlist, not a denylist (M4); the port test checks every published port line, not just >=4 (M5, plus the 8123 ClickHouse default); reject_old_samples: true is now pinned alongside its max_age (M6); a `\Z` in a JS RegExp (not a real escape) is now `$(?![\s\S])` (M7); the Dockerfile secret check catches Dockerfile's space-separated ENV/ARG form too, not just `KEY=` (M8); a duplicate sub-path test was removed (M10); stop_grace_period's coupling to O11Y_STOP_GRACE_SECONDS is now documented, including that the 30s default is only validated against this repo's own light local test traffic, not production chunk sizes (M12); round-trip line bodies now embed RUN_ID and the script tears its own stack down when done, O11Y_ROUNDTRIP_KEEP=1 to keep it for debugging (M13). Also fixed two more test blind spots found while writing this round's own mutation-evidence cases (26 -> 33 total, all re-verified fail-when-mutated after the fix): a datasource-uid pin using a trailing \b that still matched a renamed/suffixed uid, and the port-count test not checking that EVERY published port line matches the env-overridable shape. Full details, logs and the revert-evidence transcript are in .superpowers/sdd/README/T01-report.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…arker Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds engine-specific timing hooks to SandpackRuntime (onCompileTiming, onCompileError, onBundlerUnreachable) and ContainerRuntime (onSessionStart, onHmr), in the same style as the existing onProgress/onStderr extension points — neither runtime imports the telemetry facade. Adds apps/authoring/src/telemetry/metrics.ts implementing the observability contract §5 browser metric catalogue (preview.ready_ms, sandpack.compile_ms/ compile_error/bundler_unreachable, session.start_ms, hmr.roundtrip_ms, version.switch, bucket.resolve_ms) against an injected Telemetry — T06's facade (apps/authoring/src/telemetry/index.ts) is not created or imported here, per the controller's phase-1 scope. Tests: pipeline/browser-metrics.test.mjs (fake runtime + recordingTelemetry, 21 cases) plus real-runtime extensions to sandpack-reload.test.mjs (+4) and session-start-failure.test.mjs (+10). Every new guard confirmed to fail when reverted (see the T07 task file Outcome). App.tsx, apps/authoring/src/telemetry/index.ts and e2e/telemetry-metrics.spec.ts are deliberately out of scope for this phase (T06 owns App.tsx's reporting regions and lands after this). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Seven provisioned Grafana dashboards (Runner overview, Tier-2 sessions, Tier-1 playground, Version health, Docs embeds, AI assist, Observability self) covering every ADR-0041 §F.2 metric outside Cost (T13) and Examples & features (T12), backed by a synthetic seed generator (scripts/o11y-seed.mjs) that writes every contract §5 metric to local ClickHouse and OTLP log records to both Loki tenants. pipeline/o11y-dashboards.test.mjs lints every panel: only Analytics Engine's *documented* SQL functions (quantileExactWeighted, not the ClickHouse-only quantileTDigestWeighted an earlier draft used), only assigned contract columns, the SUM(_sample_interval * double1) reading rule, only contract Loki labels with a named tenant datasource, and no Grafana alert rule anywhere. Each rule is proven against a real committed dashboard (mutate, fail, revert), not just synthetic input. Advisor review caught two real seed-generator bugs (double1 silently zeroed for most metrics; surface/tier fixed per-metric instead of per-point), the ClickHouse-only quantile function, a hardcoded local environment default that would blank every production panel, a static framework variable that would drop real frameworks, and a datasource- resolution gap in the lint itself — all fixed and re-verified end to end against a rebuilt local stack. See T09 Outcome for deltas and concerns. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wires ADR-0041 into the API worker: one structured JSON line + one api.request Analytics Engine point per non-proxy request (nothing on the preview proxy path); session.start/session.end/container.boot_ms/chat.answer/ chat.edit/theme.ai/import.url/payload.boot metrics; the */5 pool.gauge/ budget.gauge cron tick alongside the unchanged nightly one, with a marked one-line hook for T04's watchdog; the Sentry scope switch (SENTRY_SCOPE) classifying every diagnostic vs. uncaught capture site. Cross-task fix: packages/runtime/src/telemetry/facade.ts minted its noopTelemetry page-load id in a module-top-level IIFE, which crashes any real Worker importing the barrel (workerd disallows async/random I/O in global scope) — T05 is the first real consumer outside a throwaway typecheck probe. Fixed to mint lazily. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
One facade (apps/authoring/src/telemetry/) backed by Faro (contract §6): local gate (§10), Faro init with errors+web-vitals instrumentations only, session tracking off, scrubTelemetry wired into beforeSend, session.id set to the page-load id. reportError, reportRuntimeError's Tier-1/Tier-2 branches and the versions-fetch diagnostic now go through the facade unconditionally and gate their existing Sentry call on VITE_SENTRY_SCOPE (contract §11/ADR §E.3, sentryScope.ts). reportDemoEvent leaves Sentry entirely and becomes preview.runtime_error counts via a new pure demoEventReport.ts decision module (ADR §E.1). Sentry.ErrorBoundary tees a render crash to Faro too. Every API fetch site carries x-hot-session via apiHeaders(). /telemetry dev proxy added. Known gap (T06-D1, documented in the task file and kept red in e2e/telemetry-faro.spec.ts on purpose): packages/runtime/src/telemetry's attribute allowlist (T00-owned) drops the bare `handled` key before a Faro item leaves the browser, so context.handled="true" never reaches the wire. Flagged for the controller — fixing it means extending an allowlist and the contract doc, outside this task's Owns rows. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…xWriter (T02)
Implements ADR-0041 §B.1-B.2, B.5-B.6, C.1-C.2, E.4, F.1: router.ts
(COMMON.md interface 2), gates/ (host+env, bot, size, rate limit,
x-o11y-secret, GitHub OIDC, Sentry HMAC, Access JWT), normalise/ (Faro
TransportBody unpacking, OTLP JSON+protobuf decode, hashing before
timestamp stamping, no clamp on OTLP, §3 resource-attribute defaults,
extra query-string/UA body-text scrub), and the real InboxWriter DO
(dedupe, fingerprint registry, row-chunked storage, pack/commit,
recordWake). Wires real handlers for /telemetry/{collect,v1/logs,deploy,
hooks/sentry} into index.ts; lite/grafana/* stay 501 for their owning
tasks.
Fixes a real global-scope bug in packages/runtime/src/telemetry/facade.ts
(T00's file, outside this task's Owns row) that failed the whole o11y
Worker's boot under a real wrangler dev: noopTelemetry minted its
pageLoadId eagerly via crypto.randomUUID() at module scope, which
workerd disallows. Made lazy; regression test added.
49 new pipeline/o11y-*.test.mjs cases plus a facade regression test, all
passing; hand-built OTLP (JSON+protobuf) and Faro fixtures; a wrangler-dev
replay script; a sandbox probe (blocked on a missing API scope for
Workers Observability Destinations, documented and cleaned up) plus a
local grafana/loki container check confirming exit criterion 15's label
set for the Faro source. Full details, every T02-D deviation, and revert
evidence in runner/tasks/o11y/T02-o11y-ingest.md's Outcome section.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e (T01 phase 2)
box.ts: wake(reason) is the only start path -- mints a wakeId, persists it
durably (survives a DO eviction), calls InboxWriter.recordWake before
start() (fails closed on rejection), latches its in-flight promise
synchronously so two back-to-back calls collapse into one start (caught a
real race in pipeline/o11y-box.test.mjs before it shipped -- checking
getState() before latching left a window where both calls could mint a
wakeId). containerFetch is overridden to refuse /api/live/* and every
percent-encoded/double-slash/websocket-upgrade variant with 404 before
touching the container (the controller's I1 ruling, kept required after
re-verifying with a real Centrifuge protocol frame -- a raw client still
gets a full connection regardless of max_connections), and to refuse with
503 rather than auto-start when not running (the base class's own
containerFetch auto-starts on any request, the compose.yml WAKE_ID bug one
layer up). isReady() checks real HTTP 200s, not just "didn't throw".
onStop records exactly what the platform reported and claims nothing about
cleanliness. envVars are rebuilt from scratch every start, fail closed
without LOKI_S3_*/CLOUDFLARE_ACCOUNT_ID, never include SLACK_WEBHOOK_URL,
and the only GF_* var is GF_SERVER_ROOT_URL.
wrangler.jsonc: added the containers block (standard-1, EU jurisdiction,
image path found via wrangler 4.136.3's own config-schema.json) and
vars.CLOUDFLARE_ACCOUNT_ID (box.ts needs the account id at runtime; a
Worker has no other way to read it -- T01-D4).
pipeline/o11y-box.test.mjs (new): drives the real GrafanaBox under
node --test via a structural @cloudflare/containers stub, 14 cases.
pipeline/o11y-box-config.test.mjs: +2 tests pinning the containers block
and CLOUDFLARE_ACCOUNT_ID via a zero-dependency JSONC comment stripper.
Sandbox probe (e17e41cc82bda15dfa63960aa172fb87, throwaway
wrangler.probe.jsonc + probe-index.ts, neither committed -- contents in
the report): cold start 46.5s worst of 5 (threshold 90s); idle-tab stop at
1059s with zero requests during the wait; EU placement confirmed (colo
mxp04, Milan); destroy() (SIGKILL) reports onStop identically to a clean
stop ({exitCode:0, reason:"exit"}), empirically confirming ADR-0041's own
claim that onStop cannot distinguish a host loss from a clean exit;
platform's own SIGTERM->SIGKILL grace is 15 minutes (Cloudflare docs), far
above this box's ~30-45s internal grace; amd64 compressed image 212.9 MB;
R2 lifecycle rules file applies and reads back correctly on the real
bucket. Criterion 1 not reproven with a production-scoped R2 token
(T01-D7): minting one failed with 9109 Unauthorized on both the Cloudflare
MCP tool's grant and wrangler's own broader OAuth token -- recorded rather
than substituted with a broad key. All sandbox resources deleted and
confirmed gone (Worker, container app, registry image, R2 bucket, 3
secrets); no token was created so none to delete.
Task file Status: done, Outcome filled.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
I1: allTargets()/aeTargetsOf() now also lint dashboard.templating.list[]
AE queries (environment/framework variables), previously invisible to
the lint entirely. Added DISTINCT to the keyword allowlist.
I2: new per-dashboard test pins each ht_major variable's options against
the real HT_MAJORS export (attrs.ts) instead of a hand-duplicated copy
in all 7 dashboards.
I3: Tier-2 sessions' "Pool gauge (live/builder) vs cap" panel now
actually selects double8 (cap) as a second target in the same panel,
via a new timeseriesPanel({queries}) multi-target mode.
I4: added api.request 5xx-rate/p95-duration panels to Runner overview
and reconcile.run rate/usd-drift panels to Observability self, per
controller ruling.
C-D3: deploy annotation body realigned to the controller's ruled shape
({"event":"deploy","service":...,"sha":...,"cf_version_id":...}, no
"actor") in scripts/o11y-seed.mjs; T09-D3 updated in the Outcome.
All four findings have revert evidence against real committed
dashboards (mutate, fail, revert, re-pass), re-verified live against a
rebuilt local stack (screenshots), and the full pipeline/
o11y-dashboards.test.mjs (47/47) + pnpm test (1305/1308, 1 known
baseline failure) suites pass via rtk proxy.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, cronStep capture, facade boot safety Review found three fixes with no test that goes red on revert: - reportDiagnostic's SENTRY_SCOPE gate: reportDiagnostic now takes an injectable capture function; pipeline/api-telemetry-diagnostic.test.mjs drives it directly with a recorder (1 call under full, 0 under uncaught). - cronStep's explicit ungated Sentry capture (T05-D8): extracted from index.ts into telemetry/cron-step.ts with an injectable capture, so pipeline/api-telemetry-cron-step.test.mjs can assert a throwing step is actually captured, not just that it doesn't rethrow. - facade.ts's lazy id mint: pipeline/telemetry-facade-boot-safety.test.mjs stubs globalThis.crypto.randomUUID before importing the module and asserts the import itself never calls it — the prior noop test only pinned the post-import contract, which passes identically whether the mint is eager or lazy. Each test was confirmed red with its fix reverted, then restored. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…k fail-closed edges (T01 phase 2 fix round) C1 (critical): wake() during a "stopping" container state fell through to mint a second wakeId and call InboxWriter.recordWake again -- marking the still-draining wake `over: true` in the ledger before its own marker exists, while @cloudflare/containers' start() may not even restart a mid-shutdown process or deliver the new WAKE_ID to it; the eventual onStop for the OLD process would then be tagged with the NEW wakeId. Fixed: wake() now refuses while "stopping", leaving the caller (T03's future waking page, which already polls/refreshes) to retry rather than corrupting the ledger. I2: recordWake ran before buildEnvVars validated required secrets, so a missing-secret throw left the ledger believing a wake had started that never would. Fixed: buildEnvVars runs first (a pure, synchronous step) -- before the storage write and before recordWake. I3: the live-path block had two fail-open edges. A malformed percent-escape made normalizedPathname fall back to the raw, still-undecoded path, which then typically did NOT match the regex (e.g. "%6Cive%ZZ" contains no literal "live") -- fixed to return null and fail closed. A requestOrUrl argument containerFetch couldn't construct into a Request skipped the block entirely via `request && ...` -- fixed to `!request || ...`. Also made the regex case-insensitive. I4: LOKI_S3_BUCKET was an unconditionally hardcoded production bucket name, which the sandbox probe silently inherited -- its container therefore targeted a bucket that was never provisioned on the sandbox account, confounding the criterion-12 fail-closed observation (credential rejection vs. a nonexistent bucket produce the identical symptom). Fixed: box.ts now reads env.LOKI_S3_BUCKET, falling back to the production name only when unset. Outcome's T01-D7 corrected with this caveat; T01-D9 records the fix. pipeline/o11y-box.test.mjs: +6 tests (C1's stopping-state refusal, I2's inboxWriterCalls-stays-zero assertion on the existing missing-secrets test, I3's malformed-escape/case-insensitive/null-Request cases, I4's env override). Every new/changed assertion verified by hand to fail against the pre-fix code (revert evidence in the report), then restored byte-identical. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…box probe Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ze-reason points Addresses the controller's review findings: I1: gates/oidc.ts now also checks the GitHub OIDC token's `workflow_ref` claim against a new `GITHUB_OIDC_WORKFLOW_REF` env var (ADR §B.5 names "issuer, audience, repository, workflow" — the workflow check was missing). Also fixes a real test-validity bug the revert check for I1 surfaced: the module-level JWKS cache in oidc.ts was keyed by a constant issuer string, so multiple gate tests signing different RSA keys in one process silently reused a stale cached key set and failed verification for the wrong reason; added `_resetGithubJwksCacheForTests` and call it per signed token. I2: normalise/faro.ts now drops a record over INBOX_RECORD_MAX_BYTES (256 KB) the same way the OTLP path already did — pack.ts's row-chunking assumed normalise enforced this on every path, but the Faro path did not. I3: oversize drops on both paths now write an `o11y.ingest` point with reason="size" via a new normalise/respond.ts#recordOversizeDrop, instead of being folded into reason="invalid_item" (a real observability gap: a query for reason="size" would have seen nothing). Removes workers/o11y/wrangler.probe.jsonc from the tree per the controller (probe configs are throwaway and should not be committed; it also carried a plaintext probe secret). Re-ran the previously-blocked sandbox probe with a properly-scoped Workers Observability API token: created a real export destination pointed at a throwaway raw-capture Worker, captured real Cloudflare OTLP log export bodies (gzip-encoded JSON, never protobuf in this sample), observed batch sizes and the forced-5xx exporter retry behaviour, and found two real gaps fixed here too: - the ray id arrives as `cloudflare.ray_id`, not the contract's `cf.ray` — normalise/otlp.ts now remaps it before hoisting attributes; - Cloudflare's own automatic export never sends `service.version` at all — normalise/points.ts now defaults it to "unknown", matching the other seven §3 resource attributes it already defaulted. Verified exit criterion 15's label set against a real (scrubbed) captured body through a local Loki container. All sandbox resources (destination, two probe Workers, two R2 buckets) deleted and confirmed absent afterward. Every new/changed assertion has revert-evidence (reverted, confirmed red for the right reason, restored, confirmed green) — see the task file's Outcome "Fix round" section for the full list and the exact probe transcript. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ix round D1)
packages/runtime/src/telemetry/attrs.ts's ALLOWED_ATTRIBUTE_KEYS only
covered dotted OTLP resource attributes and the four structured-metadata
keys, so a Faro item's bare `handled` key (contract §6's error.handled
split) was stripped by the browser-side beforeSend scrub before the
request ever left the browser -- confirmed with a live capture
(context: {} where { handled: "true", context: "versions-fetch" } was
sent).
Adds a third, flat/non-dotted category, DIAGNOSTIC_TAG_KEYS: `handled`,
`context`, `sentry_event_id` (the ADR SS E.2 tee), and the
versions-fetch diagnostic's own tags (`versions_fetch_attempts`,
`versions_fetch_outcome`, `versions_fetch_elapsed_bucket`,
`versions_fetch_online`, `api_base_origin`, `net_effective_type`).
Every entry is a boolean flag, an opaque platform id, an enum/bucketed
value, or a reporting call site's own name -- never user or request
content (controller ruling). Distinct from STRUCTURED_METADATA_KEYS:
not hoisted to a resource attribute or structured metadata by
convert.ts#hoistAttributes -- this only decides whether the scrub keeps
the key at all.
docs/observability-contract.md SS3 gains a "Diagnostic tags" paragraph;
pipeline/telemetry-contract.test.mjs gains a parsing test pinning doc
and module together (reverted with a bogus 5th tag, seen red, restored).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…try e2e proof I1 (controller ruling: ADR SS E.3 is binding over the task file's "leave Sentry" line). reportDemoEvent now follows the scope switch like every other diagnostic instead of leaving Sentry unconditionally: - full (default): today's pre-T06 Sentry behaviour restored byte for byte -- captureException/captureMessage/addBreadcrumb, the tier2Report.ts classification, and the DEMO_SURFACE beforeSend re-homing (also restored). - uncaught: facade only; the re-homing branch is simply unreachable, since nothing tagged DEMO_SURFACE is ever sent to Sentry in that scope. - always: one preview.runtime_error count to the facade, from a new reportDemoEventUnguarded split out of the monitorDemos gate. I2. New scripts/check-telemetry-leak.mjs (pnpm check:telemetry-leak, next to check:compiler-chunk). Greps a production dist/ for the local telemetry path's sentinels -- CrashProbe's strings, the raw VITE_TELEMETRY_LOCAL name, and I3's two new test hooks. Fails on a VITE_TELEMETRY_LOCAL=1 build, passes on a plain one -- both measured. I3. Acceptance says "an uncaught error reaches Sentry (transport spy)", untestable against the real production gate. Adds a second, e2e-only Sentry.init() (localTestSentryEnabled(), the same build-time+host gate as CrashProbe, mutually exclusive with the real production init) that captures every envelope on window.__t06SentryCapture instead of sending it, plus window.__t06ReportDemoEvent to drive reportDemoEvent without a real preview mount. Six new e2e tests across two describe blocks/ports/dists (the second built with VITE_SENTRY_SCOPE=uncaught) prove, per scope: uncaught always reaches Sentry; reportError and demo-runtime reach it only under full. Building I3 surfaced a real bug this commit also fixes: diagnosticsGoToSentry was still gated on reportingEnabled directly (production-host only), so even with a local Sentry client listening, reportError/reportDemoEvent's Sentry calls never fired locally. Fixed by gating on a new sentryActive (reportingEnabled || localTestSentryEnabled()) instead -- reverted and seen red (0 Sentry events captured), then restored. Task file Outcome updated: T06-D1 fixed (own commit, see fix(contract)), D2 narrowed (I3's hook now proves the demo-runtime Sentry-reach question live, the ladder-fingerprint claim stays unit-only), D3 formalised into the I2 script, D4/D5 superseded by I1's restore. The spec's former KNOWN RED case is green; Status stays done. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…vidence Fills in the "Fix round" section: I1/I2/I3 fixes, the controller's wrangler.probe.jsonc removal, and the full transcript of the re-run sandbox probe (real captured Cloudflare OTLP export bodies, two new contract-conformance findings fixed, exit criterion 15 re-confirmed against real data, full resource cleanup list, and an honest account of a credential-handling mistake made and corrected during the probe). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…the metric never sets (F24)
All 708 bucket.resolve_ms points have empty blob6 (framework)/blob7
(ht_major): the §5 registry row for this metric lists only
`bucket, outcome` (packages/runtime/src/telemetry/metrics.ts's own
"bucket.resolve_ms": { blobs: ["bucket", "outcome"], ... }), never
framework/ht_major -- the docs-bucket resolve fires before a framework is
chosen (the manifest fetch precedes picking a framework-specific example
within it), and the shared emitBucketResolve() helper both call sites use
takes only { bucket, outcome, durationMs }. Both Tier-1 "by bucket" panels
filtered `blob6 IN (...) AND blob7 IN (...)` anyway, so no row ever
matched -- an empty panel, not a query error (the local ClickHouse shim
accepts the columns same as the real one).
Fix matches the contract as it already stands: dropped the two filters
from both panel queries, rather than adding framework/ht_major to the
metric (which the docs-bucket call site genuinely cannot supply, and
which would need a §5 contract edit this task does not otherwise call
for).
Also adds a 4th dashboard-lint rule (pipeline/o11y-dashboards.test.mjs):
every blobN an AE panel query filters on must be one its referenced
metric(s) actually set per §5 (translated through the same METRICS/
AE_COLUMNS the runtime package's own toAePoint uses) -- this is exactly
the class of bug F24 was, and the local shim's looser column acceptance
is why rule 1 (column existence) couldn't have caught it. Scoped to the
WHERE clause only, so a SELECT-list value expression like
`sum(... * (blob8 = 'error'))` is never mistaken for a row filter, and a
multi-metric `index1 IN (...)` query requires the filtered blob to be set
by every named metric. Revert-checked against the real dashboard (putting
blob6/blob7 back reproduces the exact F24 failure) and against 6 synthetic
cases (fails on the old query shape, passes the fixed one, ignores the
SELECT-list shape, catches the multi-metric case). Ran the new rule over
all 9 dashboards -- no other panel trips it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… locally so fixture replay works (F21)
workers/o11y/.dev.vars.example declares O11Y_EXPORT_SECRET and
SENTRY_HOOK_SECRET empty by design, and bootstrapDevVars only ever patches
a FRESH file -- so a .dev.vars bootstrapped before this fix (the common
case: it's the finding's own repro) stays empty forever. With both empty,
scripts/o11y-replay-fixtures.mjs 401s on all 12 OTLP/deploy/Sentry
fixtures ({"error":"secret"}/{"error":"hmac"}), and dev.mjs --replay hits
the same wall -- the worker-tenant and Sentry panels can never fill on a
local run.
Fix: fillEmptyDevVarsSecrets() (dev-lib.mjs) fills any of these two keys
still declared empty with a fresh ephemeral value (ephemeralSecret(), the
same non-"real"-secret 32-byte hex O11Y_SESSION_SECRET already uses) --
called on EVERY dev.mjs --tier=full run, not only a fresh bootstrap
(unlike bootstrapDevVars's own patch), and unlike O11Y_SESSION_SECRET's
own handling, persisted into .dev.vars rather than injected fresh via
--var every run: these two only gate local fixture replay, so a stable
value across restarts is what lets the standalone
`node scripts/o11y-replay-fixtures.mjs` command work with no running
dev.mjs process to inherit a --var from. Never touches a key that already
holds a real value. Only the two key NAMES are logged, never the value.
o11y-replay-fixtures.mjs now resolves both secrets via
resolveReplaySecret(): the environment first (covers dev.mjs --replay,
which inherits its own env as a child process), falling back to reading
workers/o11y/.dev.vars directly (covers the fully standalone invocation
the finding's own repro line names).
docs/run-and-deploy.md's ".dev.vars bootstrap" section updated to match
(was: "deliberately left empty... stay fail-closed until you paste a real
value in").
Tests: pipeline/dev-script.test.mjs -- fillEmptyDevVarsSecrets (fills
empty declared lines including on a pre-existing file, never touches a
real value, no-op on a missing file, generates a real 32-byte-hex value)
and resolveReplaySecret (env wins, falls back to .dev.vars, empty when
neither, wired against a real readDevVarsLine call). Revert-checked: with
dev-lib.mjs reverted, the whole spec file fails to import
(O11Y_DEVVARS_AUTOFILL_SECRET_KEYS/fillEmptyDevVarsSecrets/
resolveReplaySecret undefined).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…, bucket.resolve_ms panel filters, local replay secrets Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…LLM throw requestTheme's fetch() to LiteLLM was never wrapped, so a connection failure (DNS, refused, reset — a raw TypeError, not a ChatUnavailableError) fell past the `instanceof ChatUnavailableError` guard straight to `throw err`, reaching the generic fetch catch-all with no theme.ai point at all. Contract §5 promises a point on every outcome, error included. Emit unconditionally in the catch, before the instanceof branch decides the response. Test: pipeline/theme-ai-network-error.test.mjs, driven through the real worker route with a stubbed network-level fetch rejection. Verified failing against the unfixed catch (0 points captured) and passing against the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
POST /api/session and POST /api/session/:id/file both called request.json() with no .catch(), so an unparseable body threw a raw SyntaxError past the handler into the generic fetch catch-all — a 500 on ordinary client garbage that pollutes the api.request 5xx rate and the api-5xx-rate alert. Both routes already had a shape check that answers 400 for a non-plain-record body (isPlainRecord / validateFileWrite's InvalidFilePathError); the fix is the same one-line .catch(() => null) /api/theme and /api/chat already use, so a parse failure now reaches that existing check instead of throwing. Pre-existing on master; minimal fix scoped to the two request.json() sites in the session route family (the only public, unauthenticated ones — every other unguarded site in index.ts sits behind authenticate()/authenticate Service(), a different failure shape out of this fix's scope). Test: pipeline/session-malformed-json.test.mjs, driven through the real worker router. Verified failing (500) against the unfixed code and passing (400) with the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
withSnapshotBuildPoint's emitted point never carried a bytes value on either the inline or detached build path, so contract §5's snapshot.build.bytes field stayed permanently absent regardless of artifact size. withSnapshotBuildPoint now hands its callback an addBytes accumulator; createDemo/updateDemo call it with the real byte length of every object written to R2 — a fresh build's own output (contentsByteLength, a new TextEncoder-based helper alongside the existing Uint8Array case) or a build_cache hit's copied objects (R2Object.size) — the only place either branch has that number in hand. Left at 0 on a failed outcome (nothing finished writing) and on the "identical code already built here" branch (nothing is written at all). Test: pipeline/snapshot-build-bytes.test.mjs, driving createDemo/updateDemo directly (companion to F25's snapshot-build-point.test.mjs, which proved the point fires but never checked this field) with a scripted sandbox stub returning known, differently-sized file contents. Verified failing (bytes always 0) against the unfixed code and passing with the fix; a failed build is asserted to never report a positive byte count. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ssing demo
/share/<bad-id> showed "entry file /index.html not found in example files"
instead of anything about a missing demo. Root cause: on a failed GET
/api/demos/:id/source, the loader set a friendly errorMessage and flipped
sourceLoaded true, but never called loadWorkspace — so files/entry stayed at
their toPlaceholderEntry value (entry.entry set, files: {}). sourceLoaded
flipping true is what un-gates rendering EditorShell, whose preview-mount
effect then ran against that still-empty, inconsistent placeholder and threw
its own "entry file … not found" error, which overwrote the friendly message
before anything ever rendered it.
Adds a demoNotFound short-circuit (same pattern docsNotFound already uses
for the docs-example loader) that renders before EditorShell — and hence
before the mount effect — on a 404/410. The 404 vs 410 distinction comes
from the metadata fetch (GET /api/demos/:id?view=share), since the source
route collapses "never existed" and "revoked" to the same 404. Reset on
every effect run so a stale demoNotFound from a previous id (a fork landing
on a fresh one) can't keep shadowing a demo that resolves fine.
Pre-existing on master.
Test: e2e/share-not-found.spec.ts (deterministic, stubs both API calls, runs
against a local vite preview — no live backend needed). Verified failing
against the unfixed App.tsx (timed out waiting for either message) and
passing with the fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…aunch smoke - Post-deploy smoke item 1: the malformed POST /api/session probe no longer exercises the fetch catch-all now that malformed JSON there is a 400 by design (previous commit). Replace it with a malformed PATCH /api/demos/:id for a demo you own — auth and the ownership check both run before the body is parsed, so it throws past them with nothing written, same as the old probe's intent. Updates the item-1 cross-reference in the symbolication criterion (item 7) to match. - Rollback: add an unverified-behaviour caveat to "drop the export destinations" — wrangler.jsonc's observability.logs.destinations still names o11y-logs after that step, and whether wrangler deploy rejects a destinations entry naming a destination that no longer exists has not been checked against a real deploy. Says so explicitly rather than asserting either way, and gives the safe ordering (remove the wrangler.jsonc entry before or together with deleting the destination). - New post-deploy smoke item 8: exit criterion 13 (R2/Loki retention) and the AE SQL rollup, both checked against real production data rather than only the sandbox probe (item 4) — the production Loki bucket's lifecycle rules and object ages per prefix, a real Grafana AE-datasource panel, and the nightly example_daily D1 rollup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Dashboards are provisioned read-only, so no legitimate proxied request is anywhere near this size. Returns 413 before buffering when Content-Length alone already exceeds the cap, and enforces the same cap while reading (cancelling the reader on overflow) when Content-Length is absent or lies small. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
isReady() re-probes Loki/Grafana on every tick, including the whole SIGTERM->exit window after this instance has already asked the container to stop -- getState() cannot see that in flight (stop() never flips its status), so every probe during that window logged the library's own "not listening" line. GrafanaBox now tracks, per wakeId, whether it has requested a stop (a stop() override that covers the base class's own idle-timeout path too), and isReady() short-circuits on it without touching the container. F6 (body release), F13 (probe timeout, wedge reset) are untouched. Also threads the "reopen" reason (already applied on the success path, fix round B-M5) through to the error point a throwing drain step writes -- takeReopenedFlag is one-shot, so the outer catch used to report the wake's own backlog/visit reason instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e Faro dedupe hash The dedupe hash left out the raw measurement/event type (faroBody()'s measurement case stringifies only `values`, dropping `type`), the AE-only hot.* attributes (never hoisted into a stored record's attributes), and Faro's own meta.session.id (a batch-level field faroItemToRecord never reads) -- two different records arriving in the same millisecond could hash identically and lose one to dedupe. F18 made measurements AE-only, but they are still hashed for dedupe, so this applies there too. hash.ts's PreHashRecord gains an optional `extra` field that every other caller leaves unset, so their hash output is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
/telemetry/lite still stored a Loki record for every web-vital beacon from /d and /embed. Applies the same storeRecord = false pattern F18 used for Faro measurements: AE only, dedupe and accounting kept (a record-less IngestItem still gets a real dedupe hash). An error beacon (t: "err") is unaffected -- it still stores its record. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…endent of Slack The fire-once Slack line already names the failing rule id(s) (alertEvalErrorRule's own detail), but that line was the only place it ever went -- with no SLACK_WEBHOOK_URL (local dev, or a webhook outage), slackPoster is a silent no-op and nothing else recorded which rule failed. runAlerts now persists the failing rule id(s) and detail to InboxWriter.alertMeta on every firing tick, independent of Slack, and never erases it on resolve, so a later read still shows what was last wrong. Fire-once/resolve-once and Slack's mrkdwn escaping are unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
"Top error fingerprints by count" -> "...by report count" and "Error rate (uncaught + handled)" -> "Error report rate (uncaught + handled)" -- Faro's global dedupe (F12) already means error.uncaught/handled counts are reports, not occurrences (the wording is already in the observability contract); the panel titles did not say so. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…n a cache hit A build_cache hit's copy loop (createDemo/updateDemo) copies every object under the cached prefix, including __source.json (and a detached create's __job.json) — private files, not the built artifact, and never served (serveDemoAsset refuses any __-prefixed segment). The previous commit's addBytes call counted all of them, so "the built artifact's total size" on the cache-hit path — likely the most common production path — included another demo's private source snapshot. Excludes any __-prefixed path segment from the byte count, matching the same rule the serve path already enforces. Test: pipeline/snapshot-build-bytes.test.mjs, new case seeding a cache-hit copy with both a real artifact file and an oversized __source.json, and overriding env.ARTIFACTS.list for just this test (the shared fakeR2.list() stub always answers empty, a pre-existing harness limitation left alone). Verified failing (counted the __source.json bytes too) against the code before this commit and passing with the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ed e2e helper - docs/run-and-deploy.md: the item-8 edit had deleted the "Flipping SENTRY_SCOPE / VITE_SENTRY_SCOPE to uncaught" heading — restored. Softened two unverified/overstated claims: the example_daily row-count expectation (the rollup only writes groups with an event, so a quiet day adds none) and the R2 dashboard sort-order claim (never verified that feature exists). Trimmed the rollback caveat and dropped the unverified "Logpush destination" term — the doc only ever calls it an export destination. - pipeline/session-malformed-json.test.mjs: bare makeEnv() sends the route's api.request point through the local-ClickHouse-HTTP sink, a real network call to :8123 that only failed silently because nothing is listening in this environment. Routes it through an in-memory RUNNER_EVENTS sink instead (same fix snapshot-build-point.test.mjs's own helper already uses). Other pre-existing router specs likely have the same leak; not touched here. - e2e/share-not-found.spec.ts: reuses e2e/helpers.ts's stubShell() instead of re-declaring its three routes locally, per the runner-playwright-e2e skill's first rule. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8 tasks
…ted record, not just stored ones (F28) Since 2167e83 (F18) made a Faro measurement AE-only, and A-I4's remainder did the same for example.* events, handleCollect's own accepted/duplicate counters were still gated on `ingestItem.record !== undefined` (the old NB3 rationale: "this panel tracks stored-record volume"). Once measurements became ~99% of real /telemetry/collect traffic, that gate starved the Observability-self "o11y.ingest rate by outcome" panel almost entirely — a Round 6 traffic run showed 1,946 collect 204s and 4,031 AE points written, but only 6 accepted self-metric points. respondIngested's own contract ("a batch with N accepted records writes one accepted point, count=N") and contract §5's o11y.ingest row name no narrower "stored-record-only" definition, so the fix drops the record gate: every hash InboxWriter.ingest reports as accepted/duplicate now counts toward the self-metric, whether it produced a stored row or only an AE point. AE point writing itself was never gated on `record` and is unaffected. /telemetry/lite's own accepted/duplicate counting never had this gate to begin with, so a first-time AE-only web-vital beacon already counted correctly there — added a route-level test to pin that. Also files F29: local rate-limit key falls back to cf-connecting-ip ?? "unknown", so every local browser shares one 100-per-60s bucket — one line added to the local-dev section of run-and-deploy.md (no code change, production has real per-visitor IPs). Route-level tests through the real worker.fetch (pipeline/o11y-routes.test.mjs, pipeline/lite-beacon.test.mjs): a measurement-only batch, a mixed batch (stored + AE-only + duplicate + oversize), and the two pre-existing tests that pinned the old (wrong) behaviour, updated to the new counts. Each new/ changed collect-route test was confirmed failing with workers/o11y/src/index.ts reverted to base (8c0a070) before this fix was reapplied. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…y mutation (F28 follow-up) Advisor review of 9e43d5a found two test gaps: 1. The old NB3 gate skipped BOTH the accepted and the duplicate counter for an AE-only item — a redelivered measurement-only batch used to write no o11y.ingest point at all. Only the accepted side had a test. Added a route-level test asserting the second delivery of a measurement-only batch writes an o11y.ingest duplicate point (count=1); confirmed it fails with workers/o11y/src/index.ts reverted to base (8c0a070), alongside the four F28 tests from the prior commit (5/5 fail as expected, all other collect-route tests unaffected). 2. lite.ts's own accepted/duplicate counting never had the collect route's bug (no record-presence gate to begin with), so reverting index.ts can never fail the new lite.ts pinning test — that was never a real revert-check. Proved it a different way: temporarily reintroduced the collect bug's exact shape in lite.ts (`if (item.record !== undefined) ...`), confirmed 3 lite-beacon.test.mjs tests then failed (including the new F28 one), and restored the file from git (unmodified — the fix commit never touched lite.ts). Also tightened that test to assert count === 1, not just presence. Fixed a real bug hit while writing test #2: a static top-level `import … from "@handsontable/demo-runtime/telemetry"` in lite-beacon.test.mjs resolves before this file's `register()` call installs the custom loader hook that specifier needs (o11y-worker-hooks.mjs's resolve()), unlike every other pipeline spec's dynamic `await import(...)` placed after register(). Converted to the same dynamic-import pattern. Full suite: 2225/2225 pass (was 2224; net +1 test — the duplicate-batch test is new, the lite mutation check left no test behind). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ocal rate-limit note Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aro record per collapsed error (F26, F10) F26: typing one throwing line into a Tier-1 editor relayed one preview.runtime_error per half-typed prefix (~20 per line; a 30-min traffic run gave 415 points for 10 edits). The facade side of reportDemoEvent now goes through an edit-burst collapse (demoEventCollapse.ts): an edit that re-runs the preview discards the previous run's reports, and 2 s after the last edit the last run's reports are emitted once per fingerprint. Errors outside a burst (first load, an interaction) still count at once. The collapse sits before the Sentry relay budgets, which stay exactly as they were, so a ladder no longer spends the budget that the metric used to share. F10 Loki: each collapsed report (except console-warn) is also one handled Faro exception whose value is the §7 fingerprint shape (new fingerprintShape export), with no stack. The "Recent demo-runtime errors" panels now read it with `| hot_kind="exception"`. E2E ports in telemetry-faro.spec.ts are env-overridable so another worktree's port block can run it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…records and local serve.share doubling (F27) F27: one /share/<id> view fetches the ?view=share metadata twice under vite dev (React StrictMode re-runs the load effect) and once in a production build (measured on vite preview with the API stubbed), so the 2:1 serve.share ratio is local-only. Documented, no code change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t-load counting (F26) The reset case in the collapse tests went through an edit, which already clears the counted set, so dropping reset's own clear passed unnoticed. Also drops emit()'s redundant counted check (report() owns it; a held key cannot already be counted). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…te the Tier-2 limit (F26) Only a console.error carrying an Error shares the throw's key (the reporter re-homes it onto the error channel, DEV-2552). A React 18 boundary log or prose console line is a different fingerprint and counts separately. Tier-2 rebuilds outlast the 2 s settle window, so a superseded rebuild's report can still count after the burst closes. Refactor-only: comment and contract wording, no behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… records, serve.share dev doubling note Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… error-rate panel R6B made collapsed demo-runtime errors emit error.handled with surface=demo-runtime; the app-health panel would otherwise count errors inside users' previews. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…orkerd (F30) source-map-js 1.2.1 builds its comparator-specialised quickSort with `new Function(...)` (lib/quick-sort.js:111) on the first originalPositionFor. workerd forbids code generation from strings, so every lookup in the real Worker threw `EvalError: Code generation from strings disallowed for this context`; the per-frame catch swallowed it and every frame stayed minified. Node allows `new Function`, so every unit test stayed green. Captured live under wrangler dev: the 6.88 MB map was read from the maps bucket, then every frame's lookup threw that EvalError. - symbolicate.ts: resolve with @jridgewell/trace-mapping (no code generation; already in the lockfile). source-map-js moves to devDependencies (tests build maps with its SourceMapGenerator). - Skip signal: one `o11y.symbolicate.skip` JSON line per map key whose frames were attempted and left unresolved (no_map, fetch_error, parse_error, over_budget, lookup_error, no_frames_matched), at most 20 keys per call plus a suppressed count; injectable via deps.onSkip. - Render-time source normalisation (normaliseSourcePath): strips leading ../ and cuts a leaked checkout path at `runner/<workspace root>`, so a CI build renders src/... and packages/...; not done at build time because the same maps go to Sentry, whose grouping keys on frame filenames. - A page-level frame (`http://host/:303:30`) no longer fetches `sourcemaps/<sha>/.map`. - New pipeline test runs the real drainBatch + symbolicateResourceLogs over a real minified vite bundle and map in a child Node process with --disallow-code-generation-from-strings (workerd's policy), asserting the policy is really on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…new Function is blocked in workerd), skip-reason log, source path normalisation Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
The runner reports faults through Sentry and nothing else. There are no traces, no metric history and no browser performance data, and every number the team looks at is a D1 counter rendered by
/admin. This PR implements ADR-0041 (observability on Cloudflare) and ADR-0042 (example analytics). The shared names, Analytics Engine slots and payload shapes are frozen inrunner/docs/observability-contract.md.What ships:
handsontable-demos-o11y, a new Worker (runner/workers/o11y/) that owns/telemetry/*and/grafana/*. The ingest routes arecollect(Faro),lite(beacon),v1/logs(Cloudflare OTLP export),deploy(CI) andhooks/sentry, and each one is gated: host/env, bot filter, size and item caps, rate limit,x-o11y-secret, GitHub OIDC, Sentry HMAC, or (for/grafana/*) a signed session cookie from the Handsontable login broker. Records are normalised to OTLP, scrubbed, hashed for dedupe and committed to theInboxWriterDurable Object before the 2xx. They are then packed into EU R2 objects per tenant while the box sleeps.runner/containers/o11y/,GrafanaBoxContainer,standard-1, EU). It wakes only for a backlog or a signed-in Grafana visit. A ledgered drain pushes inbox objects to Loki, and a stop protocol writes a clean-shutdown marker only after the TSDB index is really uploaded. An unclean stop re-opens and replays the wake's keys.runner_events), written by the API worker and by the o11y worker at ingest. The alert rules are evaluated outside the box on a*/10cron, with fire-once/resolve-once state and a Slack notifier. The API worker's*/5cron adds pool and budget gauges and a watchdog on the o11y heartbeat.session.*,container.boot_ms,snapshot.build,chat.*,serve.*and more.observabilityblock, withinvocation_logs: falseand logs exported to theo11y-logsdestination.x-hot-sessionon every API fetch, and a local-only telemetry path with a post-build leak check./dand/embed(errors, plus web vitals sampled at 10%), under a 2 KB payload.example.*events,example_daily(D1 migrations0008and0009, the latter adding thedownloadedcolumn) with a nightly rollup, and the Examples & features dashboard.cf.ray/session.id/demo-id search, error panels, Sentry issue events, and the top error fingerprints. Signed-in users also get Grafana Explore for ad-hoc LogQL; dashboards stay provisioned and read-only. Every emitted metric now has a panel.e2e-o11y-localworkflow runs the Docker-backed local end-to-end specs. It triggers on demand, nightly, and on PRs that touch the o11y paths. A path-gateddeploy-o11yjob is ordered beforedeploy-api, and source maps uploaded to Sentry and to the maps bucket with a bucket-scoped key. Deploy events use GitHub OIDC. The leak checks are wired into CI./adminuses; no Cloudflare Access application. The o11y Worker verifies the broker token once and issues its own signed__Host-session cookie. The session lasts no longer than the broker token, and a request that isn't signed in never wakes the box.pnpm dev(Tier 1),pnpm dev:live(plus the API worker, Tier-2 containers and local D1 migrations),pnpm dev:full(plus the o11y worker, compose, telemetry wiring and a local Slack capture server). Local Loki and ClickHouse data persists across restarts;--freshwipes all local o11y state together. The script adopts an already-migrated local D1, pre-pulls container base images, and stops with a clear message if a pull fails.node scripts/dev.mjs --helplists the options.runner/docs/run-and-deploy.md.Sentry stays exactly as it is at merge. Both
SENTRY_SCOPE(API) andVITE_SENTRY_SCOPE(authoring) default tofull, so every report that reaches Sentry today still does. It additionally goes to the new stack. Trimming Sentry to uncaught errors is a later, separate flip with explicit preconditions in the launch plan.Merging deploys the o11y worker (
master.ymlpath gate). The one-time production setup must be done first. It is written up, self-contained, in DEV-3087: EU R2 buckets and lifecycle rules, two bucket-scoped R2 keys, the Analytics Engine token, theo11y-logsexport destination, the Worker secrets (includingO11Y_SESSION_SECRET), the WAF exception for/telemetry/*, the Slack webhook and the Sentry internal integration.ADR-0040 decisions A, B, C.2 and C.3 are superseded (C.1 stands). ADR-0043 (
/adminreads in Grafana) is not part of this PR; it follows after a week of production data.Types of changes
runner/) changeHow was this verified?
Local suite on the branch head:
pnpm test: 2237/2237 pass, 0 failures, 0 todo.pnpm -r run typecheck,pnpm --filter @handsontable/demo-authoring build,pnpm check:telemetry-leakandpnpm check:compiler-chunkall pass.wrangler deploy --dry-runpasses for the o11y, API and authoring Workers.node scripts/check-test-presence.mjs masterpasses.actionlintis clean on all three changed workflows, at the version CI's reviewdog bundles. The workflows' actions are bumped off the retirednode20runtime.e2e-o11y-localjobs.Every new test was seen failing with its change reverted.
Local end-to-end. The whole stack ran on localhost: authoring, the API worker, the o11y worker and the box under
composeandwrangler dev, driven with real traffic rather than seed data./dand/embedbeacons, fixture replay including a duplicate delivery, both crons, a watchdog fire and resolve, and a new-fingerprint alert through a Slack capture server.E2E_O11Y_LOCAL=1 pnpm e2e e2e/o11y-local.spec.ts2/2E2E_TELEMETRYFaro, metrics and example-analytics specs greencontainers/o11y/local/stop-roundtrip.mjspasses every check, including a SIGKILL negative control, a failed-listing control and a zero-ingest wakepnpm dev,dev:liveanddev:fulleach started for real, and Ctrl-C leftdocker psclean. Local data survives a restart, and--freshempties it.cf.rayandsession.id. Explore runs LogQL. A Viewer's attempt to save a dashboard is refused (403)./grafana/_o11y/callbackreturn address (302 to Google)Sandbox Cloudflare account. These throwaway probes ran on the PoCs sandbox account, never production, and every resource was deleted afterwards:
count_over_timeboth equal one clean replaymxp04)hot.demo_id,session.idandcf.rayare structured metadata onlyCriteria 3, 4 and 11 pass locally. The real Cloudflare OTLP export was captured on the sandbox: it is always JSON+gzip, the ray id arrives as
cloudflare.ray_id, and a Worker'sconsole.logJSON arrives as body text, so the ingest now parses it with an anti-spoof strip. The captured bodies were scrubbed and committed as fixtures.Still open, so ADR-0041 and ADR-0042 stay Proposed:
head_sampling_rate) as a pre-launch step.All of these are in the launch plan's post-deploy smoke.
Review. Each task had one task-scoped review and one fix round. The whole branch then had a deep review in four areas: ingest and security, box and durability, API with alerts and CI, and browser, contract and docs. It found 3 critical and 16 important issues, including:
service.nameforgery on the public routesexample_dailyrollup silently deleting daysAll of them were fixed with tests, followed by a scoped re-review, a second fix round, a security review of the broker login, and a final re-review and fix pass. That work covered:
__Host-cookies, key separation and session capping for GrafanaA focused review of the latest fix round, then one more whole-branch review looking only for Critical and High issues, found and fixed:
O11yHeartbeatRPC entrypoint after the heartbeat moved to RPC/telemetry/collectpath, now linear with timing testslineno: 0stack frame that made symbolication throw and blocked the drain queue; a failing key is now rejected on its ownexample.*eventspnpm dev:fullsharing one compose project across worktrees, so--freshin one worktree could delete another's local data/grafana/*proxied through a Durable Object RPC method, which turned every Grafana POST body into an RPC stream and loggedReadableStream received over RPC disconnected prematurelyeach time; it now goes through the box'sfetch()with a buffered bodyIndependent local verification. A separate agent drove the whole local stack with real browser traffic, fault injection and container kills. It found:
start()or the probeso11y.wakenever recording wake-to-ready timepreview.runtime_errorwithoutht_majorandcontainer.boot_ms/session.endwithout a framework, which left their panels emptyLater rounds also found:
pool.gaugecounted every 24-hour meter instead of awake sessionssnapshot.buildwas never emitted on inline builds,serve.sharecounted every API call, and thebucket.resolve_mspanels filtered on columns the metric never sets (a new dashboard-lint rule now catches that)A 30-minute traffic run then found that live typing emitted one
preview.runtime_errorper keystroke prefix (now collapsed to one per edit burst, with a scrubbed demo-runtime Loki line), and that measurement-only requests had stopped counting as accepted ingest.A final pass of small follow-ups added a 10 MB cap on
/grafana/*bodies, a 400 (not 500) for malformed session JSON,theme.aipoints on network failures, lite-beacon vitals sent to Analytics Engine only, and a "demo not found" message on/share.All are fixed with tests, and the kill and reload paths were re-run live. The idle stop was re-tested live: the box stopped after 15.1 idle minutes. It also confirmed replay idempotency (criterion 2, locally), the exact Loki label set (criterion 15) and the privacy canaries.
/adminnow links to Grafana.Launch list. None of these blocks the merge, but each one blocks launch or the
SENTRY_SCOPEflip:SENTRY_SCOPEflip.return_tosuffix allowlist admits the Tier-2 preview hosts. The Grafana login inherits this./ddemos. Decide whether it needs its own host.head_sampling_rate).Checklist
runner/config/frameworks.json(see CONTRIBUTING.md); otherwise it won't appear on demos.handsontable.com. Not applicable: no example added or renamed.pnpm build(andpnpm dev) in the affected example/server-example locally. Not applicable: no example or server-example changed. The runner's own build, typecheck, tests and dry-run deploys are listed above.Related issue(s):
return_toallowlist admits Tier-2 preview hosts