Skip to content

feat(telemetry): add aggregate capture pilot - #18

Closed
vincentkoc wants to merge 2 commits into
mainfrom
feat/telemetry-aggregate-backup-pilot-20260910
Closed

feat(telemetry): add aggregate capture pilot#18
vincentkoc wants to merge 2 commits into
mainfrom
feat/telemetry-aggregate-backup-pilot-20260910

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

  • Add npm run telemetry:capture for one explicit closed UTC day, with a
    side-effect-free dry run by default.
  • Capture the existing hourly Analytics Engine query and an independently
    scoped HTTP country query. Preserve sampled estimates, exact SQL/wire
    hashes, receipts, missing coverage, and byte-identical offline regeneration.
  • Integrate current main, including the Worker-health CLI, private-export
    checkout guard, and removal of the public statistics surface.
  • Reject capture output inside the executing telemetry checkout or any linked
    worktree before existing-bundle handling, credentials, writes, or requests.
  • Use the canonical physical-directory helpers to keep verification scratch
    outside the bundle and all telemetry checkouts, including filesystem case
    aliases, before creating a temporary directory.

Safety And Scope

Execution still requires separate operator-provided AE and HTTP read tokens,
explicit account/zone identity, and an operator-selected private output.
There is no credential discovery, refresh, provisioning, or persistence.
Existing complete bundles verify offline without credentials or requests,
including after upstream retention expires. Private destinations and unrelated
private repositories outside telemetry checkouts remain supported.

The approved storage, retention, and operations scope is unchanged; those
decisions are not awaiting further approval. Permanent operator-input and
private-output requirements still apply to each invocation.

Queries, transport, quota/lookback semantics, strict response schemas, and the
CLI interface are unchanged by this repair. Both source contracts must validate
before completion. Partial/conflicting bundles fail without repair, overwrite,
resume, or automatic retries.

This remains a manual aggregate-capture pilot, not a scheduled, row-level,
lossless, version/plugin, or finer-geography backup. HTTP estimates are not
joined to AE reports or converted into users. No Worker runtime, dependency,
schema, upload, storage binding, retention policy, unattended credential, or
backup job is added. Backup automation in
#20 is closed for now and remains
outside this PR. Settled privacy disclosures are unchanged.

Validation

  • Fresh focused capture/history/private-path suites: 128 tests passed.
  • Fresh npm run check on Node.js 24.19.0: vocabulary consistency,
    typechecking, and 461 tests across 13 files passed.
  • Fresh Wrangler 4.129.0 deploy --dry-run: passed.
  • Three targeted regressions passed after demonstrated pre-fix failures:
    capture from source and linked fixtures reached forbidden credential reads;
    a scratch case alias physically inside a bundle incorrectly returned
    unchanged.
  • Real Git fixtures cover source/linked/nested checkouts and available
    filesystem case aliases, successful unrelated private destinations,
    side-effect-free planning, offline reruns, and immutable content/mtime
    snapshots. All payloads are synthetic.
  • Scoped formatter, main-relative whitespace checks, and privacy scan passed.
  • Fresh independent P0-P2 review: scoped-clean, with no actionable findings.
  • Exact-head hosted validation
    passed for c1c57cc626628d99ec17ceb0c78dda86436260ab: Node.js 24
    clean install, full check, and Wrangler dry-run. CodeQL and both Analyze
    checks passed; deployment was intentionally skipped.

Landing Hold

Do not merge yet. Authenticated execution of the exact production queries
with the strict AE/HTTP response schemas, followed by successful offline
verification, has not been completed. Exact-head hosted CI has passed.
Local synthetic proof does not waive this remaining acceptance gate. No raw production
metrics, credentials, or private source identities are published.

@clawsweeper

clawsweeper Bot commented Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 10, 2026
@clawsweeper

clawsweeper Bot commented Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed September 14, 2026, 11:44 PM ET / September 15, 2026, 03:44 UTC (Revision 4).

ClawSweeper review

What this changes

Adds a dry-run-default operator command that captures one closed UTC day of hourly telemetry reports and separate country estimates into a private bundle with offline verification.

Merge readiness

⛔ Blocked before merge - 2 items remain

This remains distinct, useful work beyond current main. No actionable introduced defect was found, but the MEMBER-authored PR retains an explicit authenticated-acceptance landing hold.

Priority: P2
Reviewed head: c1c57cc626628d99ec17ceb0c78dda86436260ab

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A defensively implemented pilot with substantial boundary coverage and no concrete blocking code defect identified.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER author is exempt from ordinary contributor proof. Supplied filesystem and offline-CLI validation uses synthetic Cloudflare responses, so it does not establish authenticated acquisition; the explicit landing hold remains independently binding. No unresolved cross-principal authority change establishes an additional proof gate.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The MEMBER author is exempt from ordinary contributor proof. Supplied filesystem and offline-CLI validation uses synthetic Cloudflare responses, so it does not establish authenticated acquisition; the explicit landing hold remains independently binding. No unresolved cross-principal authority change establishes an additional proof gate.
Evidence reviewed 8 items Repository policy and patch scope: Verified the origin repository. No AGENTS.md or maintainer-notes files were found. The complete seven-file introduced delta adds capture tooling, tests, documentation and a package script; it changes no Worker runtime, dependency pins or workflows. The checkout remained clean.
Still necessary on main: Current main exposes telemetry:history and worker:health, but has neither the capture command nor its implementation. The merged offline exporter processes saved evidence rather than acquiring it.
Release check: GitHub returned no published releases; no release or merged replacement establishes that this capture capability has shipped.
Findings None None.
Security None None.

How this fits together

The telemetry Worker records update-request reports in Cloudflare Analytics Engine. This operator tool reads aggregate reports and HTTP country estimates, then passes saved reports through the existing offline exporter.

flowchart TD
  A[Operator selects UTC day] --> B{Execute or verify?}
  B -->|Neither| C[Dry-run plan]
  B --> D[Validate private destination]
  D -->|New capture| E[Read Cloudflare aggregates]
  D -->|Existing bundle| F[Offline validation and regeneration]
  E --> F
  F --> G[Verified private bundle]
Loading

Before merge

  • Resolve merge risk (P1) - The exact authenticated Cloudflare queries and strict response representations remain unverified; an unsupported production response would stop capture and leave an incomplete bundle.
  • Complete next step (P2) - Complete authenticated execution of the exact production queries and successful offline verification, retain redacted acceptance evidence, and lift the stated landing hold before merge.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test growth Production scripts +902 lines; tests +806/-6 lines, net +800 The production growth implements the stated acquisition, bounded transport, private bundle and offline-verification responsibilities.

Merge-risk options

Maintainer options:

  1. Complete the declared acceptance gate (recommended)
    Retain redacted authenticated capture and offline-verification results before lifting the existing landing hold.

Technical review

Best possible solution:

Keep the manual, private capture boundary and unchanged exporter semantics, with demonstrated account-specific query compatibility and byte-identical offline verification.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this adds an acquisition capability rather than fixing a reported existing-behavior bug; production acceptance remains explicitly uncompleted.

Is this the best way to solve the issue?

Yes, for the approved pilot scope: acquisition reuses the existing exporter and containment helpers without changing their contracts or adding unattended storage.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 635334fe8733.

Labels

Label changes:

  • add merge-risk: 🚨 other: Synthetic validation cannot establish whether the new strict capture contract accepts authenticated production responses.

Label justifications:

  • P2: This is bounded operator tooling with no change to public update delivery or Worker collection.
  • merge-risk: 🚨 other: Synthetic validation cannot establish whether the new strict capture contract accepts authenticated production responses.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The MEMBER author is exempt from ordinary contributor proof. Supplied filesystem and offline-CLI validation uses synthetic Cloudflare responses, so it does not establish authenticated acquisition; the explicit landing hold remains independently binding. No unresolved cross-principal authority change establishes an additional proof gate.

Evidence

What I checked:

  • Repository policy and patch scope: Verified the origin repository. No AGENTS.md or maintainer-notes files were found. The complete seven-file introduced delta adds capture tooling, tests, documentation and a package script; it changes no Worker runtime, dependency pins or workflows. The checkout remained clean. (c1c57cc62662)
  • Still necessary on main: Current main exposes telemetry:history and worker:health, but has neither the capture command nor its implementation. The merged offline exporter processes saved evidence rather than acquiring it. (package.json:16, 635334fe8733)
  • Release check: GitHub returned no published releases; no release or merged replacement establishes that this capture capability has shipped.
  • Private output and credential boundaries: The full capture module was inspected. Checkout rejection precedes existing-bundle handling and credential reads; existing bundles verify offline. New captures require separate explicit tokens, exclusive private output creation, fixed Cloudflare endpoints, bounded responses and validation before completion. (scripts/lib/telemetry-aggregate-capture.mjs:763, c1c57cc62662)
  • Review continuity: GitHub comparison against the previously reviewed head confirms that the capture-module update adds shared checkout guards and physical scratch containment checks without changing queries or transport. The earlier review had no findings. Local historical comparison encountered an unavailable object; the read-only GitHub comparison supplied the relevant patch. (scripts/lib/telemetry-aggregate-capture.mjs:661, c1c57cc62662)
  • Acceptance hold and supplied validation: The captured body at sourceRevision 99b00bef94d90970580329afb788fc1b1077083dacbc4356be944b25a4ae37e9 explicitly states that authenticated production queries and successful offline verification remain incomplete. The live body matches that hold. Storage, retention and operations approval are explicitly settled. Tests mock Cloudflare fetch responses; reported hosted validation passed, but no tests or target code were executed during this read-only review. (test/telemetry-aggregate-capture.test.mjs:207, c1c57cc62662)

Likely related people:

  • Vincent Koc: Raw commit bd314c2 adds scripts/lib/private-export-path.mjs:63 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: bd314c2bab12; files: scripts/lib/private-export-path.mjs)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Complete the stated authenticated capture and offline-rerun acceptance, retaining a transcript with credentials, private identifiers, endpoints and metrics redacted.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (3 earlier review cycles)
  • reviewed 2026-09-10T19:05:07.702Z sha 2a97ccd :: needs maintainer review before merge. :: none
  • reviewed 2026-09-10T19:20:05.895Z sha 2a97ccd :: blocked before merge. :: none
  • reviewed 2026-09-10T21:56:37.687Z sha 2a97ccd :: blocked before merge. :: none

This was referenced Sep 11, 2026
Merged
Merged
@vincentkoc vincentkoc mentioned this pull request Sep 12, 2026
Closed
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. label Sep 15, 2026
@vincentkoc vincentkoc closed this Sep 15, 2026
@vincentkoc
vincentkoc deleted the feat/telemetry-aggregate-backup-pilot-20260910 branch September 15, 2026 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@vincentkoc