PROD-10283

[KartikSuthar]

Jira Issue:

https://buddyboss.atlassian.net/browse/PROD-10283

General Note

Keep all conversations related to this PR in the associated Jira issue(s). Do NOT add comment on this PR or edit this PR’s description.

Notes to Developer

• Ensure the IDs (i.e. PROD-1) of all associated Jira issues are reference in this PR’s title
• Ensure that you have achieved the Definition of Done before submitting for review
• When this PR is ready for review, move the associate Jira issue(s) to “Needs Review” (or “Code Review” for Dev Tasks)

Notes to Reviewer

• Ensure that the Definition of Done have been achieved before approving a PR
• When this PR is approved, move the associated Jira issue(s) to “Needs QA” (or “Approved” for Dev Tasks)

[claude]

Claude finished @KartikSuthar's task in 4m 7s —— View job

---

Claude finished the review (ultracode — from scratch)

Todo list

• Gather context (diff, changed files)
• Confirm the two previously-flagged issues are fixed in the latest commit
• Deep-dive on class-bb-addons-manager.php backoff wrapper (correctness, concurrency/races, static state, caching invariants)
• Independent adversarial verification of candidate findings via background agents
• Post final review

Summary

This PR adds a backoff/dedup wrapper (BB_Addons_Manager::bb_products_list_with_backoff()) around the Mothership products-list API client, installed via the shared vendor AddonsManager::$productsApiClient static property, plus two small call sites (bb-admin-placeholder-features.php, bb-admin-settings-page.php) that use productsApiErrored() to avoid mis-reporting plan/tier status during an outage.

Both issues from the previous review round are now fixed:

1. The catch block now catches \Throwable, not \Exception (class-bb-addons-manager.php:261) — so a TypeError/Error from the strictly-typed vendor client is converted into an error Response instead of propagating uncaught.
2. The cached-error early-return path (class-bb-addons-manager.php:144-152) now also tries bb_stale_products_response() before returning the raw error, matching the live-failure path — consistent with the docblock's "serve that stale catalog instead of the error" intent.

This round's deep dive

I re-traced the whole mechanism end-to-end (wrapper installation idempotency, the "in-progress" placeholder dedup, rate-limit gating, backoff clamping, stale-catalog fallback, cache invalidation on license change) plus the two small call sites, and used independent agents to adversarially verify two candidate concerns before reporting them:

• Static-state loss on re-wrap (could loadHooks() run twice and clobber self::$live_products_api_client with a stale/empty value?) — refuted. BB_Addons_Manager::loadHooks() has exactly one call site, reached only through BB_Mothership_Loader::instance()'s singleton guard and BuddyPress::instance()'s singleton guard, both non-bypassable in normal request execution. Not concretely triggerable.
• XSS via exception message ($e->getMessage() now flows into Response->error, which generateAddonsHtml() renders via unescaped sprintf()) — refuted as a new finding. The unescaped sink at class-bb-addons-manager.php:367 is pre-existing (already phpcs:ignore'd, predates this PR, and only reachable behind manage_options + an active license). The new \Throwable case exists specifically for a TypeError from implode() on a malformed payload — TypeError::getMessage() reports PHP type names, not response body content — so this PR doesn't meaningfully add exploitability over the pre-existing data flow.

No other correctness, race-condition, or caching-invariant bugs found. The design holds up: idempotent wrapper installation, bounded self-healing "in progress" placeholder (2 min), rate-limit-aware backoff clamped 5 min–1 hour, and stale-catalog fallback applied consistently across all three return paths (cached-error, rate-limited, and live-failure).

The bb-admin-placeholder-features.php and bb-admin-settings-page.php changes are small, correct, and narrowly scoped: both fall back to filesystem/install state (or suppress an upsell) only when productsApiErrored() is true, and both reuse the same _add_ons transient already populated by the preceding checkProductBySlug() call, so no extra live API call is introduced.

Remaining caveats (unchanged from last round, still can't be resolved in this environment)

• caseproof/ground-level-mothership is a private Composer package not vendored in this checkout, so Response's constructor signature, AddonsManager::$productsApiClient visibility, and Products::list()'s signature still can't be directly confirmed — the in-file comments suggest these were checked against real vendor source; worth a final confirmation in CI/a real environment.
• I was unable to run composer lint-php, php -l, or composer test in this sandbox (non-git Bash commands required interactive approval unavailable here). Please run the standard pre-commit checklist before merging.
• No test coverage was added for the new backoff logic, understandable given its tight coupling to the private vendor package's runtime behavior.

Verdict

Both prior findings are fixed, and this round's deeper adversarial pass didn't surface any new confirmed issues. Looks good to merge from a correctness/security standpoint, pending the standard lint/test run.