Skip to content

ci: reuse one package in QA smoke#99793

Merged
steipete merged 3 commits into
mainfrom
codex/qa-smoke-shared-package
Jul 4, 2026
Merged

ci: reuse one package in QA smoke#99793
steipete merged 3 commits into
mainfrom
codex/qa-smoke-shared-package

Conversation

@steipete

@steipete steipete commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Related: #99767

What Problem This Solves

Fixes an issue where QA Smoke CI still exhausted a 12 GB Node heap on the default 4-vCPU checks-fast-core runner while building the shared package introduced by #99784.

Why This Change Was Made

Route only QA Smoke CI to the existing 16-vCPU Blacksmith class and raise its inherited Node heap to 16 GB. Keep #99784's single validated package tarball, existing OPENCLAW_CURRENT_PACKAGE_TGZ contract, manual-dispatch fallback, and fork fallback unchanged.

User Impact

No product behavior changes. Maintainers get a stable QA smoke gate without the reproduced tsdown heap failure.

Evidence

AI-assisted: yes. I reviewed the implementation and validation results. A transcript is not attached because the source session also contains unrelated authenticated live-test work.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation size: XS maintainer Maintainer-authored PR labels Jul 4, 2026
@clawsweeper

clawsweeper Bot commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 4, 2026, 12:38 AM ET / 04:38 UTC.

Summary
The PR changes QA Smoke CI to use a 16-vCPU Blacksmith runner and 16GB Node heap, with matching CI docs and workflow guard expectations.

PR surface: Tests -1, Docs 0, Config +5. Total +4 across 3 files.

Reproducibility: unclear. for current main. The linked runs show the pre-fix OOM/rebuild problem, but current main already builds one package and exports OPENCLAW_CURRENT_PACKAGE_TGZ, so I do not have a failing current-main reproduction for the remaining 16-vCPU/16GB change.

Review metrics: 2 noteworthy metrics.

  • QA Smoke runner override: 1 matrix row changed to blacksmith-16vcpu-ubuntu-2404. Runner sizing affects Blacksmith scheduling and should be intentional after the merged package-reuse fix.
  • QA Smoke heap cap: 12288 MB changed to 16384 MB. The heap increase is the remaining unique mitigation and needs after-fix evidence that 12GB is insufficient.

Root-cause cluster
Relationship: partial_overlap
Canonical: #99784
Summary: The merged related PR is canonical for the shared-package QA Smoke repair; this PR overlaps that root cause but still has a distinct runner/heap capacity proposal.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add a completed exact-head QA Smoke CI run or redacted log that shows the changed runner path passing.
  • [P1] Have a maintainer decide whether the runner/heap bump is still needed after the merged shared-package fix.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body does not include a completed after-fix exact-head QA Smoke run or redacted log for the 16-vCPU path; the exact-head QA Smoke CI check was still in progress at review. 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.

Risk before merge

  • [P1] The central package-reuse fix is already on current main through fix(qa): stabilize primary smoke runtime evidence #99784, so merging this branch now mainly adds runner and heap capacity changes whose necessity after that fix is unproven.
  • [P1] The exact-head QA Smoke CI job was still in progress at review, and the PR body has before-failure links plus focused checks but no completed after-fix run or redacted log for the 16-vCPU path.
  • [P1] Moving one checks-fast-core matrix row from the default 4-vCPU runner to 16-vCPU changes Blacksmith scheduling and CI automation capacity, which green unit-style checks do not settle.

Maintainer options:

  1. Prove the runner bump is still needed (recommended)
    Wait for exact-head QA Smoke CI to finish and compare it against the merged 4-vCPU/12GB main behavior before changing the runner tier.
  2. Keep the merged baseline
    A maintainer can close or supersede this branch if fix(qa): stabilize primary smoke runtime evidence #99784 keeps QA Smoke stable without the extra runner and heap increase.
  3. Accept extra CI headroom
    Maintainers may intentionally land the 16-vCPU/16GB path if they accept the Blacksmith capacity cost as operational headroom.

Next step before merge

  • [P1] Manual review is needed because the protected maintainer label and the remaining CI runner-capacity choice are not a safe automated repair decision.

Security
Cleared: The diff changes an existing workflow runner expression, Node heap value, docs, and tests without adding third-party actions, permissions, secrets, dependencies, or external downloads.

Review details

Best possible solution:

Keep the merged shared-package implementation as canonical, and land a narrow runner/heap bump only if completed exact-head or fresh main QA Smoke evidence shows the 4-vCPU/12GB path still fails or materially needs more capacity.

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

Unclear for current main. The linked runs show the pre-fix OOM/rebuild problem, but current main already builds one package and exports OPENCLAW_CURRENT_PACKAGE_TGZ, so I do not have a failing current-main reproduction for the remaining 16-vCPU/16GB change.

Is this the best way to solve the issue?

No, not clearly as the final landing path. Shared package reuse is already implemented by the merged related PR, and this PR's remaining runner/heap bump is a plausible operational mitigation that still needs exact-head proof and maintainer capacity judgment.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 26cb189276af.

Label changes

Label justifications:

  • P2: This is a CI reliability and capacity change with limited blast radius and no product runtime behavior change.
  • merge-risk: 🚨 automation: The diff changes GitHub Actions runner selection and QA Smoke resource limits, so merge risk is CI automation behavior rather than ordinary code correctness.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • 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 does not include a completed after-fix exact-head QA Smoke run or redacted log for the 16-vCPU path; the exact-head QA Smoke CI check was still in progress at review. 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

PR surface:

Tests -1, Docs 0, Config +5. Total +4 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 1 3 4 -1
Docs 1 9 9 0
Config 1 8 3 +5
Generated 0 0 0 0
Other 0 0 0 0
Total 3 20 16 +4

What I checked:

  • Repository policy read: Read the full root AGENTS.md plus scoped docs/test guides; the protected-label, CI proof, and workflow tuning review rules apply to this PR. (AGENTS.md:1, 26cb189276af)
  • Current main already reuses one QA package: Current main builds one smoke package, sets a 12GB heap, and exports OPENCLAW_CURRENT_PACKAGE_TGZ before running the smoke profile. (.github/workflows/ci.yml:873, 26cb189276af)
  • Existing Docker package reuse contract: scripts/test-docker-all.mjs treats OPENCLAW_CURRENT_PACKAGE_TGZ as an existing package contract and skips preparing another package when it is present. (scripts/test-docker-all.mjs:973, 26cb189276af)
  • Merged overlapping fix: The merged related PR implemented the central shared-package smoke repair and OTEL smoke fix on current main at merge commit 26cb189. (.github/workflows/ci.yml:875, 26cb189276af)
  • Remaining PR diff: Against current main, this PR mainly adds a runner override for QA Smoke CI, changes checks-fast-core to read matrix.runner, raises the heap from 12288 to 16384, and updates docs/tests. (.github/workflows/ci.yml:288, 7ff2cccf11ac)
  • Exact-head proof gap: At review time, the exact-head QA Smoke CI check for 7ff2ccc was still in progress, so the changed 16-vCPU path had not completed successfully. (7ff2cccf11ac)

Likely related people:

  • RomneyDa: Merged the current-main QA Smoke package reuse and OTEL smoke repair that overlaps the central purpose of this PR. (role: adjacent QA smoke reliability contributor; confidence: high; commits: 26cb189276af; files: .github/workflows/ci.yml, test/e2e/qa-lab/runtime/qa-otel-smoke-runtime.ts, test/scripts/ci-workflow-guards.test.ts)
  • Vincent Koc: Recent history shows repeated CI runner, workflow fanout, and runner-registration work in the same workflow area. (role: recent CI capacity contributor; confidence: medium; commits: e085fa1a3ffd, dd618aa54550, a97188ceb308; files: .github/workflows/ci.yml, docs/ci.md)
  • steipete: The PR author also appears in recent CI/docs history around runner and release-validation changes, so they are connected beyond this branch alone. (role: current proposer and adjacent CI contributor; confidence: medium; commits: 3e163667f258, 6bc9c37cd17f, 7ff2cccf11ac; files: .github/workflows/ci.yml, docs/ci.md, test/scripts/ci-workflow-guards.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. labels Jul 4, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 4, 2026
@steipete
steipete force-pushed the codex/qa-smoke-shared-package branch from a8a4c39 to 7ff2ccc Compare July 4, 2026 04:30
@clawsweeper clawsweeper Bot mentioned this pull request Jul 4, 2026
Merged
@steipete

steipete commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Land-ready at exact head 7ff2cccf11acd97128bb56e47d92e3f78bd6df01.

  • Before: PR feat(macos): install and run the local Gateway automatically #99767 run 28694693735 exhausted the 12 GB heap during tsdown on the default 4-vCPU runner.
  • After: run 28694945254 passed on the exact head, including QA Smoke CI job 85102890881 on the 16-vCPU/16-GB configuration.
  • Local proof: 33 workflow guard tests, workflow validation, docs MDX validation, and git diff --check passed.
  • Review artifacts validate with READY FOR /prepare-pr, zero findings, and no security concerns.

@steipete
steipete merged commit 5ef2c81 into main Jul 4, 2026
88 checks passed
@steipete
steipete deleted the codex/qa-smoke-shared-package branch July 4, 2026 04:39
@steipete

steipete commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jul 4, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 4, 2026
@steipete
ci: reuse one package in QA smoke (openclaw#99793)
0c99539 
* docs(ci): document QA smoke runner

* ci: increase QA smoke build memory

* ci: route QA smoke to 16 vCPU runner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

docs Improvements or additions to documentation maintainer Maintainer-authored PR merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@steipete