fix(onboard-status): report audit/scrape state truthfully during onboarding

[tkotthakota-adobe]

Problem

During site onboarding, the opportunity-status-processor status report conflated "still running" with "failed". A snapshot taken mid-onboarding wrongly showed most audits as having failed — e.g. Scraping ❌ and <audit> audit has not been executed — when the pipeline was simply still in flight. A follow-up onboard status then reported everything complete, but the initial message had already caused confusion.

Root causes & fixes

1. Audit run-status from the DB, not CloudWatch logs
Execution was inferred by grepping the audit-worker CloudWatch log group for a Received <audit> audit request line. Within the onboard wait window that line frequently hadn't landed yet, producing a false <audit> audit has not been executed. This now uses the Audit DB records already loaded for the completion check (computeAuditCompletion): an audit that hasn't completed is reported as in progress (⏳), never as "not executed". The fragile/slow CloudWatch path is removed entirely (cloudwatch-utils.js + its test and obsolete CloudWatch test blocks deleted).

analyzeMissingOpportunities is now a pure function (DB-driven, no I/O) and unit-tested directly.

2. Scraping tri-state
scrapingAvailable was binary (completedCount > 0 ? white_check_mark : x), so a scrape still running (0 complete, many URLs PENDING/RUNNING) showed as failed. New deriveScrapingStatus returns available / in_progress / failed / unknown, rendered as ✅ / ⏳ / ❌. The stats message also surfaces an In progress: N count (explains snapshots where total far exceeds completed + failed).

3. Delivery-type-aware expectations
security-vulnerabilities only runs for AEM_CS delivery type, so on aem_edge/other sites it can never produce an opportunity, yet was reported as missing/failed. New isOpportunityApplicableForDeliveryType filters expectedOpportunityTypes by the site's delivery type.

Tests

• New unit tests: analyze-missing-opportunities, derive-scraping-status, delivery-type-applicability, plus a delivery-type integration test.
• Full suite 414 passing / 0 failing; npm test coverage gate passes; lint clean.
• Net ~800 fewer lines (mostly dead CloudWatch code/tests).
• Slack command run: https://cq-dev.slack.com/archives/C060T2PPF8V/p1785454924075159

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

This PR will trigger a patch release when merged.

[tathagat2241]

Review: fix(onboard-status): report audit/scrape state truthfully during onboarding

Bugfix (+437/−1006; net simplification, mostly dead CloudWatch removal). Fixes the "still running" vs "failed" conflation in the onboard status report across three surfaces: audit run-status (now DB-driven, not CloudWatch log-grepping), scraping (binary → tri-state), and delivery-type-aware opportunity expectations. I focused on the one place this could go subtly wrong — the delivery-type casing — and verified it against the authoritative enum.

Blockers

• None.

Should-fix

• None.

Nits / things to confirm

• True audit-level failures now read as "in progress" (⏳), not "failed." Removing the CloudWatch path also removed the only audit-crash failure signal — a genuinely failed audit that wrote no completion record now shows as in-progress within the onboard window (dependency failures are still surfaced as ❌). This is a deliberate, sound tradeoff (eliminating false failures during a bounded, snapshot-then-follow-up flow), but worth a PM nod that a crashed audit is no longer distinctly flagged by this processor.
• deriveScrapingStatus treats any non-{COMPLETE, PENDING, RUNNING} URL status as failed (when nothing has completed): a scrape-URL in some other non-terminal state (e.g. QUEUED/PROCESSING, if those exist) would read as failed rather than in-progress, and unknown (total 0) renders as ❌. Worth confirming the scrape-URL status enum is exactly those four; otherwise widen the pending predicate.
• Rare label inconsistency: if an audit is completed but scraping still shows 0-completed, the top-line scraping shows ⏳ (tri-state) while the missing-opportunity analysis still evaluates serviceStatus.scraping as an unavailable dependency (binary ❌). Very low likelihood (audits need scraping first), just noting the two surfaces can disagree in that narrow ordering.

Strengths

• Delivery-type filter casing is correct — verified against the source of truth. DELIVERY_TYPES.AEM_CS === 'aem_cs' (lowercase) in spacecat-shared-utils, and the audit-worker gates on exactly site.getDeliveryType() !== DELIVERY_TYPES.AEM_CS, so the PR's lowercase {'security-vulnerabilities': ['aem_cs']} matches — it keeps the opportunity on AEM CS sites and drops it elsewhere, as intended. The map is conservatively scoped to the one known hard gate, avoiding hiding legitimately-producible opportunities, with a comment pointing at the audit-worker source.
• analyzeMissingOpportunities is now a pure, DB-driven function (no CloudWatch I/O), unit-tested directly. computeAuditCompletion is computed once and its completed/pending split is reused as the single source for both the in-progress (⏳) opportunity statuses and the missing-opportunity analysis — and the !completed.has(auditType) in-progress check correctly short-circuits before the dependency check, so an in-flight audit reads as ⏳ rather than a dependency failure.
• Removes a fragile, slow path (CloudWatch log-grepping for Received <audit> audit request, which routinely hadn't landed within the wait window) and ~1000 lines of dead code/tests with it.
• Conservative DB-error fallback — on an Audit.allLatestForSite failure, everything is marked pending so nothing is misreported as failed/executed and the "may still be in progress" disclaimer shows.
• Good targeted tests — one unit test file per new pure function (analyze-missing-opportunities, derive-scraping-status, delivery-type-applicability) plus a delivery-type integration test.

Tests

Strong for the changed surfaces — the three new pure functions are tested directly and the delivery-type unit test uses lowercase inputs matching the verified enum. I could not execute the suite here, so 414 passing / 0 failing is trusted rather than confirmed.

Verdict

Approve. Correctly fixes the running-vs-failed conflation on all three surfaces, with the delivery-type casing verified safe against the authoritative DELIVERY_TYPES enum and the audit-worker's own gate, plus a healthy dead-code removal and direct unit tests. The nits are minor (an intended failure-reporting tradeoff and two edge-case labelings), none blocking.

[tkotthakota-adobe]

Thanks for the thorough review 🙏 — responses to the three nits:

1. Crashed audits now read as ⏳ instead of a distinct failure. Confirmed intentional. This is the deliberate tradeoff: a hard-crashed audit writes no Audit record, so within the bounded onboard window it reads as in-progress rather than being falsely flagged. Dependency failures still surface as ❌, and the follow-up onboard status re-check (plus the "may still be in progress" disclaimer) closes the loop. Noting for PM visibility as you suggested — the processor no longer distinctly flags an audit crash, by design.

2. deriveScrapingStatus non-terminal predicate. Verified against the source enum. scrape_url_status is exactly { PENDING, RUNNING, REDIRECT, COMPLETE, FAILED, STOPPED } (@mysticat/data-service-types). PENDING and RUNNING are the only non-terminal states, so the pending predicate is complete — there is no QUEUED/PROCESSING. REDIRECT/FAILED/STOPPED are terminal non-successes and correctly read as failed when nothing has completed. Added a clarifying comment citing the enum in e77ebaf so this is self-documenting.

3. Tri-state (⏳) vs binary dependency (❌) label disagreement. Acknowledged as a known, near-impossible edge: the missing-opportunity analysis only reaches the serviceStatus.scraping dependency check for a completed audit, and every scraping-dependent audit must have a successful scrape to complete — so a "completed audit + 0-completed scrape" ordering effectively can't occur. Leaving as-is to avoid changing the pure function's semantics on an approved PR; happy to align the two surfaces in a follow-up if we ever see it in practice.

[solaris007]

🎉 This PR is included in version 1.16.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀