fix(supervise): resolve approval target by bottle_id, not human slug
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 <id> 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
This commit is contained in:
@@ -157,8 +157,17 @@ class Orchestrator:
|
|||||||
return [p.to_dict() for p in list_all_pending_proposals()]
|
return [p.to_dict() for p in list_all_pending_proposals()]
|
||||||
|
|
||||||
def _record_for_slug(self, slug: str) -> BottleRecord | None:
|
def _record_for_slug(self, slug: str) -> BottleRecord | None:
|
||||||
"""The live registry record whose metadata carries `slug`, or None
|
"""The live registry record for a proposal's bottle, or None (e.g. the
|
||||||
(e.g. the bottle was torn down before the operator responded)."""
|
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():
|
for rec in self.registry.all():
|
||||||
try:
|
try:
|
||||||
meta = json.loads(rec.metadata) if rec.metadata else {}
|
meta = json.loads(rec.metadata) if rec.metadata else {}
|
||||||
|
|||||||
@@ -226,6 +226,21 @@ class TestOrchestratorSupervise(unittest.TestCase):
|
|||||||
self.assertEqual(STATUS_APPROVED, read_response("demo", pid).status)
|
self.assertEqual(STATUS_APPROVED, read_response("demo", pid).status)
|
||||||
self.assertEqual([], self.orch.supervise_pending())
|
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 <id> 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:
|
def test_modify_applies_final_file_not_proposed(self) -> None:
|
||||||
bottle_id = self._register("demo", "routes: []\n")
|
bottle_id = self._register("demo", "routes: []\n")
|
||||||
pid = self._queue("demo", "routes:\n - host: google.com\n")
|
pid = self._queue("demo", "routes:\n - host: google.com\n")
|
||||||
|
|||||||
Reference in New Issue
Block a user