From 5f59df9e10db2331c7d3b5fd0296df9571d87c6f Mon Sep 17 00:00:00 2001 From: didericis Date: Thu, 16 Jul 2026 22:01:14 -0400 Subject: [PATCH] fix(supervise): resolve approval target by bottle_id, not human slug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In consolidated mode the supervise server attributes each proposal to the orchestrator-assigned bottle_id and stores that as the proposal's `bottle_slug` (supervise_server `_attributed_config`). But `_record_for_slug` only scanned registry metadata for a matching human slug, so approving an egress proposal 409'd with "bottle is no longer registered; cannot apply the route change" — the id never matched a human slug. Resolve the record by bottle_id first (the consolidated reality), keeping the metadata-slug scan as a fallback for legacy single-tenant proposals. The prior tests keyed proposals by the human slug matching the metadata, exercising only the fallback path — added a test that mirrors production (proposal keyed by bottle_id, distinct human slug in metadata). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ --- bot_bottle/orchestrator/service.py | 13 +++++++++++-- tests/unit/test_orchestrator_service.py | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/bot_bottle/orchestrator/service.py b/bot_bottle/orchestrator/service.py index 5c34815..924fa7b 100644 --- a/bot_bottle/orchestrator/service.py +++ b/bot_bottle/orchestrator/service.py @@ -157,8 +157,17 @@ class Orchestrator: return [p.to_dict() for p in list_all_pending_proposals()] def _record_for_slug(self, slug: str) -> BottleRecord | None: - """The live registry record whose metadata carries `slug`, or None - (e.g. the bottle was torn down before the operator responded).""" + """The live registry record for a proposal's bottle, or None (e.g. the + bottle was torn down before the operator responded). + + In consolidated mode the supervise server attributes each proposal to + the orchestrator-assigned bottle_id and stores that as the proposal's + `bottle_slug` (see supervise_server `_attributed_config`), so the fast + path is a direct bottle_id lookup. The metadata-slug scan is the + fallback for legacy single-tenant proposals keyed by the human slug.""" + rec = self.registry.get(slug) + if rec is not None: + return rec for rec in self.registry.all(): try: meta = json.loads(rec.metadata) if rec.metadata else {} diff --git a/tests/unit/test_orchestrator_service.py b/tests/unit/test_orchestrator_service.py index 8957ba0..f724d66 100644 --- a/tests/unit/test_orchestrator_service.py +++ b/tests/unit/test_orchestrator_service.py @@ -226,6 +226,21 @@ class TestOrchestratorSupervise(unittest.TestCase): self.assertEqual(STATUS_APPROVED, read_response("demo", pid).status) self.assertEqual([], self.orch.supervise_pending()) + def test_approve_by_bottle_id_applies_policy(self) -> None: + # Consolidated reality: the supervise server keys each proposal by the + # orchestrator-assigned bottle_id, not the human slug. Approval must + # resolve the record by that id and apply the policy (regression for + # the "bottle is no longer registered" 409). + bottle_id = self._register("codex-dev-a1b2c", "routes: []\n") + new_routes = "routes:\n - host: google.com\n" + pid = self._queue(bottle_id, new_routes) + ok, err = self.orch.supervise_respond( + pid, bottle_slug=bottle_id, decision="approve") + self.assertTrue(ok, err) + rec = self.store.get(bottle_id) + assert rec is not None + self.assertEqual(new_routes, rec.policy) + def test_modify_applies_final_file_not_proposed(self) -> None: bottle_id = self._register("demo", "routes: []\n") pid = self._queue("demo", "routes:\n - host: google.com\n")