Skip to content

fix(qa): prevent smoke gateways from losing built files#99736

Merged
RomneyDa merged 1 commit into
mainfrom
dallin/fix-qa-smoke-dist-race
Jul 4, 2026
Merged

fix(qa): prevent smoke gateways from losing built files#99736
RomneyDa merged 1 commit into
mainfrom
dallin/fix-qa-smoke-dist-race

Conversation

@RomneyDa

@RomneyDa RomneyDa commented Jul 4, 2026

Copy link
Copy Markdown
Member

Closes #99734

What Problem This Solves

Fixes an issue where QA Smoke Gateway processes could fail to start or load generated modules when script-backed scenarios rebuilt the checkout's shared package output during the same run.

Why This Change Was Made

Script-backed scenarios now run only after flow Gateway work has completed. Playwright and Vitest scenarios keep their existing concurrent execution, limiting serialization to the process type that can mutate shared build output.

User Impact

Maintainers can validate mixed QA Smoke selections without intermittent missing dist/index.js or generated chunk failures caused by concurrent package rebuilds.

Evidence

@RomneyDa

RomneyDa commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper

clawsweeper Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 9:18 PM ET / 01:18 UTC.

Summary
The PR changes QA Lab unified suite scheduling so script-backed test-file scenarios run after flow and non-script partitions, with a regression test that preserves Playwright concurrency.

PR surface: Source +19, Tests +72. Total +91 across 2 files.

Reproducibility: yes. Current main includes the script-backed Docker scenario and still schedules test-file work alongside flow partitions, while the linked issue and Actions evidence show missing shared dist files under that mixed selection; I did not rerun the expensive smoke path locally.

Review metrics: 1 noteworthy metric.

  • Script scenario scheduling: 1 execution kind moved to post-flow serial phase. The PR changes when script-backed QA scenarios run relative to flow Gateways, which is the key automation behavior maintainers should verify before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99734
Summary: This PR is the candidate fix for the canonical QA Smoke shared-dist scheduler race; the merged Docker-scenario PR exposed the race, while a separate QA Smoke timeout PR is adjacent but distinct.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • Let the relevant PR or merge-result QA Smoke path finish green before landing.

Risk before merge

  • [P1] Merging changes QA automation scheduling by serializing script-backed scenarios after flow partitions; that is the intended fix for shared dist invalidation, but maintainers should accept the runtime/throughput tradeoff with final smoke evidence.

Maintainer options:

  1. Land after final smoke proof (recommended)
    Wait for the PR head or merge-result QA Smoke path to pass so the scheduler tradeoff is validated with the now-merged Docker script scenarios.
  2. Accept focused proof deliberately
    Maintainers can merge on the focused scheduler test and existing Testbox evidence if they are comfortable accepting the serialized script-runner automation tradeoff.

Next step before merge

  • [P2] No repair lane is needed; maintainers should handle final review and merge timing because the PR has no discrete code finding but changes QA automation scheduling.

Security
Cleared: The diff only changes QA Lab scheduler logic and its regression test; it adds no dependencies, workflows, permissions, secrets handling, or third-party execution sources.

Review details

Best possible solution:

Land the scheduler-level isolation after final PR or merge-result QA checks, keeping Docker lane behavior unchanged and preventing package-mutating scripts from overlapping flow Gateways.

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

Yes. Current main includes the script-backed Docker scenario and still schedules test-file work alongside flow partitions, while the linked issue and Actions evidence show missing shared dist files under that mixed selection; I did not rerun the expensive smoke path locally.

Is this the best way to solve the issue?

Yes. The scheduler is the best layer because the failure comes from package-mutating script scenarios overlapping flow Gateway processes, not from a specific Docker lane implementation.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This fixes a normal-priority QA Smoke orchestration bug that blocks maintainer validation for mixed flow plus Docker script selections without directly breaking released user runtime behavior.
  • add merge-risk: 🚨 automation: The diff changes QA suite scheduling, so merge safety depends on accepting the CI/proof-runtime tradeoff beyond ordinary unit correctness.
  • 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. Not applicable: The external-contributor proof gate does not apply to this maintainer-labeled MEMBER PR; the PR body still provides failure evidence, focused test output, Testbox proof, and linked Actions runs for maintainer review.

Label justifications:

  • P2: This fixes a normal-priority QA Smoke orchestration bug that blocks maintainer validation for mixed flow plus Docker script selections without directly breaking released user runtime behavior.
  • merge-risk: 🚨 automation: The diff changes QA suite scheduling, so merge safety depends on accepting the CI/proof-runtime tradeoff beyond ordinary unit correctness.
  • 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. Not applicable: The external-contributor proof gate does not apply to this maintainer-labeled MEMBER PR; the PR body still provides failure evidence, focused test output, Testbox proof, and linked Actions runs for maintainer review.
Evidence reviewed

PR surface:

Source +19, Tests +72. Total +91 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 25 6 +19
Tests 1 72 0 +72
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 97 6 +91

What I checked:

Likely related people:

  • RomneyDa: GitHub path history shows repeated merged work introducing and refining unified QA suite execution, test-file routing, and smoke-profile scheduling in the files touched by this PR. (role: recent QA Lab scheduler contributor; confidence: high; commits: 561b293c7a96, 438f208a7666, f29dbd3ebd1c; files: extensions/qa-lab/src/suite-launch.runtime.ts, extensions/qa-lab/src/suite-launch.runtime.test.ts, extensions/qa-lab/src/test-file-scenario-runner.ts)
  • Solvely-Colin: Commit 4ae0a5d added the QA smoke profile CI workflow path and guard tests where the shared-dist race is observed. (role: CI smoke profile introducer; confidence: medium; commits: 4ae0a5d958a0; files: .github/workflows/ci.yml, docs/ci.md, test/scripts/ci-workflow-guards.test.ts)
  • vincentkoc: Recent history on the test-file scenario runner includes script timeout and process-tree handling, which is adjacent to the script execution path isolated by this PR. (role: test-file runner adjacent contributor; confidence: medium; commits: 405e5072fd51, a6e41a0cc1b8, fd66b44f5e2c; files: extensions/qa-lab/src/test-file-scenario-runner.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: 🐚 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. 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
@RomneyDa

RomneyDa commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

Exact-head verification for eb48efc7294ae843f50c57b6c9d06a707bd1f130:

No known proof gaps remain. Cancelled auto-response and Real behavior proof jobs are superseded/non-blocking; the PR has no pending or failed checks.

@RomneyDa
RomneyDa merged commit 24f654e into main Jul 4, 2026
148 of 156 checks passed
@RomneyDa
RomneyDa deleted the dallin/fix-qa-smoke-dist-race branch July 4, 2026 01:25
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extensions: qa-lab 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: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S 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.

[Bug]: QA Smoke scripts can invalidate dist used by flow Gateways

1 participant