feat(supervise): show the bottle's human slug, not its opaque id
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 <id>`, 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UoEZHDjv84ChoZbozQERhJ
This commit is contained in:
@@ -47,9 +47,15 @@ _REPORT_ONLY_TOOLS: tuple[str, ...] = (TOOL_GITLEAKS_ALLOW, TOOL_EGRESS_TOKEN_AL
|
|||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class QueuedProposal:
|
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
|
proposal: Proposal
|
||||||
|
label: str = ""
|
||||||
|
|
||||||
|
|
||||||
# A failed operator action (orchestrator unreachable, bottle torn down,
|
# A failed operator action (orchestrator unreachable, bottle torn down,
|
||||||
@@ -91,7 +97,10 @@ def _client() -> OrchestratorClient:
|
|||||||
def discover_pending() -> list[QueuedProposal]:
|
def discover_pending() -> list[QueuedProposal]:
|
||||||
"""Collect pending proposals across bottles from the orchestrator."""
|
"""Collect pending proposals across bottles from the orchestrator."""
|
||||||
out = [
|
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()
|
for d in _client().supervise_pending()
|
||||||
]
|
]
|
||||||
out.sort(key=lambda q: q.proposal.arrival_timestamp)
|
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:
|
def _approval_status(qp: QueuedProposal, verb: str) -> str:
|
||||||
"""Status-line text after a successful approval."""
|
"""Status-line text after a successful approval."""
|
||||||
base = f"{verb} {qp.proposal.tool} for [{qp.proposal.bottle_slug}]"
|
base = f"{verb} {qp.proposal.tool} for [{qp.label}]"
|
||||||
return f"{base}; resume: ./cli.py resume {qp.proposal.bottle_slug}"
|
return f"{base}; resume: ./cli.py resume {qp.label}"
|
||||||
|
|
||||||
|
|
||||||
def _detail_lines(
|
def _detail_lines(
|
||||||
@@ -112,7 +121,7 @@ def _detail_lines(
|
|||||||
"""Return the detail-view body as (text, curses-attr) tuples."""
|
"""Return the detail-view body as (text, curses-attr) tuples."""
|
||||||
p = qp.proposal
|
p = qp.proposal
|
||||||
out: list[tuple[str, int]] = [
|
out: list[tuple[str, int]] = [
|
||||||
(f"bottle: {p.bottle_slug}", 0),
|
(f"bottle: {qp.label}", 0),
|
||||||
(f"tool: {p.tool}", 0),
|
(f"tool: {p.tool}", 0),
|
||||||
(f"id: {p.id}", 0),
|
(f"id: {p.id}", 0),
|
||||||
(f"arrived: {p.arrival_timestamp}", 0),
|
(f"arrived: {p.arrival_timestamp}", 0),
|
||||||
@@ -287,7 +296,7 @@ def _list_once() -> int:
|
|||||||
for qp in pending:
|
for qp in pending:
|
||||||
sys.stdout.write(
|
sys.stdout.write(
|
||||||
f"{qp.proposal.arrival_timestamp} "
|
f"{qp.proposal.arrival_timestamp} "
|
||||||
f"[{qp.proposal.bottle_slug}] "
|
f"[{qp.label}] "
|
||||||
f"{qp.proposal.tool} "
|
f"{qp.proposal.tool} "
|
||||||
f"{qp.proposal.id}\n"
|
f"{qp.proposal.id}\n"
|
||||||
)
|
)
|
||||||
@@ -384,7 +393,7 @@ def _main_loop(stdscr: "curses._CursesWindow") -> None: # type: ignore # pragm
|
|||||||
reason = _prompt(stdscr, "reject reason: ")
|
reason = _prompt(stdscr, "reject reason: ")
|
||||||
if reason:
|
if reason:
|
||||||
reject(qp, reason=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:
|
else:
|
||||||
status_line = "reject aborted (empty reason)"
|
status_line = "reject aborted (empty reason)"
|
||||||
|
|
||||||
@@ -423,7 +432,7 @@ def _render(
|
|||||||
cursor = "> " if i == selected else " "
|
cursor = "> " if i == selected else " "
|
||||||
line = (
|
line = (
|
||||||
f"{cursor}{ts_short} "
|
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
|
attr = curses.A_REVERSE if i == selected else curses.A_NORMAL
|
||||||
stdscr.addnstr(row, 0, line, w - 1, attr)
|
stdscr.addnstr(row, 0, line, w - 1, attr)
|
||||||
|
|||||||
@@ -153,8 +153,32 @@ class Orchestrator:
|
|||||||
|
|
||||||
def supervise_pending(self) -> list[dict[str, object]]:
|
def supervise_pending(self) -> list[dict[str, object]]:
|
||||||
"""All pending proposals across bottles, FIFO, as JSON dicts
|
"""All pending proposals across bottles, FIFO, as JSON dicts
|
||||||
(`Proposal.to_dict`, round-trippable via `Proposal.from_dict`)."""
|
(`Proposal.to_dict`, round-trippable via `Proposal.from_dict`).
|
||||||
return [p.to_dict() for p in list_all_pending_proposals()]
|
|
||||||
|
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:
|
def _record_for_slug(self, slug: str) -> BottleRecord | None:
|
||||||
"""The live registry record for a proposal's bottle, or None (e.g. the
|
"""The live registry record for a proposal's bottle, or None (e.g. the
|
||||||
|
|||||||
@@ -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_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:
|
def test_approve_by_bottle_id_applies_policy(self) -> None:
|
||||||
# Consolidated reality: the supervise server keys each proposal by the
|
# Consolidated reality: the supervise server keys each proposal by the
|
||||||
# orchestrator-assigned bottle_id, not the human slug. Approval must
|
# orchestrator-assigned bottle_id, not the human slug. Approval must
|
||||||
|
|||||||
@@ -78,6 +78,23 @@ class TestDiscoverPending(_ClientMixin, unittest.TestCase):
|
|||||||
pending = supervise_cli.discover_pending()
|
pending = supervise_cli.discover_pending()
|
||||||
self.assertEqual([early.id, late.id], [qp.proposal.id for qp in 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):
|
class TestApproveReject(_ClientMixin, unittest.TestCase):
|
||||||
def _qp(self, tool: str = TOOL_EGRESS_ALLOW) -> "supervise_cli.QueuedProposal":
|
def _qp(self, tool: str = TOOL_EGRESS_ALLOW) -> "supervise_cli.QueuedProposal":
|
||||||
|
|||||||
Reference in New Issue
Block a user