Skip to content

ci: add protected broker soak canary - #1218

Merged
vincentkoc merged 3 commits into
mainfrom
ops/broker-soak-activation
Aug 1, 2026
Merged

ci: add protected broker soak canary#1218
vincentkoc merged 3 commits into
mainfrom
ops/broker-soak-activation

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

Closes #1217

What Problem This Solves

Resolves an operational gap where broker maintenance and Daytona fallback readiness had no bounded production canary with durable, sanitized evidence.

Why This Change Was Made

Adds a manual, protected default-branch workflow that audits scheduled AWS and Azure orphan sweeps, runs one broker-only Daytona command, and verifies cleanup for a run-scoped owner. It reuses the existing image-publisher broker credentials, never receives direct provider credentials, and creates no warm pool.

User Impact

Operators can validate broker scheduler health and Daytona fallback behavior without local AWS, Azure, or Daytona authentication. Each run is serialized, time-bounded, and fails closed on stale or errored maintenance evidence or incomplete lease cleanup.

Evidence

  • actionlint .github/workflows/broker-soak-canary.yml
  • node --test scripts/broker-soak-canary-workflow.test.js scripts/workflow-action-pins.test.js (4 passed)
  • Direct jq validation with millisecond Date.toISOString() timestamps
  • Fresh autoreview: no accepted/actionable findings
  • actions/upload-artifact@v7.0.1 pinned to 043fb46d1a93c77aae656e7c1c64a875d1fc6a0a; official manifest uses Node 24

@vincentkoc
vincentkoc marked this pull request as ready for review August 1, 2026 09:26
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Aug 1, 2026
@clawsweeper

clawsweeper Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed August 1, 2026, 5:35 AM ET / 09:35 UTC.

ClawSweeper review

What this changes

The branch adds a manually dispatched, protected GitHub Actions canary that audits broker maintenance, performs one bounded Daytona run through Crabbox, verifies canary-lease cleanup, and uploads sanitized evidence.

Merge readiness

⚠️ Ready for maintainer review - 5 items remain

This PR remains necessary as the candidate implementation for the linked operations request, but the current head still has the two prior P2 policy findings: it edits the release-owned changelog and hard-codes OpenClaw-specific workflow identities in a repository that requires generic examples and workflows. The protected-dispatch pattern otherwise matches current main’s credentialed workflow safeguards; because this is a MEMBER-authored, production-credential workflow, it should remain open for explicit maintainer approval after those repairs.

Priority: P2
Reviewed head: 5ad282dc39117b10b3eb5a5fbbbead324a610ab9
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch follows an established protected-workflow pattern and includes focused static assertions, but two unchanged policy findings and production-boundary approval remain.
Proof confidence 🌊 off-meta tidepool Not applicable: This MEMBER-authored operational PR is not subject to the external-contributor proof gate; however, a redacted protected-branch dispatch record remains the appropriate deployment validation before maintainer approval.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This MEMBER-authored operational PR is not subject to the external-contributor proof gate; however, a redacted protected-branch dispatch record remains the appropriate deployment validation before maintainer approval.
Evidence reviewed 5 items Unchanged prior finding: release-owned changelog: The current PR head still adds an Unreleased changelog entry, although the prior ClawSweeper review explicitly requested its removal; release preparation owns this file for normal PR review.
Unchanged prior finding: project-specific workflow identity: The proposed workflow sets both a run owner ending in @openclaw.invalid and CRABBOX_ORG: openclaw, contrary to the repository policy requiring generic examples and workflows outside legacy or release-history files.
Current-main protected-workflow precedent: Current main’s coordinator token rotation workflow uses the same protected default-branch guard, workflow-SHA checkout, and disabled persisted checkout credentials; the proposed guard follows that established pattern.
Findings 2 actionable findings [P2] Remove the release-owned changelog entry
[P2] Replace project-specific workflow identity values
Security None None.

How this fits together

Crabbox uses GitHub Actions for protected operational workflows that call the coordinator with environment-scoped credentials. This new workflow would inspect maintenance records, use the broker to create a short Daytona lease, wait for cleanup, and expose redacted results as an operator artifact.

flowchart LR
  Dispatch[Manual protected dispatch] --> Guard[Default-branch guard]
  Guard --> Audit[Maintenance audit]
  Audit --> Run[Brokered Daytona run]
  Run --> Cleanup[Scoped lease cleanup check]
  Cleanup --> Artifact[Sanitized proof artifact]
  Artifact --> Operators[Operator review]
Loading

Decision needed

Question Recommendation
Should this repository authorize the image-publisher environment’s coordinator and Access credentials for a manually dispatched Daytona production canary after the generic-identity and release-note repairs? Authorize the bounded canary: Accept the credentialed operational scope after the two mechanical policy repairs and a redacted protected-branch proof demonstrates acquisition and cleanup.

Why: The patch introduces a credentialed workflow that provisions and releases real broker resources; code review can verify guards, but maintainers must decide whether this production operational boundary is acceptable.

Before merge

  • Remove the release-owned changelog entry (P2) - This repeats the prior review finding: the unchanged head still edits CHANGELOG.md for a normal workflow PR. Keep the operator-visible summary in the PR body or commit message and leave release preparation as the sole owner of the Unreleased section.
  • Replace project-specific workflow identity values (P2) - This repeats the prior review finding: the workflow still hard-codes openclaw in its scoped owner and organization. The repository policy requires generic workflow values outside legacy or release-history files; derive the organization from repository context and use a generic invalid owner suffix, then update the matching assertion.
  • Resolve merge risk (P1) - Merging a workflow that uses coordinator and Access secrets authorizes a real brokered production canary; static tests do not establish that the configured environment, scoped owner, and cleanup path behave safely in the target deployment.
  • Resolve merge risk (P1) - The hard-coded organization and owner identity make a generic repository workflow deployment-specific, creating avoidable maintenance drift and an incorrect ownership scope if reused elsewhere.

Findings

  • [P2] Remove the release-owned changelog entry — CHANGELOG.md:7-9
  • [P2] Replace project-specific workflow identity values — .github/workflows/broker-soak-canary.yml:49-50
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Workflow surface 1 workflow added; 377 workflow lines The new workflow contains credential handling, remote execution, cleanup polling, and artifact upload, so its operational boundary deserves focused approval.
Patch scope 451 added, 0 removed across 3 files Most of the patch is one new automation path plus a string-level test and release-note edit.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #1217
Summary: This PR is the unmerged candidate implementation for the linked operational-canary request.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Repair policy gaps and approve the canary (recommended)
    Remove the changelog edit, replace the hard-coded identity values with repository-derived or generic equivalents, update the string-level workflow test, then have a maintainer approve one redacted protected-branch run.
  2. Pause the credentialed workflow
    Keep the operational goal in ops: add protected Daytona canary and broker maintenance audit #1217 but defer repository-managed production credentials until maintainers explicitly sponsor this boundary.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Remove the release-owned CHANGELOG.md entry; derive the workflow organization from github.repository_owner, replace the OpenClaw-specific invalid owner address with a generic one, update the focused workflow test, and provide redacted protected-branch run output.

Technical review

Best possible solution:

Keep the protected guard, bounded run, cleanup verification, and sanitized artifact design; remove the release-owned changelog edit, derive the organization from the repository context, use a generic canary owner identity, and obtain a redacted protected-branch dispatch record before enabling the production workflow.

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

Not applicable: this PR adds a new operational canary, not a correction to an existing reproducible user failure. The proposed workflow has focused static tests, but no completed protected-branch canary run is available as deployment proof.

Is this the best way to solve the issue?

No: the protected workflow structure is consistent with current main, but retaining the release-owned changelog edit and deployment-specific identity values is not the narrowest maintainable solution. Repository-derived organization data and generic example identity values preserve the intended behavior without coupling the workflow to one project name.

Full review comments:

  • [P2] Remove the release-owned changelog entry — CHANGELOG.md:7-9
    This repeats the prior review finding: the unchanged head still edits CHANGELOG.md for a normal workflow PR. Keep the operator-visible summary in the PR body or commit message and leave release preparation as the sole owner of the Unreleased section.
    Confidence: 0.98
  • [P2] Replace project-specific workflow identity values — .github/workflows/broker-soak-canary.yml:49-50
    This repeats the prior review finding: the workflow still hard-codes openclaw in its scoped owner and organization. The repository policy requires generic workflow values outside legacy or release-history files; derive the organization from repository context and use a generic invalid owner suffix, then update the matching assertion.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 64a7f8f50e00.

Labels

Label justifications:

  • P2: The proposed production canary is a bounded operator-facing improvement with limited blast radius but credentialed execution.
  • merge-risk: 🚨 security-boundary: The workflow receives coordinator admin and Access credentials while creating and releasing a real Daytona lease.
  • merge-risk: 🚨 automation: The PR introduces a protected GitHub Actions workflow whose guards, cleanup polling, and artifact behavior run outside normal application tests.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This MEMBER-authored operational PR is not subject to the external-contributor proof gate; however, a redacted protected-branch dispatch record remains the appropriate deployment validation before maintainer approval.

Evidence

What I checked:

  • Unchanged prior finding: release-owned changelog: The current PR head still adds an Unreleased changelog entry, although the prior ClawSweeper review explicitly requested its removal; release preparation owns this file for normal PR review. (CHANGELOG.md:7, 5ad282dc3911)
  • Unchanged prior finding: project-specific workflow identity: The proposed workflow sets both a run owner ending in @openclaw.invalid and CRABBOX_ORG: openclaw, contrary to the repository policy requiring generic examples and workflows outside legacy or release-history files. (.github/workflows/broker-soak-canary.yml:49, 5ad282dc3911)
  • Current-main protected-workflow precedent: Current main’s coordinator token rotation workflow uses the same protected default-branch guard, workflow-SHA checkout, and disabled persisted checkout credentials; the proposed guard follows that established pattern. (.github/workflows/coordinator-admin-token-rotate.yml:20, 64a7f8f50e00)
  • Feature-history routing: Git blame attributes the current protected coordinator workflow guard to Vincent Koc in the current-main commit, connecting vincentkoc to the adjacent credentialed-workflow design rather than only to this proposed branch. (.github/workflows/coordinator-admin-token-rotate.yml:23, 64a7f8f50e00)
  • Linked implementation target: The PR body identifies ops: add protected Daytona canary and broker maintenance audit #1217 as the request for a protected broker and Daytona canary; the PR is still unmerged, so it is a candidate fix rather than evidence that the issue is resolved.

Likely related people:

  • vincentkoc: Current main attributes the adjacent protected coordinator workflow to Vincent Koc, and this contributor also authored the proposed broker canary commits. (role: recent workflow and broker-area contributor; confidence: high; commits: 64a7f8f50e00, 6aa95b418938; files: .github/workflows/coordinator-admin-token-rotate.yml, .github/workflows/broker-soak-canary.yml)

Rank-up moves

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

  • Remove the release-owned changelog entry.
  • Replace the project-specific identity values and update the focused workflow test.
  • Provide a redacted protected-branch dispatch result for maintainer approval.

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 (1 earlier review cycle)
  • reviewed 2026-08-01T09:30:43.202Z sha 5ad282d :: found issues before merge. :: [P2] Remove the release-owned changelog entry | [P2] Replace project-specific workflow identity values

@vincentkoc

Copy link
Copy Markdown
Member Author

Land-ready proof for exact head 5ad282dc39117b10b3eb5a5fbbbead324a610ab9:

Known proof gap: the protected production canary can only be dispatched after this workflow lands on the default branch. I will dispatch it from the exact merged main SHA and attach the sanitized maintenance, Daytona timing, and cleanup evidence afterward.

@vincentkoc
vincentkoc merged commit 0171ae2 into main Aug 1, 2026
26 of 27 checks passed
@vincentkoc
vincentkoc deleted the ops/broker-soak-activation branch August 1, 2026 09:45
@vincentkoc

Copy link
Copy Markdown
Member Author

Production proof is complete on exact merged main SHA 0171ae2ee19fbf18ef0c2d5a2d04d8fe56a437f2:

  • workflow run: https://github.com/openclaw/crabbox/actions/runs/30694383255 (success, 3m24s)
  • AWS scheduled sweep: alarm-triggered delete mode, finished 2026-08-01T09:03:40.737Z, 26 scanned, 1 candidate, 0 terminated, 0 errors
  • Azure scheduled sweep: alarm-triggered delete mode, finished 2026-08-01T08:51:59.771Z, 7 scanned, 0 candidates, 0 terminated, 0 errors
  • Daytona doctor: broker/provider/fallback/admin checks all ok
  • Daytona canary: succeeded, totalMs=8147, commandMs=7144, syncMs=0, exitCode=0
  • cleanup: 1 run-scoped owner record, unsafeBeforeCleanup=0, unsafeAfterCleanup=0

The uploaded artifact contains only the sanitized maintenance, doctor, timing, and cleanup records; no lease IDs, hosts, slugs, or provider credentials.

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. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ops: add protected Daytona canary and broker maintenance audit

1 participant