feat: onboard status command

[tkotthakota-adobe]

https://jira.corp.adobe.com/browse/SITES-41933

• Onboard status command to check the status of previously run onboard site command.

Tests:
Check onboard status immediately after onboard site command returned with onboard completed slack message.
https://cq-dev.slack.com/archives/C060T2PPF8V/p1774835519747329

[github-actions]

This PR will trigger no release when merged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[solaris007]

Hey @tkotthakota-adobe,

Strengths

• Clean separation of concerns: computeAuditCompletion() extracted as a pure, exported function with no DB calls, easy to test in isolation (onboard-status.js:106-134)
• Thorough test suite: 507 lines covering argument validation, site lookup, opportunity status rendering, dedup, filtering, grammar, error recovery, and unit tests for computeAuditCompletion
• Graceful degradation: try/catch around LatestAudit.allBySiteId logs a warning and continues rather than failing the whole command (onboard-status.js:176-187)
• Defensive null handling throughout: optional chaining for getConfig(), getHandlers(), handlers?.[auditType]?.lastAuditRunTime
• Follows established command patterns: factory function shape, BaseCommand usage, registration in commands.js

Issues

Important (Should Fix)

1. Direct mutation of siteConfig.state.handlers bypasses Config model encapsulation - utils.js:1362

Every other mutation in this function uses accessor methods (enableImport(), updateFetchConfig()). This line reaches into siteConfig.state directly, coupling the code to the internal storage shape. If the Config model is refactored, this breaks silently. Additionally, there is zero test coverage for this write path - onboard.test.js stubs out onboardSingleSite entirely.

Fix: Use a Config setter if one exists, or add one. At minimum, add a test verifying lastAuditRunTime is written correctly.

2. Duplicated AUDIT_OPPORTUNITY_MAP will drift - onboard-status.js:42-53

The comment says "Kept in sync with audit-opportunity-map.js in spacecat-task-processor." Manual sync across repos is a maintenance hazard. When a new audit type is added to the task processor, this map will silently become stale.

Fix: Extract to a shared package, or at minimum add a tracking issue and a comment with a link to the canonical source.

3. Two divergent "audit completion" implementations across the paired PRs

This PR uses lastAuditRunTime from config handlers + LatestAudit.allBySiteId. The companion PR #218 uses onboardStartTime from message context + Audit.allLatestForSite. Same logic, different anchor points and data sources - they can disagree about whether an audit is pending.

Fix: Converge on a single anchor. Since lastAuditRunTime is persisted to site config, the task-processor should read from there too.

4. Sequential await in for...of loop for getSuggestions - onboard-status.js:188-196

Each opportunity's getSuggestions() is awaited sequentially. For 9+ opportunity types, that is 9+ serial round-trips. These are independent reads.

Fix: Use Promise.all to parallelize.

5. getAuditTitle called with opportunity types - onboard-status.js:247

getAuditTitle(oppType) is called with opportunity types, but the title map only has audit type entries. For experimentation-opportunities -> high-organic-low-ctr, the fallback produces "High Organic Low Ctr" rather than a meaningful name.

Fix: Add proper title mappings for opportunity types, or rename the function to reflect its actual usage.

6. Boundary condition: auditedAt === lastAuditRunTime - onboard-status.js:123

The < comparison means exact equality is treated as "completed." Since lastAuditRunTime is written via Date.now() at trigger time, an audit whose timestamp matches was likely in-flight. No test covers this boundary.

Fix: Document the intended behavior with a test case, and consider <= instead.

Minor (Nice to Have)

7. Multiple say() calls create Slack message fragmentation - onboard-status.js:200-217 - Consider building a single response string.

8. error.message leakage to Slack - onboard-status.js:294 - Could expose internal details (DB connection strings, hostnames). Pre-existing pattern across other commands, but worth a follow-up to sanitize.

9. Duplicated getAuditTitle - onboard-status.js:26-37 - Same duplication concern as the map.

Recommendations

• Extract AUDIT_OPPORTUNITY_MAP, getAuditTitle, and computeAuditCompletion to a shared package (@adobe/spacecat-shared-utils or similar). The current "keep in sync manually" approach does not scale.
• Converge on lastAuditRunTime (persisted in site config) as the single anchor for audit completion across both repos - it is durable, per-audit-type, and available to any consumer.
• Consider adding a timeout/expiry to the "pending" state. If an audit fails silently (never writes a DB record), the hourglass will show indefinitely on every recheck.

Assessment

Ready to merge? With fixes

The feature is well-structured and thoroughly tested. The blocking concerns are the untested direct siteConfig.state.handlers mutation (#1), the sequential DB calls (#4), and the audit-type/opportunity-type naming mismatch (#5). The cross-repo duplication (#2, #3) can be addressed via a tracked follow-up if extracting to shared is out of scope for this PR.

Next Steps

1. Address the siteConfig.state.handlers mutation (#1) and add test coverage for the write path.
2. Parallelize getSuggestions calls (#4).
3. Fix the getAuditTitle / opportunity type mismatch (#5).
4. Create a follow-up Jira ticket for the shared package extraction (#2, #3).
5. Minor items are optional.

[tkotthakota-adobe]

@solaris007 Thanks for the review comments. First I will try to move shared maps to common lib in this PR by working with @rpapani. data-access or utils are in my mind, if you have any recommendation let us know.

[tkotthakota-adobe]

@solaris007 Review comments are addressed. Thanks for the review. Test results are added to PR description.

  API Service PR — Review Comments Addressed
                                                                         
  1. computeAuditCompletion moved to shared library         
  The local computeAuditCompletion function that was defined in          
  onboard-status.js has been removed. It now lives in                    
  @adobe/spacecat-shared-utils (src/opportunity/audit-completion.js) and 
  is imported from there:                                                
  import { ..., computeAuditCompletion } from               
  '@adobe/spacecat-shared-utils';            
                                 
  2. Anchor point is consistent with task-processor
  lastStartTime is read from                                             
  site.getConfig()?.getOnboardConfig()?.lastStartTime — the same value   
  that was written by the api-service during onboarding (Date.now()) and 
  passed to the task-processor via the SQS message. Both consumers now   
  use the identical shared function with the same semantics.

  3. Pending check uses all map-known audit types                        
  computeAuditCompletion is called against
  Object.keys(AUDIT_OPPORTUNITY_MAP) (all known types), not just the     
  types that happen to have DB records — so audits not yet started (no DB
   record) are correctly identified as pending.                          
                                                            
  4. getSuggestions calls are parallelised                               
  Opportunity statuses are built with
  Promise.all(visibleOpportunities.map(...)) — no sequential await in a  
  loop.                                                     
                                                                         
  5. Legacy sites (no lastStartTime) handled gracefully                  
  Shared computeAuditCompletion falls back to treating any existing audit
   record as completed when onboardStartTime is absent — same behaviour  
  as before, no regression.                                 
                                                                         
  6. Test import updated                                    
  onboard-status.test.js now imports computeAuditCompletion from
  @adobe/spacecat-shared-utils directly, matching the production import. 

[tkotthakota-adobe]

Hi @solaris007 I already addressed all review comments. Let me know if any further comments.

[solaris007]

Hey @tkotthakota-adobe,

All 9 findings from the prior review are resolved. Clean rework.

Previously Flagged Issues - Resolution Status

• Direct siteConfig.state.handlers mutation (Important): Resolved - now uses siteConfig.updateOnboardConfig() via the Config model API (utils.js:1610).
• Duplicated AUDIT_OPPORTUNITY_MAP (Important): Resolved - now imported from @adobe/spacecat-shared-utils.
• Divergent audit completion implementations (Important): Resolved - both api-service and task-processor now use computeAuditCompletion from shared-utils with the same lastStartTime anchor.
• Sequential getSuggestions calls (Important): Resolved - parallelized via Promise.all (onboard-status.js:143-155).
• getAuditTitle called with opportunity types (Important): Resolved - now uses getOpportunityTitle from shared-utils, designed for opportunity types.
• Boundary condition auditedAt === lastAuditRunTime (Important): Resolved - computeAuditCompletion uses <= with documented JSDoc. Explicit boundary test at line 1008-1016.
• Multiple say() calls, error.message leakage, duplicated helpers (Minor): say pattern is consistent with other commands. Duplication resolved via shared-utils. Error message leakage is a pre-existing cross-command pattern.

Strengths

• Single onboardTime timestamp computed once and reused for both config persistence and task context (utils.js:1609-1610). Eliminates the timestamp drift between the two that the prior review flagged.
• Promise.all for opportunity status resolution (onboard-status.js:143-155) - addresses the sequential DB call concern cleanly.
• Comprehensive 514-line test file covering: argument validation, site lookup, pending/complete status rendering, dedup, filtering by expected audit types, unknown audit type handling, grammar (singular/plural), error recovery, null config, and computeAuditCompletion unit tests including the boundary case.
• The command checks ALL map-known audit types for pending status (onboard-status.js:106-109), not just those with existing DB records. This means audits that haven't started yet are correctly shown as pending.

Assessment

Ready to merge. All prior findings addressed, test coverage is thorough, and the shared-utils integration aligns cleanly with the companion PRs (task-processor PR 218, audit-worker PR 2220).

[solaris007]

🎉 This PR is included in version 1.412.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀