From b1df380ae1e02b012b0a85107ff60c2131b568e9 Mon Sep 17 00:00:00 2001 From: didericis Date: Thu, 16 Jul 2026 22:05:21 -0400 Subject: [PATCH] feat(supervise): show the bottle's human slug, not its opaque id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidated proposals are keyed by the orchestrator-assigned bottle_id, so the supervise TUI was rendering a hex id (e.g. 3601cbe883c2786d) as the bottle name — and the resume hint printed `./cli.py resume `, which resume can't take (it wants the human identity/slug). `supervise_pending` now tags each dict with `bottle_label` — the human slug resolved from registry metadata, falling back to the id when the bottle is gone. `QueuedProposal` carries the label; every display site (list rows, detail view, status lines, resume hint) shows it, while approve/reject still key on `proposal.bottle_slug` (the id). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ --- bot_bottle/cli/supervise.py | 25 +++++++++++++++------- bot_bottle/orchestrator/service.py | 28 +++++++++++++++++++++++-- tests/unit/test_orchestrator_service.py | 15 +++++++++++++ tests/unit/test_supervise_cli.py | 17 +++++++++++++++ 4 files changed, 75 insertions(+), 10 deletions(-) diff --git a/bot_bottle/cli/supervise.py b/bot_bottle/cli/supervise.py index f8d8055..d87726c 100644 --- a/bot_bottle/cli/supervise.py +++ b/bot_bottle/cli/supervise.py @@ -47,9 +47,15 @@ _REPORT_ONLY_TOOLS: tuple[str, ...] = (TOOL_GITLEAKS_ALLOW, TOOL_EGRESS_TOKEN_AL @dataclass(frozen=True) class QueuedProposal: - """A pending proposal from the supervise queue.""" + """A pending proposal from the supervise queue. + + `label` is the operator-facing bottle name (the human slug the + orchestrator resolved from the registry); `proposal.bottle_slug` is the + opaque bottle_id every operator action is keyed by. Display uses `label`; + respond calls use `proposal.bottle_slug`.""" proposal: Proposal + label: str = "" # A failed operator action (orchestrator unreachable, bottle torn down, @@ -91,7 +97,10 @@ def _client() -> OrchestratorClient: def discover_pending() -> list[QueuedProposal]: """Collect pending proposals across bottles from the orchestrator.""" out = [ - QueuedProposal(proposal=Proposal.from_dict(d)) + QueuedProposal( + proposal=Proposal.from_dict(d), + label=str(d.get("bottle_label") or d.get("bottle_slug") or ""), + ) for d in _client().supervise_pending() ] out.sort(key=lambda q: q.proposal.arrival_timestamp) @@ -100,8 +109,8 @@ def discover_pending() -> list[QueuedProposal]: def _approval_status(qp: QueuedProposal, verb: str) -> str: """Status-line text after a successful approval.""" - base = f"{verb} {qp.proposal.tool} for [{qp.proposal.bottle_slug}]" - return f"{base}; resume: ./cli.py resume {qp.proposal.bottle_slug}" + base = f"{verb} {qp.proposal.tool} for [{qp.label}]" + return f"{base}; resume: ./cli.py resume {qp.label}" def _detail_lines( @@ -112,7 +121,7 @@ def _detail_lines( """Return the detail-view body as (text, curses-attr) tuples.""" p = qp.proposal out: list[tuple[str, int]] = [ - (f"bottle: {p.bottle_slug}", 0), + (f"bottle: {qp.label}", 0), (f"tool: {p.tool}", 0), (f"id: {p.id}", 0), (f"arrived: {p.arrival_timestamp}", 0), @@ -287,7 +296,7 @@ def _list_once() -> int: for qp in pending: sys.stdout.write( f"{qp.proposal.arrival_timestamp} " - f"[{qp.proposal.bottle_slug}] " + f"[{qp.label}] " f"{qp.proposal.tool} " f"{qp.proposal.id}\n" ) @@ -384,7 +393,7 @@ def _main_loop(stdscr: "curses._CursesWindow") -> None: # type: ignore # pragm reason = _prompt(stdscr, "reject reason: ") if reason: reject(qp, reason=reason) - status_line = f"rejected {qp.proposal.tool} for [{qp.proposal.bottle_slug}]" + status_line = f"rejected {qp.proposal.tool} for [{qp.label}]" else: status_line = "reject aborted (empty reason)" @@ -423,7 +432,7 @@ def _render( cursor = "> " if i == selected else " " line = ( f"{cursor}{ts_short} " - f"[{p.bottle_slug}] {p.tool:<18} {p.id[:8]}" + f"[{qp.label}] {p.tool:<18} {p.id[:8]}" ) attr = curses.A_REVERSE if i == selected else curses.A_NORMAL stdscr.addnstr(row, 0, line, w - 1, attr) diff --git a/bot_bottle/orchestrator/service.py b/bot_bottle/orchestrator/service.py index 924fa7b..7701b1c 100644 --- a/bot_bottle/orchestrator/service.py +++ b/bot_bottle/orchestrator/service.py @@ -153,8 +153,32 @@ class Orchestrator: def supervise_pending(self) -> list[dict[str, object]]: """All pending proposals across bottles, FIFO, as JSON dicts - (`Proposal.to_dict`, round-trippable via `Proposal.from_dict`).""" - return [p.to_dict() for p in list_all_pending_proposals()] + (`Proposal.to_dict`, round-trippable via `Proposal.from_dict`). + + Each dict carries an extra `bottle_label`: the bottle's human slug + resolved from the registry (the proposal itself is keyed by the + orchestrator-assigned bottle_id, which is opaque to an operator). The + CLI renders the label but still responds against `bottle_slug`.""" + out: list[dict[str, object]] = [] + for p in list_all_pending_proposals(): + d = p.to_dict() + d["bottle_label"] = self._label_for(p.bottle_slug) + out.append(d) + return out + + def _label_for(self, bottle_slug: str) -> str: + """The human slug recorded in registry metadata for a proposal's + bottle, or the bottle_slug unchanged when the bottle is gone or has no + recorded slug — so the label is always non-empty.""" + rec = self.registry.get(bottle_slug) + if rec is None: + return bottle_slug + try: + meta = json.loads(rec.metadata) if rec.metadata else {} + except ValueError: + meta = {} + slug = meta.get("slug") if isinstance(meta, dict) else None + return slug if isinstance(slug, str) and slug else bottle_slug def _record_for_slug(self, slug: str) -> BottleRecord | None: """The live registry record for a proposal's bottle, or None (e.g. the diff --git a/tests/unit/test_orchestrator_service.py b/tests/unit/test_orchestrator_service.py index f724d66..d99008b 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_pending_carries_human_label(self) -> None: + # The proposal is keyed by bottle_id, but pending dicts also expose the + # bottle's human slug so the operator sees a name, not a hex id. + bottle_id = self._register("codex-dev-a1b2c", "routes: []\n") + self._queue(bottle_id, "routes:\n - host: google.com\n") + pending = self.orch.supervise_pending() + self.assertEqual(bottle_id, pending[0]["bottle_slug"]) + self.assertEqual("codex-dev-a1b2c", pending[0]["bottle_label"]) + + def test_pending_label_falls_back_to_slug_when_bottle_gone(self) -> None: + # No registry record (torn down): label is the id, never empty. + self._queue("ghost-id", "routes:\n - host: google.com\n") + pending = self.orch.supervise_pending() + self.assertEqual("ghost-id", pending[0]["bottle_label"]) + 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 diff --git a/tests/unit/test_supervise_cli.py b/tests/unit/test_supervise_cli.py index 2e2bd65..b785a43 100644 --- a/tests/unit/test_supervise_cli.py +++ b/tests/unit/test_supervise_cli.py @@ -78,6 +78,23 @@ class TestDiscoverPending(_ClientMixin, unittest.TestCase): pending = supervise_cli.discover_pending() self.assertEqual([early.id, late.id], [qp.proposal.id for qp in pending]) + def test_label_comes_from_bottle_label(self) -> None: + # The server tags each dict with the human slug; the CLI displays it + # while the proposal stays keyed by the opaque bottle_id. + client = MagicMock() + d = _proposal("3601cbe883c2786d").to_dict() + d["bottle_label"] = "codex-dev-a1b2c" + client.supervise_pending.return_value = [d] + with patch.object(supervise_cli, "_client", return_value=client): + pending = supervise_cli.discover_pending() + self.assertEqual("codex-dev-a1b2c", pending[0].label) + self.assertEqual("3601cbe883c2786d", pending[0].proposal.bottle_slug) + + def test_label_falls_back_to_slug_when_absent(self) -> None: + # Legacy dicts without bottle_label (e.g. an older orchestrator). + self._install_client([_proposal("dev")]) + self.assertEqual("dev", supervise_cli.discover_pending()[0].label) + class TestApproveReject(_ClientMixin, unittest.TestCase): def _qp(self, tool: str = TOOL_EGRESS_ALLOW) -> "supervise_cli.QueuedProposal":