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

refactor(skills): simplify release validation workflow#84
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

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