Skip to content

feat(telemetry): accept identifier-free update outcomes - #23

Draft
roboclaw-bot wants to merge 2 commits into
openclaw:mainfrom
roboclaw-bot:openclaw/update-outcome-receiver-20260919
Draft

roboclaw-bot wants to merge 2 commits into
openclaw:mainfrom
roboclaw-bot:openclaw/update-outcome-receiver-20260919

Conversation

@roboclaw-bot

Copy link
Copy Markdown

Summary

Add the receiver companion for identifier-free terminal update outcomes on the existing POST /api/latest-version endpoint. Review only: no merge, deployment, production outcome requests, dataset provisioning, or credential changes.

  • Strict schema-2/event update_result, 18 mandatory flat fields, public version syntax and bounded enums; reject unknown fields and free-form diagnostics.
  • Valid UTF-8, maximum 4096 upload bytes including BOM/whitespace; fixed openclaw-update-result/1 UA isolates invalid outcome uploads from legacy recording.
  • Dedicated UPDATE_RESULTS / openclaw_update_results dataset; fixed outcome-only 503 if missing or a synchronous write fails. Never fall back to the daily dataset.
  • No outcome geography, install/device/account identifiers, raw User-Agent, logs, or arbitrary errors. GET and schema-1 behavior, daily columns, limiter version responses, and disabled Worker logging remain unchanged.
  • Retention, positional storage contract, private aggregate SQL, deployment dependency and statistical limitations documented. No public individual-report route.

Verification

  • npm run check: 13 files, 474 tests passed; deterministic vocabulary and TypeScript checks passed.
  • npx wrangler deploy --dry-run: passed; no deployment.
  • Actual pinned workerd HTTP acceptance/rejection using synthetic shared fixture, isolated outcome/daily sinks, ID/private-version rejection, byte cap and malformed UTF-8.
  • Unit coverage for every required field, extra keys, version labels, BOM/chunked byte limits, unavailable dataset, no geography reads, and limiter regressions.
  • Independent installed AutoReview, local candidate against d57a65267837dc77c9d07e58aa8aae6674fbbae5, P0–P2: scoped-clean before commit; no actionable findings.
  • Fixture test/fixtures/update-result.json SHA-256: 2eb0cffc863c8b3b5335ffcc0a38bcc8495264e8c988647419b328fce0dba754.
  • Tested implementation commit: 39d985d45b7d6bdf70fb2e1a914d18a9d196f6e5; publication marker commit 960dd6597bdb9881bd859ed1c8822cc2c719013f has the identical tree 3c982baaade1cc1a9d6aff34f03b339526bbe635. Both commits include both contributor trailers.

Rollout dependency and limits

A separately authorized rollout must supply the new dataset binding. It has not been provisioned or verified against production. SQL examples were not executed against real data. Version response failure may follow recording; no exactly-once or retry guarantee. Without the fixed UA, undecodable bodies cannot be identified as outcomes and retain legacy invalid-feature behavior; companion clients must always send that UA. Existing malformed schema-2 feature test now uses unsupported schema 3 because schema 2 is reserved for outcomes.

The managed publisher could not push to upstream (roboclaw-bot has read-only upstream access), so this draft uses that same managed identity’s public fork. No alternate credentials or upstream branch changes.

Contributor credit: Json (@fuller-stack-dev) and Peter (@steipete).

Work sessions


View the OpenClaw team session

roboclaw-bot and others added 2 commits September 19, 2026 17:26
Co-authored-by: fuller-stack-dev <263060202+fuller-stack-dev@users.noreply.github.com>
Co-authored-by: steipete <58493+steipete@users.noreply.github.com>
Co-authored-by: fuller-stack-dev <263060202+fuller-stack-dev@users.noreply.github.com>
Co-authored-by: steipete <58493+steipete@users.noreply.github.com>
OpenClaw-Publication: 6f22cf3b-82c5-4ca7-9d1c-c79f01c7dbd7
@clawsweeper

clawsweeper Bot commented Sep 19, 2026

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. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Sep 19, 2026
@clawsweeper

clawsweeper Bot commented Sep 19, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 19, 2026, 1:34 PM ET / 17:34 UTC.

ClawSweeper review

What this changes

The PR adds strictly validated, identifier-free update outcome reports to the existing version endpoint, stores them in a separate dataset, and documents their privacy and storage contract.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

Blocked before merge - 6 items remain

This adds distinct behavior absent from main. Keep it open: production activation conflicts with the stated separate rollout boundary, and moving body reads ahead of the recording limiter introduces a response-stalling regression.

Priority: P2
Reviewed head: 960dd6597bdb9881bd859ed1c8822cc2c719013f
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Recorded reviewer rating: Focused validation and isolation coverage support the implementation, but rollout coupling and the limiter-order regression prevent landing. Real behavior proof remains required by host policy.
Proof confidence 🌊 off-meta tidepool Recorded reviewer rating; Required by policy: the recorded not-applicable assessment does not satisfy the current PR proof policy. Put relevant after-change evidence in the main PR body, then request a fresh review with @clawsweeper re-review. Recorded reviewer context: This is a managed bot publication, so the ordinary contributor-proof gate does not apply. The reported workerd HTTP run exercises the actual receiver's acceptance, rejection and isolated writes; it does not establish production dataset readiness or upgrade deployment safety.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Required by policy Required by policy: the recorded not-applicable assessment does not satisfy the current PR proof policy. Put relevant after-change evidence in the main PR body, then request a fresh review with @clawsweeper re-review. Recorded reviewer context: This is a managed bot publication, so the ordinary contributor-proof gate does not apply. The reported workerd HTTP run exercises the actual receiver's acceptance, rejection and isolated writes; it does not establish production dataset readiness or upgrade deployment safety.
Evidence reviewed 8 items Verified branch scope: The introduced delta contains 13 files. The checkout matches the pinned head, and the implementation and publication commits have identical trees. No AGENTS.md or maintainer-note directory was found.
Distinct capability on this branch: The inspected main endpoint records daily telemetry only; the branch introduces outcome classification, strict validation and a separate writer. The repository's complete pull-request listing contained no replacement outcome receiver, and its release endpoint returned no releases.
Production rollout coupling: The introduced UPDATE_RESULTS binding is in the production Wrangler configuration. The existing workflow runs wrangler deploy after successful checks on pushes to main, with no separate approval job. This conflicts with the PR body and docs reserving production rollout for separate authorization.
Findings 2 actionable findings [P1] Keep the production outcome binding behind rollout approval
[P2] Preserve the quota-exhausted response path before reading uploads
Security None None.

How this fits together

The telemetry Worker receives OpenClaw version checks and returns the latest npm release while recording bounded analytics. This change routes update outcomes through separate validation and storage instead of the daily telemetry dataset.

flowchart TD
  A[Version check or outcome report] --> B[Request classification]
  B --> C[Daily telemetry validation]
  B --> D[Strict outcome validation]
  C --> E[Daily dataset]
  D --> F[Outcome dataset]
  C --> G[Latest version response]
  D --> G
Loading

Decision needed

Question Recommendation
Should landing this receiver activate the production outcome binding, or must that binding remain deferred to a separately approved rollout? Defer production activation: Keep the new binding out of the production configuration until a separately authorized rollout verifies dataset readiness and retention.

Why: The stated rollout boundary and the repository's automatic main deployment disagree; production activation requires explicit owner intent.

Before merge

  • Add real behavior proof - Required by policy: the recorded not-applicable assessment does not satisfy the current PR proof policy. Put relevant after-change evidence in the main PR body, then request a fresh review with @clawsweeper re-review. Recorded reviewer context: This is a managed bot publication, so the ordinary contributor-proof gate does not apply. The reported workerd HTTP run exercises the actual receiver's acceptance, rejection and isolated writes; it does not establish production dataset readiness or upgrade deployment safety.
  • Keep the production outcome binding behind rollout approval (P1) - Adding this binding to the default configuration makes a merge deploy the outcome receiver through .github/workflows/deploy.yml, which runs wrangler deploy on pushes to main. There is no separate activation gate, despite the body and outcome docs reserving production collection for another authorization. Defer this production binding or explicitly gate deployment so landing receiver support cannot activate collection prematurely.
  • Preserve the quota-exhausted response path before reading uploads (P2) - Every POST now awaits the full body before checking the recording limiter. Previously, an exhausted caller skipped body reading and received its version response immediately. A slow or never-ending upload below the byte cap now stalls that response because the reader has no deadline. Preserve the early quota decision without double-consuming the limiter, and cover an exhausted caller with an unfinished body.
  • Resolve merge risk (P1) - Landing the production binding triggers automatic deployment before the separately described rollout approval; production dataset readiness and retention have not been verified.
  • Complete next step (P2) - Resolve production rollout authorization and repair the quota-exhausted upload regression before marking this draft ready to merge.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

  • [P1] Keep the production outcome binding behind rollout approval — wrangler.jsonc:27-29
  • [P2] Preserve the quota-exhausted response path before reading uploads — src/index.ts:125-129
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test growth Production source +106/-12; tests and fixtures +201/-3 The production growth implements a distinct receiver contract with focused validation and isolation coverage.
Storage surface 1 new dataset; 17 blobs, 1 double, 1 index The separate persistent contract needs rollout approval while existing daily columns remain unchanged.

Merge-risk options

Maintainer options:

  1. Separate receiver landing from activation (recommended)
    Remove the outcome binding from the automatically deployed production configuration and retain explicit local test bindings until rollout approval.
  2. Hold for rollout authorization
    Keep the draft paused until an owner approves production collection and confirms the new dataset contract.

Technical review

Best possible solution:

Keep outcome storage isolated and disabled in production until explicitly authorized, while preserving prompt version responses for quota-exhausted clients.

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

Yes, source establishes both triggers: a main push deploys the added binding, and a quota-exhausted POST now waits on an unfinished upload before returning its version response. Neither scenario was executed during this read-only review.

Is this the best way to solve the issue?

The separate dataset and strict validator are a maintainable approach, but production activation must respect the stated rollout boundary and body reading must preserve the existing limiter fast path.

Full review comments:

  • [P1] Keep the production outcome binding behind rollout approval — wrangler.jsonc:27-29
    Adding this binding to the default configuration makes a merge deploy the outcome receiver through .github/workflows/deploy.yml, which runs wrangler deploy on pushes to main. There is no separate activation gate, despite the body and outcome docs reserving production collection for another authorization. Defer this production binding or explicitly gate deployment so landing receiver support cannot activate collection prematurely.
    Confidence: 0.98
  • [P2] Preserve the quota-exhausted response path before reading uploads — src/index.ts:125-129
    Every POST now awaits the full body before checking the recording limiter. Previously, an exhausted caller skipped body reading and received its version response immediately. A slow or never-ending upload below the byte cap now stalls that response because the reader has no deadline. Preserve the early quota decision without double-consuming the limiter, and cover an exhausted caller with an unfinished body.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

  • add P2: This is a bounded telemetry capability with actionable rollout and request-handling concerns, without evidence of a current production emergency.
  • add merge-risk: 🚨 automation: Adding the production binding causes the existing main-push deployment workflow to activate outcome storage without the separately stated rollout step.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp. This is the recorded reviewer rating; real behavior proof remains required by host policy.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Required by policy: the recorded not-applicable assessment does not satisfy the current PR proof policy. Put relevant after-change evidence in the main PR body, then request a fresh review with @clawsweeper re-review. Recorded reviewer context: This is a managed bot publication, so the ordinary contributor-proof gate does not apply. The reported workerd HTTP run exercises the actual receiver's acceptance, rejection and isolated writes; it does not establish production dataset readiness or upgrade deployment safety.

Label justifications:

  • P2: This is a bounded telemetry capability with actionable rollout and request-handling concerns, without evidence of a current production emergency.
  • merge-risk: 🚨 automation: Adding the production binding causes the existing main-push deployment workflow to activate outcome storage without the separately stated rollout step.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp. This is the recorded reviewer rating; real behavior proof remains required by host policy.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Required by policy: the recorded not-applicable assessment does not satisfy the current PR proof policy. Put relevant after-change evidence in the main PR body, then request a fresh review with @clawsweeper re-review. Recorded reviewer context: This is a managed bot publication, so the ordinary contributor-proof gate does not apply. The reported workerd HTTP run exercises the actual receiver's acceptance, rejection and isolated writes; it does not establish production dataset readiness or upgrade deployment safety.

Evidence

What I checked:

  • Verified branch scope: The introduced delta contains 13 files. The checkout matches the pinned head, and the implementation and publication commits have identical trees. No AGENTS.md or maintainer-note directory was found. (960dd6597bdb)
  • Distinct capability on this branch: The inspected main endpoint records daily telemetry only; the branch introduces outcome classification, strict validation and a separate writer. The repository's complete pull-request listing contained no replacement outcome receiver, and its release endpoint returned no releases. (src/index.ts:123, d57a65267837)
  • Production rollout coupling: The introduced UPDATE_RESULTS binding is in the production Wrangler configuration. The existing workflow runs wrangler deploy after successful checks on pushes to main, with no separate approval job. This conflicts with the PR body and docs reserving production rollout for separate authorization. (wrangler.jsonc:28, 960dd6597bdb)
  • Automatic deployment boundary: The deployment job is enabled for non-pull-request events on main and executes the production deploy command. (.github/workflows/deploy.yml:27, 960dd6597bdb)
  • Limiter ordering regression: The branch awaits the complete POST body before either limiter call. Main checks the recording quota before reading the body, allowing exhausted callers to receive a version response without waiting for upload completion. The bounded reader limits bytes but has no read deadline. (src/index.ts:129, 960dd6597bdb)
  • Runtime validation and storage limits: The captured PR body reports a successful workerd HTTP run and 474 passing tests. The inspected harness sends real local HTTP requests through the bundled Worker and checks isolated outcome/daily writes, private-field rejection, oversized bodies and malformed UTF-8. Its Analytics Engine datasets are local; the body explicitly says production binding and SQL were not verified. (test/latest-version-runtime.test.mjs:116, 960dd6597bdb)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

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

  • Separate production activation from receiver landing, or obtain explicit rollout approval with storage compatibility evidence.
  • Restore the quota-exhausted response fast path and add an unfinished-upload regression check.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant