test(backend): add workflow run/delete race and disabled-workflow cov…

[shravanithouta108]

Description

Adds focused regression tests for workflow run/delete race conditions and disabled-workflow behavior as requested in issue #570.

Related Issues

Closes #570

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Testing

How Has This Been Tested?

All tests use in-memory stubs (no live DB or scheduler required) and are fully deterministic in CI.

The following scenarios are covered:

• DELETE during active run — scheduler tick after deletion is silent (no tasks created); manual run raises KeyError
• Toggle/disable during run — scheduler tick on disabled workflow is a no-op; manual run raises ValueError
• Disabled workflow behavior — zero tasks created, DB state unchanged after skipped run
• Concurrency limiter — tasks marked failed with reason when limiter is full, both via scheduled and manual paths
• Determinism — repeated runs produce consistent task counts, steps missing plugin_id are silently skipped

To run:
pytest testing/backend/test_workflow_race_and_disabled.py -v

Checklist

• My code follows the code style of this project.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.

[shravanithouta108]

Hi @utksh1 — the two failing CI checks are unrelated to this PR:

backend-lint is failing on backend/secuscan/workflows.py:82 (F821 undefined name db) which is a pre-existing issue in main, not in my test file
frontend-checks is failing on ToolConfigDynamic.test.tsx, ToolConfigTimeout.test.tsx, and Workflows.test.tsx — all files outside the scope of this PR

My change is a single new test file testing/backend/test_workflow_race_and_disabled.py with no modifications to any existing files. Happy to rebase once the upstream issues are resolved.

[utksh1]

Thanks for adding workflow race/disabled-workflow coverage. This cannot be accepted while required CI is red: backend-lint and frontend-checks fail, with backend-tests/benchmark skipped. Please rebase on the current CI baseline and fix the failures before requesting review again.

[shravanithouta108]

Hi @utksh1 — rebased on the current upstream/main. The backend-lint failure is in backend/secuscan/workflows.py:82 (F821 undefined name db) which is a pre-existing issue in main not introduced by this PR. The frontend-checks failures are also in existing frontend test files outside the scope of this PR. My change only adds a single new test file testing/backend/test_workflow_race_and_disabled.py with no modifications to any existing files. Ready for review.

[utksh1]

Thanks for the updates. This still needs changes before merge: the tests define local fake WorkflowRunner/FakeDB/FakeExecutor implementations instead of exercising the actual SecuScan workflow scheduler/routes/executor code. Please rewrite these as focused tests against the production workflow code paths (with mocks at the real dependency boundaries) so the coverage would fail if the app implementation regresses.

[utksh1]

Re-reviewed the latest push. The rewrite now uses real routes, which is good, but the tests call /api/workflows while the backend router is mounted at /api/v1. Please update route paths to /api/v1/workflows, keep auth/test_client behavior aligned with existing tests, and rerun checks.

[shravanithouta108]

Hi @utksh1 — updated all route paths from /api/workflows to /api/v1/workflows to match the actual router mount prefix. Ready for re-review. 😊

[utksh1]

Re-reviewed after the route-prefix update. The tests still patch backend.secuscan.routes.db.fetchone, db.fetchall, and db.execute, but routes.py does not expose a module-level db object; the route handlers call get_db locally. Please patch backend.secuscan.routes.get_db to return a mock database object, or use the existing test DB fixture pattern, so these tests exercise the real route DB boundary.

[shravanithouta108]

Hi @utksh1 — the get_db patching is now fixed. All tests patch backend.secuscan.routes.get_db as a plain async function returning the mock, matching the db = await get_db() call shape in routes.py. No module-level db is patched anywhere. The only failing check is frontend-checks due to the pre-existing esbuild vulnerability (GHSA-gv7w-rqvm-qjhr, CVSS 8.1) on main — this PR adds only a backend test file with zero frontend changes.

[utksh1]

Rechecking after the latest get_db patch: this is still blocked.

The frontend-checks job is failing on the current head. Please get required CI green before this backend workflow race/disabled-workflow coverage can be reconsidered.

[utksh1]

Rechecking after the latest audit-config whitespace commit: this is still blocked.

Please keep this workflow race/disabled-workflow test PR focused on backend test coverage. Remove unrelated audit-config changes from the branch so the test-only patch can be reviewed cleanly.

[utksh1]

Thanks for the update. This still is not mergeable as coverage for workflow run/delete races: the tests define their own fake WorkflowRunner and fake DB/executor instead of exercising the actual SecuScan workflow routes/scheduler/executor code. That means the tests can pass while the real workflow implementation is still broken. Please rewrite these as focused tests against the real workflow API/service behavior and keep the PR test-only.

[shravanithouta108]

Hi @utksh1 — the frontend-checks failure is now a new high severity CVE in vite (GHSA-fx2h-pf6j-xcff) that appeared on main after my last rebase. The esbuild exception is now working correctly ([OK] GHSA-gv7w-rqvm-qjhr: esbuild (excepted)). This PR is test-only with no frontend or dependency changes. Could you add the vite exception to .audit-config.yaml on main so this PR can be evaluated on its actual changes?

[utksh1]

Thanks for updating this, but it still is not mergeable. The test builds a local fake WorkflowRunner instead of exercising the real workflow scheduler/routes behavior, so it can pass while the production workflow code remains broken. Please rewrite this against the real app/test_client or real scheduler primitives and avoid duplicating production workflow logic inside the test.

[shravanithouta108]

Hi @utksh1 — I've re-checked the current head (d4141b8) carefully. There is no WorkflowRunner, FakeDB, or FakeExecutor class anywhere in the file — you can verify with:

grep -n "WorkflowRunner\|FakeDB\|FakeExecutor\|class Fake" testing/backend/test_workflow_race_and_disabled.py

This returns nothing. Every test:

• Uses the shared test_client fixture from conftest.py (real DB init, real plugins, real auth)
• Sends real HTTP requests to the actual route handlers (POST/DELETE/PATCH/GET /api/v1/workflows...)
• Only patches backend.secuscan.routes.get_db's return value — exactly the pattern you requested in your earlier get_db review

If the production workflow run/delete/update logic regresses, these tests would fail. Could you point me to the specific line you're concerned about, so I can address it precisely?

Separately, the only failing check is frontend-checks, blocked by the vite CVE (GHSA-fx2h-pf6j-xcff) on main — could you add an exception for it in .audit-config.yaml on main, similar to the esbuild one?

[utksh1]

Closing due to unresolved review feedback.

[shravanithouta108]

For the record: the final version of this PR (commit d4141b8) contained no fake WorkflowRunner, FakeDB, or FakeExecutor — all tests called the real /api/v1/workflows routes through the shared test_client fixture, with only get_db's return value mocked, exactly as requested in the prior review. Happy to reopen and continue if reconsidered.