Skip to content

improve: enforce canonical QA scenario ownership#99628

Merged
RomneyDa merged 6 commits into
mainfrom
dallin/e2e-wave0-ownership-coverage
Jul 3, 2026
Merged

improve: enforce canonical QA scenario ownership#99628
RomneyDa merged 6 commits into
mainfrom
dallin/e2e-wave0-ownership-coverage

Conversation

@RomneyDa

@RomneyDa RomneyDa commented Jul 3, 2026

Copy link
Copy Markdown
Member

Closes #99627

What Problem This Solves

QA scenario IDs can currently exist in both the canonical YAML inventory and code-defined live transport catalogs. That ambiguity can route coverage and execution references to different implementations without a validation failure. Some helper-backed scenarios are also marked as primary evidence even though they do not execute the required product boundary.

Why This Change Was Made

qa coverage now loads the remaining Discord, Slack, Telegram, and WhatsApp catalog IDs only long enough to reject duplicate IDs across YAML and non-YAML sources. The coverage report schema and output stay unchanged.

YAML coverage metadata now accepts supporting-only entries while normalizing them to the existing primary: [] runtime shape. Helper-backed claims that do not execute the required boundary move from coverage.primary to coverage.secondary. Existing operator-facing live transport commands remain unchanged.

User Impact

There is no end-user runtime behavior change. QA maintainers get deterministic duplicate-ID failures with both source paths, while coverage consumers keep the existing report and metadata shapes.

Evidence

  • node scripts/run-vitest.mjs extensions/qa-lab/src/coverage-report.test.ts extensions/qa-lab/src/scenario-catalog.test.ts extensions/qa-lab/src/scenario-packs.test.ts extensions/qa-lab/src/live-transports/shared/live-transport-scenarios.test.ts extensions/qa-lab/src/cli.runtime.test.ts — 5 files and 145 tests passed.
  • node --import tsx scripts/qa-coverage-report.ts --json — 155 YAML scenarios, 209 coverage IDs, and 0 missing coverage entries; duplicate validation also loaded 82 non-YAML live catalog IDs.
  • node scripts/generate-docs-map.mjs --check — passed.
  • node scripts/format-docs.mjs --check docs/concepts/qa-e2e-automation.md — passed.
  • git diff --check — passed.
  • Blacksmith Testbox tbx_01kwn17ex0myt2pp472bxc57wecorepack pnpm check:changed passed, including project typecheck, lint, guards, and runtime import-cycle validation.
  • Codex autoreview — no accepted/actionable findings, confidence 0.91 on the rebased head.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation extensions: qa-lab size: M maintainer Maintainer-authored PR labels Jul 3, 2026
@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 6:41 PM ET / 22:41 UTC.

Summary
The PR changes QA Lab coverage validation to reject duplicate scenario IDs across YAML and live transport catalogs, allows secondary-only coverage metadata, and reclassifies several helper-backed QA scenarios as supporting evidence.

PR surface: Source +78, Tests +32, Other -9. Total +101 across 27 files.

Reproducibility: yes. at source level for the linked problem: current main validates duplicate scenario IDs inside YAML only and reports live transport lanes separately. The PR head adds the missing cross-source duplicate validation path.

Review metrics: 2 noteworthy metrics.

  • Validation source expansion: 4 live catalog sources added. QA coverage now scans Discord, Slack, Telegram, and WhatsApp catalog IDs for duplicate scenario ownership.
  • Coverage metadata shape: 1 field made optional with normalized output. Secondary-only coverage authoring is accepted while downstream code still sees a normalized primary: [] shape.

Stored data model
Persistent data-model change detected: persistent cache schema: qa/scenarios/runtime/openai-web-search-minimal.yaml. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99627
Summary: This PR is the direct implementation candidate for the canonical QA scenario ownership issue it closes.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
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:

  • none.

Next step before merge

  • No automated repair is needed; this protected maintainer PR is ready for normal maintainer review and merge gating.

Security
Cleared: The diff changes QA validation, tests, and scenario metadata only; it does not add dependencies, workflows, secrets handling, or package execution surfaces.

Review details

Best possible solution:

Land this focused QA Lab validation change after normal maintainer approval and exact-head checks, then let the linked issue close through the merge.

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

Yes at source level for the linked problem: current main validates duplicate scenario IDs inside YAML only and reports live transport lanes separately. The PR head adds the missing cross-source duplicate validation path.

Is this the best way to solve the issue?

Yes: extending qa coverage is the best owner boundary because the CLI script already routes through that command and the coverage inventory owns scenario metadata, taxonomy coverage, and live lane summaries. A separate registry would duplicate the existing loader/reporting path.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool 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: This is a maintainer/member PR, so the external contributor real-behavior-proof gate does not apply.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove merge-risk: 🚨 automation: Current PR review selected no merge-risk labels.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a normal-priority QA infrastructure correctness improvement with limited end-user runtime blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a maintainer/member PR, so the external contributor real-behavior-proof gate does not apply.
Evidence reviewed

PR surface:

Source +78, Tests +32, Other -9. Total +101 across 27 files.

View PR surface stats
Area Files Added Removed Net
Source 7 81 3 +78
Tests 3 43 11 +32
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 17 18 27 -9
Total 27 142 41 +101

What I checked:

  • Repository policy applied: Root and scoped extension policy were read; they made the QA scenario-source and extension-boundary checks relevant to the review. (AGENTS.md:1, 219f27a5ec6b)
  • Current main still has the ownership gap: Current main builds coverage inventory from YAML scenarios and appends live transport lane summaries separately, while YAML duplicate checking remains local to readQaScenarioPack. (extensions/qa-lab/src/coverage-report.ts:205, 219f27a5ec6b)
  • PR head wires coverage through non-YAML catalog validation: At the PR head, runQaCoverageReportCommand passes await loadNonYamlScenarioRefs() into buildQaCoverageInventory, and the inventory rejects duplicate IDs across YAML and non-YAML source paths. (extensions/qa-lab/src/cli.runtime.ts:1238, 89d3d69123c9)
  • PR-head mock concern checked: The CLI runtime test mock is reset in beforeEach and then returns an array from listTelegramQaScenarioCatalog, so the earlier bare-mock concern does not hold on the full PR-head test setup. (extensions/qa-lab/src/cli.runtime.test.ts:275, 89d3d69123c9)
  • Linked issue is the canonical work item: Live GitHub state shows this PR closes the open maintainer issue requesting the same unified scenario ownership, duplicate-ID rejection, and secondary-only coverage metadata.
  • Relevant checks are green: Live PR checks did not report non-success required/relevant check states for the current head; the PR body also lists focused QA Lab and Testbox validation. (89d3d69123c9)

Likely related people:

  • RomneyDa: Authored this PR and has recent merged history across QA coverage IDs, YAML scenario conversion, and coverage reporting. (role: current implementation candidate owner and recent area contributor; confidence: high; commits: 15bfadf2bd1c, fef839407995, e12cf72b17b8; files: extensions/qa-lab/src/coverage-report.ts, extensions/qa-lab/src/scenario-catalog.ts, qa/scenarios/index.yaml)
  • vincentkoc: Recent history touches the live transport scenario helpers and coverage report surfaces that this PR extends. (role: recent area contributor; confidence: high; commits: f3f85ae5f75e, def4b5148573, 7cc21ef59dfa; files: extensions/qa-lab/src/live-transports/shared/live-transport-scenarios.ts, extensions/qa-lab/src/coverage-report.ts, extensions/qa-lab/src/scenario-catalog.ts)
  • Solvely-Colin: Merged script-backed evidence scenario support that shaped the scenario catalog and coverage report surfaces changed here. (role: feature-history contributor; confidence: medium; commits: 591313e80a5e; files: extensions/qa-lab/src/coverage-report.ts, extensions/qa-lab/src/scenario-catalog.ts, qa/scenarios/index.yaml)
  • mcaxtr: Recent WhatsApp live QA scenario work is part of the non-YAML catalog surface now loaded by coverage validation. (role: adjacent live transport contributor; confidence: medium; commits: b50a5aebba8f, 181238fb5304; files: extensions/qa-lab/src/live-transports/whatsapp/whatsapp-live.runtime.ts, extensions/qa-lab/src/live-transports/shared/live-transport-scenarios.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. and removed 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. labels Jul 3, 2026
@openclaw-barnacle openclaw-barnacle Bot added size: S and removed docs Improvements or additions to documentation size: M labels Jul 3, 2026
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 3, 2026
@RomneyDa RomneyDa force-pushed the dallin/e2e-wave0-ownership-coverage branch from 543be48 to 89d3d69 Compare July 3, 2026 22:29
@clawsweeper clawsweeper Bot removed the rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. label Jul 3, 2026
@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. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. 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. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. 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. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. labels Jul 3, 2026
@RomneyDa RomneyDa merged commit 677c8ff into main Jul 3, 2026
147 of 162 checks passed
@RomneyDa RomneyDa deleted the dallin/e2e-wave0-ownership-coverage branch July 3, 2026 22:51
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 4, 2026
* improve: enforce canonical QA scenario ownership

* docs: refresh QA documentation map

* docs: condense QA ownership guidance

* docs: remove QA migration guidance

* refactor: simplify QA scenario ownership checks

* docs: align QA coverage authoring guidance
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 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.

QA coverage should enforce canonical scenario ownership

1 participant