Skip to content

refactor(skills): simplify release validation workflow - #84

Open
Patrick-Erichsen wants to merge 9 commits into
openclaw:mainfrom
Patrick-Erichsen:pe/openclaw-release-validation
Open

Patrick-Erichsen wants to merge 9 commits into
openclaw:mainfrom
Patrick-Erichsen:pe/openclaw-release-validation

Conversation

@Patrick-Erichsen

@Patrick-Erichsen Patrick-Erichsen commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • simplify the release-validation skill around one isolated copied-state upgrade journey and a human-editable Markdown worksheet
  • derive the complete surface catalog, maturity labels, score-band guidance, and five release priorities from the live OpenClaw maturity scorecard
  • present each surface as a compact guidance table with precise runnable checks and an empty Testing notes evidence cell
  • enumerate the current tester's PRs included in the candidate above the priority surfaces
  • mirror the canonical OpenClaw skill while keeping PR, issue, and commit examples out of published release summaries
  • update OCM repository references and contract coverage for the generated worksheet structure

Validation

Further local tests were not run at maintainer request; GitHub CI is the validation gate for this PR.

Related

@clawsweeper

clawsweeper Bot commented Aug 18, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

@clawsweeper clawsweeper Bot added merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 18, 2026
@clawsweeper

clawsweeper Bot commented Aug 18, 2026 •

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 18, 2026, 2:30 AM ET / 06:30 UTC.

ClawSweeper review

What this changes

This PR replaces the broad release-validation skill with a human-led copied-gateway worksheet, updates OCM’s public repository URLs, and rewrites related contract tests.

Merge readiness

⛔ Blocked until real behavior proof is added - 11 items remain

Keep open: the prior P1 credential-authorization and source-gateway recovery blockers remain at the current head, and replacing the established package-shaped release-validation matrix with a human worksheet needs an explicit core-policy decision.

Priority: P1
Reviewed head: 1dedee5d1b357830f2de4bc05334f99887ef57bf
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The patch retains two P1 safety defects and lacks real after-fix behavior proof.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: The PR body supplies no redacted after-fix run proving both authorized copied-credential activation and recovery of the source gateway after a failed or aborted activation. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🦪 silver shellfish (2/6) Security review found an item that needs attention.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR body supplies no redacted after-fix run proving both authorized copied-credential activation and recovery of the source gateway after a failed or aborted activation. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 7 items Unsafe activation remains in the PR head: The new workflow imports selected real state, then instructs operators to stop the credential owner and start the copied environment without an explicit authorization or mock/test-account gate.
Current safety contract requires an authorization gate: Current OCM guidance classifies retained-auth clones as secret-bearing, keeps them stopped by default, and requires explicit authorization or mocked/dedicated credentials before external access.
Start is a real service activation: OCM documents that start installs and starts the environment service by default, so this is not only a local static validation step.
Findings 2 actionable findings [P1] Require authorization before starting copied credentials
[P1] Restore the original gateway when copied activation fails
Security Needs attention Copied credentials can activate without authorization: The PR directs a copied gateway with selected real state to start after stopping the original owner, but omits the current required authorization or mock-account gate for external services.

How this fits together

The release-validation skill directs OCM operators from a selected existing OpenClaw gateway through a copied test environment, candidate upgrade, and release feedback. It therefore touches retained credentials and the availability of the source gateway before producing a GitHub-facing worksheet summary.

flowchart LR
A[Existing gateway state] --> B[Copied OCM test environment]
B --> C[Credential activation decision]
C --> D[Candidate runtime upgrade]
D --> E[Gateway and human checks]
E --> F[Redacted release feedback]
C --> G[Source gateway restoration]
Loading

Decision needed

Question Recommendation
Should OCM’s bundled release-validation skill replace its package-shaped scenario matrix with a human-led worksheet workflow that creates shared GitHub campaign issues? Keep the matrix as the baseline: Retain package-shaped scenario validation and treat the worksheet as supplemental operator notes after the safety defects are fixed.

Why: This changes the repository’s release-confidence policy and external publication workflow; the current main implementation establishes the matrix as the existing baseline, while the supplied discussion records no explicit approval for replacing it.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR body supplies no redacted after-fix run proving both authorized copied-credential activation and recovery of the source gateway after a failed or aborted activation. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Require authorization before starting copied credentials (P1) - This remains a prior blocker. The instructions import selected real state, stop its credential owner, and start the copied gateway without requiring explicit authorization or mocked/dedicated credentials. Current OCM safety guidance forbids external access from retained credentials by default, so this can trigger real provider or channel activity.
  • Restore the original gateway when copied activation fails (P1) - This remains a prior blocker. After stopping the live credential owner here, a failed ocm start only tells the operator not to continue; restoration occurs solely in the later normal finish flow. Restore the original owner immediately on failure or abort so a failed validation does not leave the user’s gateway down.
  • Resolve security concern: Copied credentials can activate without authorization - The PR directs a copied gateway with selected real state to start after stopping the original owner, but omits the current required authorization or mock-account gate for external services.
  • Resolve merge risk (P1) - Starting copied retained credentials without explicit authorization can contact real providers or channels.
  • Resolve merge risk (P1) - A candidate startup failure can leave the original credential-owning gateway stopped until a later manual finish step.
  • Resolve merge risk (P1) - The PR removes the established package-shaped scenario-matrix baseline before a maintainer has accepted the lower-automation release-validation policy.
  • Improve patch quality - Add explicit authorization or mock-account gating and guaranteed source-gateway restoration, with focused contract coverage.
  • Improve patch quality - Post a redacted live validation transcript showing the authorized path and failure/abort recovery; update the PR body for re-review.
  • Improve patch quality - Obtain a maintainer decision on whether the worksheet may replace the package-shaped scenario-matrix baseline.

Findings

  • [P1] Require authorization before starting copied credentials — skills/openclaw-release-validation/SKILL.md:154-158
  • [P1] Restore the original gateway when copied activation fails — skills/openclaw-release-validation/SKILL.md:157-171
  • [high] Copied credentials can activate without authorization — skills/openclaw-release-validation/SKILL.md:154
Agent review details

Security

Needs attention: The copied-state activation path weakens the existing credential boundary by starting retained credentials without explicit authorization.

Review metrics

Metric Value Why it matters
Patch scope 10 files affected; 384 added, 226 removed The central skill rewrite is accompanied by install, self-update, and contract-test changes, so the release-process decision reaches beyond documentation.

Merge-risk options

Maintainer options:

  1. Add activation and recovery guarantees (recommended)
    Require explicit authorization or mocked/dedicated credentials before start, and restore the original gateway immediately on startup failure, abort, and normal completion.
  2. Keep the existing validation baseline
    Do not merge the worksheet replacement unless a maintainer explicitly accepts replacing package-shaped matrix coverage with this workflow.

Technical review

Best possible solution:

Retain package-shaped scenario validation as the release-confidence baseline, and only add the worksheet flow after it explicitly gates real credentials, restores the source gateway on every exit path, and receives core-policy approval.

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

Yes, source-reproducible: following the documented selected-state import and start sequence reaches credential activation, while the error path lacks immediate source-gateway restoration. No real credentials were used during this read-only review.

Is this the best way to solve the issue?

No: the proposed workflow omits existing credential-safety and recovery guarantees, and its replacement of package-shaped matrix coverage requires maintainer policy approval.

Full review comments:

  • [P1] Require authorization before starting copied credentials — skills/openclaw-release-validation/SKILL.md:154-158
    This remains a prior blocker. The instructions import selected real state, stop its credential owner, and start the copied gateway without requiring explicit authorization or mocked/dedicated credentials. Current OCM safety guidance forbids external access from retained credentials by default, so this can trigger real provider or channel activity.
    Confidence: 0.99
  • [P1] Restore the original gateway when copied activation fails — skills/openclaw-release-validation/SKILL.md:157-171
    This remains a prior blocker. After stopping the live credential owner here, a failed ocm start only tells the operator not to continue; restoration occurs solely in the later normal finish flow. Restore the original owner immediately on failure or abort so a failed validation does not leave the user’s gateway down.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 2a283cc2756c.

Labels

Label justifications:

  • P1: The current instructions can both activate retained production credentials and leave a user’s original gateway unavailable after a failed candidate start.
  • merge-risk: 🚨 security-boundary: The workflow starts a copied gateway with retained credentials without the explicit authorization or mock-account gate required by current safety guidance.
  • merge-risk: 🚨 availability: The source credential owner is stopped before candidate activation but is only restored on the normal finish path.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies no redacted after-fix run proving both authorized copied-credential activation and recovery of the source gateway after a failed or aborted activation. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

Security concerns:

  • [high] Copied credentials can activate without authorization — skills/openclaw-release-validation/SKILL.md:154
    The PR directs a copied gateway with selected real state to start after stopping the original owner, but omits the current required authorization or mock-account gate for external services.
    Confidence: 0.99

What I checked:

  • Unsafe activation remains in the PR head: The new workflow imports selected real state, then instructs operators to stop the credential owner and start the copied environment without an explicit authorization or mock/test-account gate. (skills/openclaw-release-validation/SKILL.md:154, 1dedee5d1b35)
  • Current safety contract requires an authorization gate: Current OCM guidance classifies retained-auth clones as secret-bearing, keeps them stopped by default, and requires explicit authorization or mocked/dedicated credentials before external access. (skills/ocm-operator/references/safety-and-state.md:24, 2a283cc2756c)
  • Start is a real service activation: OCM documents that start installs and starts the environment service by default, so this is not only a local static validation step. (docs/USAGE.md:57, 2a283cc2756c)
  • Failure path does not restore the source gateway: The branch stops the original credential owner immediately before startup, but restoration appears only in the normal finish sequence; its error path stops further testing without requiring immediate restoration. (skills/openclaw-release-validation/SKILL.md:157, 1dedee5d1b35)
  • Contract coverage does not enforce the missing guards: The rewritten safety test checks wording across several documents, but does not require explicit authorization before activation or recovery of the original owner on startup failure or abort. (tests/skill_contract_tests.rs:100, 1dedee5d1b35)
  • Prior blockers remain unresolved: The supplied previous ClawSweeper review at 82f7856 identified the same authorization and failed-activation restoration P1 findings; both are still visible in the current head. (skills/openclaw-release-validation/SKILL.md:154, 82f785667eb9)

Likely related people:

  • Peter Steinberger: The available current-main blame and log trace attribute both the existing release-validation instructions and credential-safety guidance to the v0.2.32 snapshot authored by Peter Steinberger; older provenance is unavailable in this shallow history. (role: current-main release-validation and operator-safety contributor; confidence: medium; commits: 2a283cc2756c; files: skills/openclaw-release-validation/SKILL.md, skills/ocm-operator/references/safety-and-state.md)

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 (2 earlier review cycles)
  • reviewed 2026-08-18T06:03:44.880Z sha f722706 :: needs real behavior proof before merge. :: [P1] Require authorization before activating copied credentials | [P1] Restore the source gateway on failed activation
  • reviewed 2026-08-18T06:17:07.520Z sha 82f7856 :: needs real behavior proof before merge. :: [P1] Require authorization before activating copied credentials | [P1] Restore the source gateway after failed copied activation

This branch has not been deployed

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

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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