Cleanup: four high-severity findings from PR #52 review

[jcschaff]

Follow-up to PR #52, addressing the four "High" findings from the code review (those that affect real production behavior).

What's in

Finding	Where	Fix
#14 Worker pod disk leak (orphaned OMEX temp files when submit raises)	biosim_runs/activities.py::submit_biosim_run_activity	Wrap biosim_service.run_biosim_sim in try/finally; os.remove itself in try/except OSError so a missing-file race is non-fatal.
#5 FAILED-row cache pollution + cache-lookup ordering hazard	biosim_runs/activities.py::poll_biosim_run_activity	Gate insert_biosimulator_workflow_run on simulation_run.status == SUCCEEDED. Activity still returns a fully-populated (un-saved) record so OmexSimWorkflow can surface the FAILED state — it doesn't consult database_id.
#10 Missing Mongo indexes on hot lookups	three DB services + init_standalone + test fixtures	New ensure_indexes() method (default no-op on the abstract base, Mongo impls override). Called from init_standalone() and every test fixture. Indexes listed below.
#2 Status endpoint 404s after Temporal history eviction even though SimulationRunRecord rows are still in Mongo	simulations/router.py::get_simulation_status	Don't immediately 404 on workflow-query failure — try the DB fallback. New _conglomerate_status_from_records helper builds a ConglomerateStatus from persisted rows (mapping CREATED/SUCCEEDED/FAILED back to processing/success/failure). Only 404 if both sources come up empty.

Indexes created (#10)

• BiosimSimulationRuns: processing_id, run_id (unique)
• BiosimSims: compound (file_hash_md5, image_digest, cache_buster) + biosim_run.id
• BiosimOmex: file_hash_md5 (non-unique — get_cached_omex_file_from_*'s check-then-insert doesn't catch DuplicateKeyError, so logical uniqueness is enforced by the caller)

Tests (#2)

Three new router tests:

• test_get_simulation_status_falls_back_to_db_when_workflow_query_fails — workflow raises, DB has 2 records (1 SUCCEEDED, 1 FAILED) → 200 with success/failure jobs and correct biosim_run_ids.
• test_get_simulation_status_404_when_workflow_and_db_both_empty — workflow raises, DB returns [] → 404.
• test_get_simulation_status_hybrid_enriches_biosim_run_id_from_db — workflow returns biosim_run_id=None mid-run, DB has the early-write value → response merges (live status from workflow, run id from DB).

Existing test_get_simulation_status_not_found still passes (no patched runs_db → falls back to None records → 404).

#5 doesn't have a dedicated unit test (the activity's deps are tied to module-global getters; the change is a 4-line guard whose behavior is obvious from inspection, and integration_local already exercises the SUCCEEDED path end-to-end). #14 and #10 are mechanical with the existing integration tests exercising the surrounding flows.

Verification

• ruff + mypy --strict clean.
• Full pytest -m "not integration": 83 passed, 1 failed (the pre-existing test_slurm_service SSH-creds env test, unchanged through every PR in this stack).

What's left

Medium and Lower findings from the review remain open (#6 HDF5 retry off-by-one, #7 stale workflow_id on cache hit, #8 submit timeout, #9 length=100 cap, #11 ImportError, #12 CancelledError, #13 initial get_sim_run no 404 handling, #15 updated bump). #13 is the most "fires under normal conditions" of the medium group — biosimulations.org eventual consistency between submit and poll. Worth addressing next if you want to keep going.

🤖 Generated with Claude Code