PR #64594: fix(qa-lab): do not crash CLI startup when qa scenario pack is absent

• Repository: openclaw/openclaw
• Author: navarrotech
• State: open | merged: False
• Link: https://github.com/openclaw/openclaw/pull/64594

Description (problem / solution / changelog)

Summary

• Problem: openclaw completion (and potentially other CLI entry points) crashes immediately on production installs with Error: qa scenario pack not found: qa/scenarios/index.md. Reported in #64522.
• Why it matters: This is a CLI-unusable-in-production bug. Any user who installs a distribution without the qa/ directory (which is intentionally excluded from packaged builds) gets a dead CLI — no command registration, no shell completion, no graceful fallback. The crash happens before openclaw --help can even print.
• What changed: Return undefined from readQaScenarioExecutionConfig when the scenario pack index is not shipped on disk, instead of throwing. Add an exported isQaScenarioPackAvailable() helper for the check. The caller in discovery-eval.ts already has a fallback-to-defaults path (config?.requiredFiles ?? [hardcoded defaults]) that was written in anticipation of this exact case — we just weren't reaching it because the library layer threw first.
• What did NOT change (scope boundary): readQaScenarioPack(), readQaScenarioById(), readQaBootstrapScenarioCatalog(), and the YAML parse paths all continue to throw loudly on malformed packs, unknown scenario ids, missing fences, and zod validation failures. Only the "pack file is not present on disk at all" case is softened. discovery-eval.ts itself is not modified — once the library returns undefined, the existing fallback pattern takes over.

Change Type (select all)

• Bug fix
• Feature
• Refactor required for the fix
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• Memory / storage
• Integrations
• API / contracts
• UI / DX
• CI/CD / infra

Linked Issue/PR

• Closes #64522
• Related #
• This PR fixes a bug or regression

Root Cause (if applicable)

• Root cause: extensions/qa-lab/src/discovery-eval.ts line 17 runs readQaScenarioExecutionConfig("source-docs-discovery-report") at module-load time via a top-level constant initializer:

  const REQUIRED_DISCOVERY_REFS = readRequiredDiscoveryRefs();

  readQaScenarioExecutionConfig calls readQaScenarioById → readQaScenarioPack, which calls readTextFile(QA_SCENARIO_PACK_INDEX_PATH). In production builds the qa/ directory is not shipped, so resolveRepoPath returns null, readTextFile returns "", and readQaScenarioPack throws qa scenario pack not found: qa/scenarios/index.md. That throw propagates up the module graph and crashes the CLI before command registration finishes.

• Missing detection / guardrail: readQaScenarioExecutionConfig's declared return type is already Record<string, unknown> | undefined, signaling that callers should handle undefined. But the only undefined-returning path was the inner execution?.config chain — the outer "pack file missing" case threw instead. The caller in discovery-eval.ts was written under the (reasonable) assumption that a missing pack would produce undefined, and wrote the ?? [hardcoded defaults] fallback accordingly. That fallback was never reached because the exception short-circuited it.

• Contributing context (if known): The bundled CLI (suite-BW4kSK9C.js in the reporter's compiled output) imports the qa-lab extension's command tree as part of registration, and any module in that import chain running a throwing top-level statement takes down the whole CLI. The reporter independently found a sed-based workaround (s/readQaScenarioExecutionConfig("source-docs-discovery-report")?.requiredFiles/undefined/) that proves the fallback defaults are fine — which they are, since they match the current qa/scenarios/index.md contents at the relevant keys.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
  • Seam / integration test
  • End-to-end test
  • Existing coverage already sufficient
• Target test or file: extensions/qa-lab/src/scenario-catalog.test.ts (new describe block: "when the pack is not shipped")
• Scenario the test should lock in: Four cases:
  1. Sanity: isQaScenarioPackAvailable() returns true in the dev repo (no fs stub).
  2. isQaScenarioPackAvailable() returns false when fs.existsSync is stubbed via vi.spyOn to simulate the production-build state where the qa/ directory is absent.
  3. readQaScenarioExecutionConfig("source-docs-discovery-report") returns undefined (instead of throwing) when the pack is absent.
  4. The bug itself: import("./discovery-eval.js") under stubbed-absent-pack resolves successfully, with reportsMissingDiscoveryFiles still functional and using the hardcoded defaults. This is the user-visible regression test — it verifies the CLI startup import path no longer crashes in production builds.
• Why this is the smallest reliable guardrail: The library layer is pure enough that vi.spyOn(fs, "existsSync") is sufficient to simulate the production state without a sandbox tempdir or real filesystem manipulation. The guardrail is a truth table over the isQaScenarioPackAvailable() values, and the import-time test specifically locks in the no-crash-on-load invariant that failed in #64522.
• Existing test that already covers this (if any): None. The existing tests in scenario-catalog.test.ts all run in the dev repo where the pack is present, so the "pack absent" code path was never exercised.
• If no new test is added, why not: N/A — four new tests added.

User-visible / Behavior Changes

• Users installing OpenClaw in production environments (where qa/ is excluded) will now be able to start the CLI normally. openclaw completion, openclaw --help, and every other command should work rather than crashing at module load.
• In the degraded state (pack absent), discovery-eval's REQUIRED_DISCOVERY_REFS falls back to its hardcoded defaults (repo/qa/scenarios/index.md, repo/extensions/qa-lab/src/suite.ts, repo/docs/help/testing.md) instead of the pack-supplied list. These are QA-surface defaults; since the pack itself isn't shipped, the discovery-eval functions are in a no-op state for production users anyway, and any future code that actually runs QA discovery will need the pack to be installed explicitly.
• No change in dev environments or for anyone who ships the qa/ directory alongside their install.

Diagram (if applicable)

Before:
  CLI startup
    -> import @openclaw/qa-lab
    -> discovery-eval.ts top-level init
    -> readQaScenarioExecutionConfig("source-docs-discovery-report")
    -> readQaScenarioPack()
    -> readTextFile("qa/scenarios/index.md") -> "" (file missing)
    -> throw "qa scenario pack not found: ..."
    -> CLI crash before any command registers

After:
  CLI startup
    -> import @openclaw/qa-lab
    -> discovery-eval.ts top-level init
    -> readQaScenarioExecutionConfig("source-docs-discovery-report")
    -> isQaScenarioPackAvailable() -> false (no pack file resolved)
    -> return undefined
    -> discovery-eval.ts: config?.requiredFiles ?? [hardcoded defaults]
    -> REQUIRED_DISCOVERY_REFS = [hardcoded defaults]
    -> module loads cleanly, CLI continues normal registration

Security Impact (required)

• New permissions/capabilities? No
• Secrets/tokens handling changed? No
• New/changed network calls? No
• Command/tool execution surface changed? No
• Data access scope changed? No — the only new filesystem read is the existence check in isQaScenarioPackAvailable(), which uses the same resolveRepoPath walk-up already used everywhere else in this file.

Repro + Verification

Environment

• OS: tested on Windows 11, Node 22
• Runtime/container: vitest run against the local checkout
• Model/provider: N/A
• Integration/channel: qa-lab extension CLI import path
• Relevant config: production build (no qa/ directory)

Steps

1. Stub fs.existsSync to return false for any candidate walk-up path (simulating a production build where qa/ is not shipped).
2. Call isQaScenarioPackAvailable() — expect false.
3. Call readQaScenarioExecutionConfig("source-docs-discovery-report") — expect undefined (not a throw).
4. await import("./discovery-eval.js") — expect the module to load successfully with its hardcoded fallback refs in effect.

Expected

• No throw. discovery-eval functions load and remain usable.

Actual

• Matches expected. Covered by the new tests in scenario-catalog.test.ts.

Evidence

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording
• Perf numbers (if relevant)

Test output after the fix:

 RUN  v4.1.4
 Test Files  1 passed (1)
      Tests  8 passed (8)

Previously the scenario-catalog.test.ts file had 4 tests (all covering the happy path with the pack present). This PR grows it to 8: 4 existing + 4 new for the absent-pack fallback.

Also verified:

• discovery-eval.test.ts still green after the change (13/13 pass).
• oxlint on both touched files: Found 0 warnings and 0 errors.

Human Verification (required)

• Verified scenarios:
  • Ran scenario-catalog.test.ts locally: 8/8 pass.
  • Ran discovery-eval.test.ts locally: 13/13 pass (unchanged behavior).
  • Stepped through the resolveRepoPath walk-up logic to confirm that a returned null correctly collapses to an empty-string readTextFile result, and that the new isQaScenarioPackAvailable() short-circuit covers that case without double-I/O in the happy path (the check is O(walk-up) and then the actual read does the same walk, which is acceptable for a non-hot-path function).
  • Ran oxlint on both touched files: clean.
  • Stashed the branch and confirmed the pre-existing qa-agent-workspace.test.ts symlink failure on Windows is not caused by this change — it fails identically on clean main because fs.symlink requires admin/developer-mode on Windows. Called out here to head off confusion if CI on Linux shows that test passing while a local Windows run shows it red.
• Edge cases checked:
  • Pack absent → undefined (intended).
  • Pack present, but id unknown → still throws unknown qa scenario: <id> via readQaScenarioById (intended — caller bug).
  • Pack present, YAML malformed → still throws from zod parse (intended — pack bug).
  • Pack present, fence missing → still throws from the fence-extract helper (intended — pack bug).
• What I did NOT verify:
  • I did not physically build a production dist, rip out the qa/ directory, and run openclaw completion against it. The simulated-absence test via vi.spyOn(fs, "existsSync") exercises the same resolveRepoPath path the reporter's crash goes through, which is sufficient evidence for a unit-test-level lock-in.

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

• Backward compatible? Yes — existing dev and installed-with-pack environments behave identically. Only the currently-broken production-without-pack case changes behavior (from "crash" to "start with fallback defaults").
• Config/env changes? No
• Migration needed? No

Risks and Mitigations

• Risk: A future developer might assume readQaScenarioExecutionConfig always returns a non-undefined value and introduce a NPE at a new call site.
  • Mitigation: The declared return type is already Record<string, unknown> | undefined, so TypeScript enforces the nullability. The existing caller in discovery-eval.ts already uses ?. chaining and ?? fallback correctly.
• Risk: A malformed pack (bad YAML, missing fence, failed zod) could previously also produce this same qa scenario pack not found text if somehow readTextFile returned empty for a corrupt file. We're now silent in that narrow case.
  • Mitigation: readTextFile returns empty only when resolveRepoPath returns null (file does not exist at all). A file that exists but contains garbage would return the garbage string, which then flows into the YAML/zod path and throws loudly. So the "silent fallback" is strictly scoped to "file not present on disk."
• Risk: Doubling the filesystem walk in the happy path (isQaScenarioPackAvailable() + readQaScenarioById() both walk up to find the pack).
  • Mitigation: Each walk is a handful of fs.existsSync / fs.statSync calls, which are microseconds on any modern filesystem. This function is not on a hot path — it runs once per discovery-eval module load. The clarity of the early-return guard outweighs the micro-cost.

Changed files

• extensions/qa-lab/src/scenario-catalog.test.ts (modified, +44/-1)
• extensions/qa-lab/src/scenario-catalog.ts (modified, +25/-0)