fix(store): /api/store/resolve 500 from registry.get_app typo#358
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR refactors manifest lookup calls from ChangesRegistry Lookup Refactoring
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Reviewed by grok-code-fast-1:optimized:free · 143,162 tokens |
The fixture was mocking the broken-but-historical .get_app attribute. After fix(store): registry.get_app -> registry.get the test would have failed because MagicMock returned a generic Mock from .get instead of the qwen manifest, and the resolver couldn't iterate .variants. Updates the fixture to mock the actual API method.
Fixes #2 — reported by @redkjuegos.
Problem
Every
POST /api/store/resolvereturned 500:AppRegistryexposes.get(app_id)but two callsites were calling the non-existent.get_app(...). This made every Store interaction that hits the resolve endpoint blow up — master-blocking.Fix
routes/store.py:204—registry.get_app(manifest_id)→registry.get(manifest_id)routes/store_install.py:158-164— collapsed_registry_gethelper: removed the deadhasattr("get_app")branch (always-false), now callsregistry.get(app_id)directlyNo alias added to
AppRegistry— fixed at the callsites.Regression test
Added
test_resolve_returns_200_not_500andtest_resolve_unknown_manifest_returns_404totests/test_routes_store.py. These exercise the happy path and 404 path ofPOST /api/store/resolveend-to-end so this class of typo is caught going forward. All 30 store + registry tests pass.Summary by CodeRabbit
Tests
/api/store/resolveendpoint to verify correct handling of valid manifest IDs (returning HTTP 200) and invalid manifest IDs (returning HTTP 404).Refactor