WA-RAILS7-024: ActiveJob adapter compatibility

[kitcommerce]

Fixes #755.

Summary

• Ensure Workarea only forces ActiveJob to Sidekiq when the adapter is not the ActiveJob test adapter.
• Adds coverage to ensure configure_plugins! does not clobber :test.

Why

Rails test suites (and Workarea::TestCase helpers) rely on ActiveJob's test adapter features like perform_enqueued_jobs. Overriding the adapter to Sidekiq breaks those helpers.

Client impact

None expected. Production behavior remains Sidekiq-backed; test suites retain the ActiveJob test adapter.

[kitcommerce]

Build Gate Results

• rubocop: PASS (0 offenses, diff-only on 2 files)
• brakeman: N/A (no security-sensitive additions)
• tests: PASS (11 runs, 15 assertions, 0 failures, 0 errors — sidekiq_test.rb)
• Docker services: ES + MongoDB + Redis ✅

Overall: gate:build-passed — ready for Wave 1 review.

[kitcommerce]

Architecture Review

{
  "reviewer": "architecture",
  "verdict": "PASS",
  "severity": null,
  "summary": "Clean, well-scoped change that stays within the configuration module boundary with no new coupling or layer violations.",
  "findings": []
}

Analysis: The change adds a runtime guard in Workarea::Configuration::Sidekiq.configure_plugins! to avoid overriding the ActiveJob test adapter. Architecturally this is sound:

• Separation of concerns — Configuration logic remains in the configuration module. No domain or UI coupling introduced.
• Dependency direction — No new imports or cross-layer dependencies. The guard checks an existing Rails framework class (ActiveJob::QueueAdapters::TestAdapter), which is an appropriate dependency for a configuration module that already owns adapter assignment.
• Module boundaries — The change is entirely self-contained within core/lib/workarea/configuration/sidekiq.rb. No boundary violations.
• Interface design — The method continues to fulfill its single responsibility (configure plugins for Sidekiq) with an appropriate environment-awareness guard.

No architectural concerns. ✅

[kitcommerce]

Security Review

Verdict: PASS ✅

No security issues found. The change is a configuration guard that conditionally sets the ActiveJob queue adapter, with no impact on authentication, authorization, data exposure, or input handling.

Production behavior is unchanged — Sidekiq remains the queue adapter for non-test environments. The guard only preserves ActiveJob's test adapter when already set, which is expected and safe. No secrets, credentials, injection surfaces, or authorization logic is affected.

[kitcommerce]

Simplicity Review

Verdict: PASS

The change is minimal and direct. The guard in is the simplest possible expression of the intent — no new abstractions, indirection layers, strategy patterns, or generalization introduced. The test is idiomatic with correct cleanup. Nothing to flag here.

[kitcommerce]

Rails Conventions Review

Verdict: PASS_WITH_NOTES

The guard logic and test structure are idiomatic. One low-priority observation:

LOW — line ~79

The implementation uses ActiveJob::Base.queue_adapter.is_a?(ActiveJob::QueueAdapters::TestAdapter) (internal class identity) while the test assertion uses ActiveJob::Base.queue_adapter_name.to_s (the public API). These are equivalent today, but queue_adapter_name is the Rails-idiomatic way to interrogate the active adapter and avoids a direct dependency on the internal constant.

Consider aligning:

unless ActiveJob::Base.queue_adapter_name.to_s == 'test'
  ActiveJob::Base.queue_adapter = :sidekiq
end

This is consistent with how the test already checks the adapter and uses the public Rails API rather than an internal class reference. Not a blocker.

[kitcommerce]

✅ Wave 1 Review Complete

All Wave 1 reviewers returned PASS or PASS_WITH_NOTES.

Reviewer	Verdict	Notes
Architecture	✅ PASS	No concerns
Security	✅ PASS	No concerns
Simplicity	✅ PASS	No concerns
Rails Conventions	📝 PASS_WITH_NOTES	LOW: Consider queue_adapter_name.to_s == "test" over is_a?(TestAdapter) for public API alignment

Wave 1 gate: PASS → Proceeding to Wave 2 (rails-security, database, test-quality).

[kitcommerce]

Rails Security Review

{
  "reviewer": "rails-security",
  "verdict": "PASS",
  "severity": null,
  "summary": "No security-relevant changes — diff is limited to internal ActiveJob queue adapter selection logic and a unit test; no user input handling, SQL, views, auth, secrets, or redirects.",
  "findings": []
}

Clean bill of health. The change guards an internal ActiveJob::Base.queue_adapter assignment with an is_a? check against a framework class (ActiveJob::QueueAdapters::TestAdapter). No attack surface is introduced or modified:

• No user-controlled input flows into the adapter selection.
• No params, SQL, views, redirects, secrets, CSRF, auth, or file handling are touched.
• The test properly restores original state via ensure, avoiding side-effects on the test suite.

[kitcommerce]

Database Review

{
  "reviewer": "database",
  "verdict": "PASS",
  "severity": null,
  "summary": "No database impact — this PR modifies ActiveJob queue adapter configuration only; no migrations, schema changes, queries, indexes, or association declarations are affected.",
  "findings": []
}

No database-relevant changes in this diff. The PR modifies configure_plugins! to conditionally set the ActiveJob queue adapter and adds a corresponding test. No migrations, no schema modifications, no query patterns, no association changes, and no data integrity implications.

[kitcommerce]

Test Quality Review

{
  "reviewer": "test-quality",
  "verdict": "CHANGES_REQUIRED",
  "severity": "MEDIUM",
  "summary": "The added test covers the guard behavior well, but issue #755 acceptance criteria (job enqueuing, retries, scheduling, unique jobs, deprecation warnings) have no test coverage in this PR or in the observed build gate run.",
  "findings": [
    {
      "severity": "MEDIUM",
      "file": "core/test/lib/workarea/configuration/sidekiq_test.rb",
      "line": 1,
      "issue": "Issue #755 acceptance criteria — jobs enqueue correctly, retries work, scheduled jobs work, unique jobs constraint enforced, no deprecation warnings — are not covered by any test in this PR. The build gate only ran sidekiq_test.rb, leaving it unknown whether integration-level tests for these criteria exist anywhere in the suite.",
      "suggestion": "Either add integration tests covering the remaining acceptance criteria in this PR, or explicitly document (in the PR description or a follow-up issue) which ticket/test file covers each remaining criterion. Do not close #755 until all acceptance criteria have test coverage you can point to."
    },
    {
      "severity": "LOW",
      "file": "core/test/lib/workarea/configuration/sidekiq_test.rb",
      "line": 1,
      "issue": "The new guard introduces two branches: skip when TestAdapter, set :sidekiq otherwise. Only the skip branch is explicitly tested. The inverse path (adapter set to :sidekiq when not TestAdapter) has no new assertion confirming the guard does not accidentally suppress normal initialization.",
      "suggestion": "Add a companion test: with a non-test adapter set, call configure_plugins! and assert adapter_name is 'sidekiq'. This makes both branches explicit and guards against future regressions in the guard condition."
    }
  ]
}

Findings

MEDIUM — Acceptance criteria gap (no tests for retries, scheduling, unique jobs, enqueue behavior)

The fix itself is narrow and correct: the guard prevents the TestAdapter from being overridden in test environments. The new test validates that behavior cleanly, with proper isolation (ensure block restores original adapter) and a clear assertion.

The gap is at the issue level. Issue #755 carries these acceptance criteria:

• Jobs enqueue correctly
• Retries work
• Scheduled jobs work
• Unique jobs constraint enforced
• No deprecation warnings

None of these are tested here, and the build gate only ran sidekiq_test.rb — so there's no evidence these criteria are covered elsewhere. This PR should not be used to close #755 unless the remaining criteria are either tested (here or in a linked test file) or explicitly deferred to a separate tracked issue.

LOW — Only the 'skip' branch of the new guard is explicitly tested

The guard adds a conditional: if TestAdapter → skip, else → set sidekiq. The added test covers the skip case. A companion test exercising the else branch would make both paths explicit and catch any future regression where the guard condition is accidentally broadened. This is minor since the else path is pre-existing behavior, but it would complete the branch coverage story for this method.

[kitcommerce]

{
  "reviewer": "performance",
  "verdict": "PASS",
  "severity": null,
  "summary": "One-time initialization guard with negligible overhead; no performance concerns.",
  "findings": []
}

Performance Review — PASS

This change adds a single is_a? type check before setting the ActiveJob queue adapter. From a performance standpoint there is nothing to flag:

• Execution context: configure_plugins! runs once at app boot — not in any request path, job handler, or loop. Even a slow operation here would be invisible at runtime.
• The guard itself: ActiveJob::Base.queue_adapter.is_a?(ActiveJob::QueueAdapters::TestAdapter) is a constant-time Ruby method call with no I/O, no allocation, and no external dependencies.
• No hot-path impact: The change has zero effect on job enqueue throughput, ActionMailer delivery latency, or Sidekiq worker scheduling.
• Test hygiene: The test properly captures and restores original_adapter in an ensure block — no leaked global state that could cause test-order-dependent slowdowns.

Nothing to optimize here. The diff is already as lean as it can be.

[kitcommerce]

{
  "reviewer": "frontend",
  "verdict": "PASS",
  "severity": null,
  "summary": "No frontend files in this diff — pure server-side Ruby change.",
  "findings": []
}

No JavaScript, TypeScript, Stimulus, Turbo, or asset pipeline changes. This PR only modifies core/lib/workarea/configuration/sidekiq.rb and its test file. Nothing to review from a frontend perspective.

[kitcommerce]

A11y review: PASS. This PR only changes Ruby configuration/test code (ActiveJob adapter guard + test) and does not touch any user-facing UI (views, HTML/CSS/JS). No accessibility impact detected.

[kitcommerce]

{"reviewer":"accessibility","verdict":"PASS","severity":null,"summary":"No user-facing UI changes; server-side ActiveJob adapter guard only.","findings":[]}

A11y review: PASS. This PR only changes Ruby configuration/test code (ActiveJob adapter guard + test) and does not touch any user-facing UI (views, HTML/CSS/JS). No accessibility impact detected.