Skip to content

fix: bind retired OCE fixtures to source blobs - #1746

Merged
RomneyDa merged 5 commits into
mainfrom
openclaw/revert-oce-fixture-qualifications
Oct 3, 2026
Merged

RomneyDa merged 5 commits into
mainfrom
openclaw/revert-oce-fixture-qualifications

Conversation

@RomneyDa

@RomneyDa RomneyDa commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Summary

  • replace the broad base-role qualification for the two retired OCE URI fixtures with exact whole-source SHA-256 pins
  • admit either fixture only in the known merge-base/head blobs of open OCE PRs that still modify the source file
  • reject arbitrary additions, removals, or context blobs even when the approved fixture line is otherwise unchanged
  • preserve the existing FTP source-pin contract and apply its whole-blob check generically to pinned reviewed attributions

Dependency and compatibility scope

OCE #955 removed the complete credential-like literals from current OCE source. A live inventory on 2026-10-03 found 13 open OCE PRs that still modify tests/conformance/kubernetes-compute.test.mjs:

Their reported base and head files deduplicate to 21 exact source hashes. No role-only or path-only exemption remains, and blobs from open PRs that do not change this file are not included.

Behavior proof

Claim: the production review path admits the retired fixtures on an exact known historical source blob, including when it is the head side of a live PR, while refusing an unlisted source blob.

Reviewed head: 8580477535f0aceeada96cf8e5d46475f08079c6

Environment: macOS, Node 24.16.0, local-only ClawSweeper review with the production native input scanner and current OCE GitHub state; no GitHub mutation or apply command.

Command:

pnpm run review --local-only \
  --target-repo openclaw/openclaw-enterprise \
  --item-number 662 \
  --output-retention summary \
  --result-format json

Observable result:

  • the native scanner emitted agent_input_scan_classified for both retired URI fixtures
  • each event bound the finding to the exact OCE test(review): fix stray reference broken by a rename in the placeholder recovery test #662 head and base blobs with roles head and base
  • the advisory review completed for OCE head de0ab632d71500684a479a8bb15825094a109a1b and actual base 3ab78e1a5c1f702bd1f1b2361959e5c068408dca; no source_not_reviewed refusal occurred
  • the retained report was complete; the proof stream contained hashes, roles, scanner coordinates, and redacted diagnostics, not raw fixture values or credentials

The focused regression independently pins a synthetic URI source, admits the exact blob, then changes an unrelated byte and observes source_not_reviewed. Against pre-fix ff794d1e0ea135eb8d97407d661778e321e87d22, that test fails at policy validation because URI source pins are unsupported; it passes on this head.

Validation

  • pnpm run build
  • node --test test/agent-input-scan-fixtures.test.ts: 487 passed, 0 failed
  • PATH=/opt/homebrew/bin:$PATH pnpm run check: passed with Bash 5.3, including static checks, builds, formatting, lint, changed coverage, and the full coverage suite
  • the earlier system-Bash run's sole failure was the known macOS Bash 3.2 mapfile harness mismatch; its exact test passes under Bash 5.3
  • pre-commit Codex review: both findings applied only to the committed base-role implementation and were resolved by the working-tree source pins
  • committed-range local ClawSweeper review at 858047753: no code findings; its requested native proof is the OCE test(review): fix stray reference broken by a rename in the placeholder recovery test #662 run above

Finding disposition

  • broad base-only qualification: removed
  • live head-side compatibility: preserved only for exact reviewed source blobs
  • arbitrary future copies: refused unless their complete source hash is reviewed and added to host policy
  • OCE dependency: current source is clean after OCE #955; only exact historical PR blobs remain qualified
  • OpenClaw Bay: unaffected because this changes no lifecycle, publication, telemetry, or dashboard contract

Limits

The proof exercises scanner admission and advisory review completion. It does not publish a hosted review, execute apply behavior, or invoke automerge.

@clawsweeper

clawsweeper Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
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.

@RomneyDa
RomneyDa marked this pull request as ready for review October 3, 2026 03:57
@RomneyDa
RomneyDa marked this pull request as draft October 3, 2026 03:59
@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 3, 2026
@clawsweeper

clawsweeper Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed October 3, 2026, 4:46 AM ET / 08:46 UTC (Revision 5).

ClawSweeper review

What this changes

Restrict two retired OpenClaw Enterprise scanner fixtures to exact historical source blobs while preserving existing FTP qualification checks.

Merge readiness

✅ Ready for maintainer review

This remains useful scanner hardening beyond current main. The prior missing-base finding is resolved, the current proof exercises that range, and no blocking patch defect was found.

Priority: P2
Reviewed head: 8580477535f0aceeada96cf8e5d46475f08079c6

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused hardening patch with current production-path observations and the prior blocker resolved.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The captured current-head macOS production review reports native classification on the exact Enterprise base/head range and advisory completion; changed-byte refusal has supplemental regression coverage. No persisted-data contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The captured current-head macOS production review reports native classification on the exact Enterprise base/head range and advisory completion; changed-byte refusal has supplemental regression coverage. No persisted-data contract changes.
Evidence reviewed 7 items Introduced change and current-main necessity: The pinned introduced delta adds 21 source hashes and applies whole-blob checking to pinned URI attributions. Current main qualifies the two Enterprise fixtures without those source pins, so it does not already implement this hardening.
Prior finding resolved: The latest commit adds fa7f7f34b57faf013f53bbd8824cf9a2263c02a42a481282ae3d7d8be29184a2, the actual Enterprise base hash identified by the previous review. The current PR body also records native admission of that base and head, satisfying the concrete prior rank-up request.
Verified dependency boundary and active range: The patch explicitly pins this repository's conformance source. GitHub confirms openclaw/openclaw-enterprise#662 remains open with base 3ab78e1a5c1f702bd1f1b2361959e5c068408dca and head de0ab632d71500684a479a8bb15825094a109a1b; read-only content hashing confirmed both source blobs are pinned.
Findings None None.
Security None None.

How this fits together

ClawSweeper scans review inputs before sending them to the reviewer. Its host-owned fixture policy distinguishes approved synthetic credentials from findings that must stop admission.

flowchart TD
  A[Committed review sources] --> B[Native secret scanner]
  B --> C[Fixture identity checks]
  C --> D[Whole source hash check]
  D --> E[Line and source attribution checks]
  E --> F[Admit review inputs]
  D --> G[Refuse unreviewed inputs]
  E --> G
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta production +38/-7, tests +80/-22 Production growth is justified by explicit historical source pins and generic enforcement, with focused boundary coverage.

Technical review

Best possible solution:

Keep historical fixture admission bounded to reviewed whole-source hashes, with exact native metadata and source attribution enforced.

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

Not applicable as an outstanding bug reproduction: the prior base-admission failure is addressed, and the captured current-head proof exercises that production range.

Is this the best way to solve the issue?

Yes. Reusing whole-source pin enforcement narrows the existing fixture policy without introducing a parallel admission mechanism.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The captured current-head macOS production review reports native classification on the exact Enterprise base/head range and advisory completion; changed-byte refusal has supplemental regression coverage. No persisted-data contract changes.
  • remove merge-risk: 🚨 automation: Current PR review selected no merge-risk labels.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This is bounded review-admission hardening that preserves active historical Enterprise ranges.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The captured current-head macOS production review reports native classification on the exact Enterprise base/head range and advisory completion; changed-byte refusal has supplemental regression coverage. No persisted-data contract changes.
  • proof: sufficient: Contributor real behavior proof is sufficient. The captured current-head macOS production review reports native classification on the exact Enterprise base/head range and advisory completion; changed-byte refusal has supplemental regression coverage. No persisted-data contract changes.

Evidence

What I checked:

  • Introduced change and current-main necessity: The pinned introduced delta adds 21 source hashes and applies whole-blob checking to pinned URI attributions. Current main qualifies the two Enterprise fixtures without those source pins, so it does not already implement this hardening. (src/agent-input-scan-fixtures.ts:252, 8580477535f0)
  • Prior finding resolved: The latest commit adds fa7f7f34b57faf013f53bbd8824cf9a2263c02a42a481282ae3d7d8be29184a2, the actual Enterprise base hash identified by the previous review. The current PR body also records native admission of that base and head, satisfying the concrete prior rank-up request. (src/agent-input-scan-fixtures.ts:274, 8580477535f0)
  • Verified dependency boundary and active range: The patch explicitly pins this repository's conformance source. GitHub confirms Add an internal conditional event operation plan adapter openclaw-enterprise#662 remains open with base 3ab78e1a5c1f702bd1f1b2361959e5c068408dca and head de0ab632d71500684a479a8bb15825094a109a1b; read-only content hashing confirmed both source blobs are pinned. (tests/conformance/kubernetes-compute.test.mjs, 3ab78e1a5c1f)
  • Fixture retirement: GitHub verifies test: avoid secret-shaped credential URL fixtures openclaw-enterprise#955 merged the runtime-constructed fixture approach. Current unpinned endpoints of feat: assign configured OpenClaw roles to Agent users openclaw-enterprise#906 contain neither approved retired whole-line digest, so their changed hashes do not establish an admission regression. Enterprise policy was read fully; its runtime fixture construction guidance supports retirement, while its platform implementation requirements do not govern ClawSweeper's classifier. (tests/conformance/kubernetes-compute.test.mjs, 92005bc88100)
  • Security and attribution invariants: Whole-source filtering precedes existing native metadata, complete-line, occurrence, path, mode, and committed-role checks. Patch findings recursively reuse the original blob policy. FTP rows still require nonempty valid source pins; no scanner verification, execution permission, dependency, or workflow setting changes. (src/agent-input-scan-fixtures.ts:1087, 8580477535f0)
  • Current production proof: The captured PR body identifies head 8580477, macOS and Node 24.16.0, and a local-only production review of the Enterprise range. It reports native classification for both fixtures on both base and head and completed advisory review without source_not_reviewed. The changed-byte refusal is supplemental classifier regression coverage; hosted publication and apply are explicitly outside the proof's scope. (8580477535f0)

Likely related people:

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

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 (4 earlier review cycles)
  • reviewed 2026-10-03T04:01:19.593Z sha 3934af1 :: blocked before merge. :: [P3] Record the rollback in a new changelog fragment
  • reviewed 2026-10-03T04:07:10.032Z sha 3934af1 :: blocked before merge. :: [P1] Preserve admission for the active historical OCE range | [P3] Record the rollback in a new changelog fragment
  • reviewed 2026-10-03T04:43:29.447Z sha ff794d1 :: blocked before merge. :: [P1] Preserve admission until the active Enterprise range is refreshed
  • reviewed 2026-10-03T08:14:00.249Z sha 421a95e :: blocked before merge. :: [P1] Include the actual base blob of the active Enterprise range

@RomneyDa
RomneyDa marked this pull request as ready for review October 3, 2026 04:02
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 3, 2026
@RomneyDa RomneyDa changed the title revert: remove OCE URI fixture qualifications fix: restrict retired OCE fixtures to base blobs Oct 3, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient Contributor real behavior proof is sufficient. label Oct 3, 2026
@RomneyDa RomneyDa changed the title fix: restrict retired OCE fixtures to base blobs fix: bind retired OCE fixtures to source blobs Oct 3, 2026
@clawsweeper clawsweeper Bot added 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. and removed merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Oct 3, 2026
@RomneyDa
RomneyDa merged commit f7c8c55 into main Oct 3, 2026
27 of 28 checks passed
@RomneyDa
RomneyDa deleted the openclaw/revert-oce-fixture-qualifications branch October 3, 2026 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. 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

Development

Successfully merging this pull request may close these issues.

1 participant