refactor(execution_engine): drop unnecessary Mutex around EngineLookup, fix JS runner error message - #83
Merged
Conversation
…p, fix JS runner error message EngineLookup's methods only take &self - there's no mutation - so the Mutex wrapping it in Engine (and the PoisonedLock error variant it required) was unneeded synchronization inherited from apid's own mutation needs, not the trait's contract. apid's OperationRepos is legitimately mutated elsewhere (background loader), so that lock is now encapsulated behind a new LockedLookup adapter instead of leaking into Engine's field type. Also fixes a copy-paste bug where Engine::run's JAVASCRIPT SimpleCode dispatch reported "Code runner not found for python" on a missing JS runner, by factoring the duplicated Python/JS dispatch blocks into one dispatch_code_runner helper parameterized on the language key. Closes #16 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019EKR96FrBygNE1sfscQ6CG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Two distinct fixes from issue #16, both in
usecases/execution_engine:1. Drop the unnecessary
MutexaroundEngineLookupEngineLookup's methods (get_service,get_credentials) only take&self— there's no mutation in the trait's contract. TheMutexwrapping it inEngine(Arc<Mutex<dyn EngineLookup + Send + Sync>>) was unneeded synchronization, inherited fromapid's own unrelated mutation needs rather than required by the trait itself. Confirmedstorage/in_memory_storage::OperationReposalready implementsEngineLookupunlocked (&self-only), proving the lock was purely incidental.Engine.lookupis nowArc<dyn EngineLookup + Send + Sync>(noMutex).error::ExecutionEngine::PoisonedLockvariant is removed.apid'sOperationReposis legitimately mutated elsewhere (the background loader), so that lock is preserved but now encapsulated behind a newLockedLookupadapter (binary/apid/src/main.rs) that implementsEngineLookupby locking internally —Engineitself no longer needs to know a lock exists.runners/workflow_runner's test fixtures updated to match the new unlocked signature.2. Fix a copy-paste bug in
Engine::run'sSimpleCodedispatchThe
JAVASCRIPTbranch's "runner not found" error incorrectly reported"Code runner not found for python"(copy-pasted from thePYTHONbranch). Fixed by factoring the duplicated Python/JS dispatch blocks into onedispatch_code_runnerhelper parameterized on the language key — this structurally prevents the class of bug (the error message is now generated from the actuallang_keyparameter, not a branch-specific hardcoded string).Closes #16
Test plan
run_reports_the_correct_language_when_no_javascript_code_runner_is_registered, verified genuine red (reintroduced the"python"bug temporarily, confirmed the test fails for the right reason) before confirming green.cargo build --workspace --all-features— clean.cargo test --workspace --all-features— all green, 0 failures.cargo clippy -p execution_engine -p workflow_runner -p apid --all-targets --all-features— no new errors/warnings introduced.cargo fmt --all -- --check— clean.Generated by Claude Code