From f3c938910d1328f047c83db7e6b8a7734c8f0b45 Mon Sep 17 00:00:00 2001 From: Julian lechner Date: Thu, 24 Sep 2026 16:22:55 +0200 Subject: [PATCH] fix(retrieval): keep active runtime after benchmark --- TODO.md | 2 +- scripts/lib/semantic-retrieval.py | 2 +- scripts/tests/test-semantic-retrieval.py | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/TODO.md b/TODO.md index 320fbb3..3150ac0 100644 --- a/TODO.md +++ b/TODO.md @@ -3,7 +3,7 @@ ## Data loss and security - [ ] Validate every path read from `.ai-config-manifest.tsv` and reject links before backup, overwrite, or removal. Both manifest writers join unchecked recorded paths to the destination; a `../` entry or linked target can reach files outside the managed directory. (`scripts/lib/manifest.ps1:20`, `scripts/lib/manifest.ps1:136`, `scripts/lib/manifest.sh:20`, `scripts/lib/manifest.sh:170`) -- [ ] Keep an installed semantic retrieval runtime when an `auto` benchmark says no. `benchmark()` deletes the whole runtime and ownership state even when `state['clients']` still contains active clients. (`scripts/lib/semantic-retrieval.py:117`, `scripts/lib/semantic-retrieval.py:142`) +- [x] Keep an installed semantic retrieval runtime when an `auto` benchmark says no. `benchmark()` deletes the whole runtime and ownership state even when `state['clients']` still contains active clients. (`scripts/lib/semantic-retrieval.py:117`, `scripts/lib/semantic-retrieval.py:142`) - [ ] Check semantic retrieval ownership and links before copying or deleting its directory. An existing owned runtime's source files are overwritten without comparing recorded hashes; a replaced target symlink is followed on enable, and disabling the last client removes foreign files inside the runtime. (`scripts/lib/semantic-retrieval.py:82`, `scripts/lib/semantic-retrieval.py:100`) - [ ] Preserve pre-existing MCPorter server entries. `ensure_mcporter()` adds a server and claims ownership without checking existing configuration; if the add succeeds for an existing name, a later removal deletes that user's entry. (`scripts/lib/managed-extensions.py:395`, `scripts/lib/managed-extensions.py:263`) - [ ] Preserve pre-existing global CLI packages. `ensure_cli()` and `ensure_qmd()` install and claim npm packages without checking whether they were already installed; later removal can uninstall a user's package. (`scripts/lib/managed-extensions.py:342`, `scripts/lib/managed-extensions.py:378`) diff --git a/scripts/lib/semantic-retrieval.py b/scripts/lib/semantic-retrieval.py index 7d49211..76a35f2 100644 --- a/scripts/lib/semantic-retrieval.py +++ b/scripts/lib/semantic-retrieval.py @@ -149,7 +149,7 @@ def benchmark(root, home, dry_run, summary=False): outcome = json.loads(result.stdout) if not isinstance(outcome, dict) or not isinstance(outcome.get('recommended'), bool): raise ValueError('Semantic retrieval benchmark returned an invalid result.') - if outcome['recommended']: + if outcome['recommended'] or state['clients']: save_state(state_path, state, False) else: shutil.rmtree(target) diff --git a/scripts/tests/test-semantic-retrieval.py b/scripts/tests/test-semantic-retrieval.py index ce78718..cd3313c 100644 --- a/scripts/tests/test-semantic-retrieval.py +++ b/scripts/tests/test-semantic-retrieval.py @@ -85,6 +85,25 @@ class SemanticRetrievalTests(unittest.TestCase): self.assertFalse(target.exists()) self.assertFalse(state_path.exists()) + def test_negative_benchmark_keeps_active_runtime(self): + with tempfile.TemporaryDirectory() as directory: + home = Path(directory) / 'home' + target = home / '.my-ai-configuration/semantic-retrieval' + target.mkdir(parents=True) + state_path = target.parent / 'semantic-retrieval.json' + files = {} + for name in MODULE.FILES: + content = (ROOT / 'shared/retrieval' / name).read_bytes() + (target / name).write_bytes(content) + files[name] = hashlib.sha256(content).hexdigest() + MODULE.save_state(state_path, {'version': 1, 'clients': ['codex'], 'files': files}, False) + result = type('Result', (), {'stdout': '{"recommended": false, "embeddingSeconds": 1, "rerankingSeconds": 1}'})() + with patch.object(MODULE, 'ensure_runtime', return_value=Path('python')), \ + patch.object(MODULE.subprocess, 'run', return_value=result): + self.assertFalse(MODULE.benchmark(ROOT, home, False, True)) + self.assertTrue(target.is_dir()) + self.assertEqual(MODULE.load_state(state_path)['clients'], ['codex']) + if __name__ == '__main__': unittest.main()