The page names what is blocked, and can now unblock it

hullwork/cli.py

@@ -34,6 +34,7 @@
     db,
     doctor,
     lease,
+    operator,
     outcomes,
     page,
     propose,
@@ -44,6 +45,7 @@
     triage,
     work,
 )
+from hullwork import decisions as decide
 from hullwork import upstream as upstream_module
 from hullwork.config import ConfigError, Settings, get_settings
 from hullwork.credentials import PushCapability
@@ -897,37 +899,25 @@ def _cmd_rotate(
 def approve(session: Session, slug: str, item_id: int) -> Item:
     """Let an agent attempt one amber item. One item, named explicitly, by a human.
 
-    A command rather than an endpoint, for the reason registration is one: the operator already has
-    the host, and an approval endpoint would be a permanent attack surface for something done by one
-    person a handful of times. There is deliberately no `--all`.
+    **The decision itself moved to `decisions.py` in item 166**, because the page grew a button and
+    a route would otherwise import a command from here — the shape item 162 removed from `sandbox/`.
+    What stays is this signature, which takes a slug because a person types a slug, and the mapping
+    from the decision's refusal to this module's.
+
+    The comment that used to live here argued that approval should be a command and never an
+    endpoint: *"the operator already has the host, and an approval endpoint would be a permanent
+    attack surface for something done by one person a handful of times."* Item 166 reversed it, with
+    the ground it stood on: the operator no longer *has* the host in any useful sense — reading the
+    page happens on a laptop and acting meant SSH, `docker compose exec` and this command, which
+    measured out at two items waiting twenty-one hours on an instance somebody was looking at. The
+    attack surface is still real, which is why the endpoint needs a second credential that never
+    appears in a URL.
     """
-    project = _require(session, slug)
-    item = (
-        session.query(Item)
-        .filter(Item.id == item_id, Item.project_id == project.id)
-        .one_or_none()
-    )
-    if item is None:
-        raise CommandError(f"'{slug}' has no item {item_id}")
-
-    if item.state is not ItemState.WAITING_APPROVAL:
-        # Naming the state it found is the difference between a refusal and a puzzle. An item that
-        # is already `ready`, or that a human closed, is the common case here.
-        raise CommandError(
-            f"item {item_id} is '{item.state.value}', not '{ItemState.WAITING_APPROVAL.value}' — "
-            f"only an item waiting for approval can be approved"
-        )
-
     try:
-        transition(item, ItemState.READY)
-    except IllegalTransitionError as exc:
-        # Red reaches here only if a manifest was edited underneath a queued item. The state machine
-        # refuses it whatever this command thinks, which is the point of enforcing it there.
+        return decide.approve(session, _require(session, slug), item_id)
+    except decide.DecisionError as exc:
         raise CommandError(str(exc)) from exc
 
-    session.commit()
-    return item
-
 
 def requeue(session: Session, slug: str, item_id: int) -> Item:
     """Put a `human-only` item back in the queue when what stopped it was the environment. Item 093.
@@ -1211,6 +1201,49 @@ def _cmd_page_token(
     return 0
 
 
+def _cmd_operator_key(
+    args: argparse.Namespace, session: Session, settings: Settings, out: TextIO
+) -> int:
+    """Mint the credential that **acts** on the read-only page. Item 166.
+
+    **A second credential rather than a promotion of the first**, and the difference is the whole
+    security model: the page token is a bearer string in a URL — a saved page, a screenshot of the
+    address bar, a link mailed to a colleague — so it reads everything and may never spend money.
+    This one is pasted into a form once and exchanged for a session cookie, so it never lands
+    anywhere a URL lands.
+
+    Refuses to replace an existing key without `--rotate`, for the reason `page-token` does: a
+    second person running this to "get in" would lock out the first, and the failure would read as
+    the buttons being broken rather than as a key having changed underneath them.
+    """
+    existing = operator.configured(session)
+    if existing and not args.rotate:
+        raise CommandError(
+            "this instance already has an operator key, and it cannot be shown again — it was "
+            "printed once and only its hash is stored.\n"
+            "  To replace it: hullwork operator-key --rotate. Every session open right now ends "
+            "the moment you do."
+        )
+
+    key = operator.issue_key(session)
+    print("Rotated. Every session that was open has ended." if existing else
+          "The page can now be acted on.", file=out)
+    print("\n  This key is shown once and cannot be recovered:\n", file=out)
+    print(f"    {key}\n", file=out)
+    print(
+        "  Paste it into the page's login, once per browser. Unlike the page URL it is **not** a\n"
+        "  link and must never become one: it is the difference between somebody reading this\n"
+        "  instance and somebody spending its budget.\n"
+        "\n"
+        "  What a session may then do: approve one amber item, or hand one to a human. Nothing\n"
+        "  else on the page changes anything, and there is no approve-everything.\n"
+        "\n"
+        "  Sessions last 12 hours. To end them all at once, rotate.",
+        file=out,
+    )
+    return 0
+
+
 def _cmd_status(
     args: argparse.Namespace, session: Session, settings: Settings, out: TextIO
 ) -> int:
@@ -3036,6 +3069,26 @@ def build_parser() -> argparse.ArgumentParser:
     )
     page_token.set_defaults(func=_cmd_page_token)
 
+    operator_key = subparsers.add_parser(
+        "operator-key",
+        help="mint the credential that acts on the read-only page",
+        description=(
+            "The page reads with a token in its URL, which is why it may not act: a URL is a thing "
+            "that gets saved, screenshotted and forwarded. This mints a second credential that "
+            "never appears in a URL — pasted into a login once per browser, exchanged for a "
+            "session cookie — and it is what the two buttons on an amber item require.\n\n"
+            "Until this command runs there are no buttons, and every route that would change "
+            "something answers 404 the way an unknown path does.\n\n"
+            "Shown once, stored as a hash."
+        ),
+    )
+    operator_key.add_argument(
+        "--rotate",
+        action="store_true",
+        help="replace the existing key, ending every session that is open right now",
+    )
+    operator_key.set_defaults(func=_cmd_operator_key)
+
     pruning = subparsers.add_parser(
         "prune", help="forget the raw bodies of old deliveries, keeping every row"
     )

hullwork/decisions.py

@@ -0,0 +1,98 @@
+"""The two decisions a human makes about an amber item, and nothing else. Item 166.
+
+**Here because two callers need them and neither should own them.** `approve` lived in `cli.py`, and
+the route that item 166 added would have had to import it from there — a module that owns a command,
+imported by a module that owns a route. That is the exact shape item 162 spent an item removing from
+`sandbox/`, for the reason that survived it: a name two modules reach for does not belong to either.
+
+Both take the **project**, already looked up, rather than a slug. The lookup is the caller's
+business — `cli` refuses with an exit code, a route refuses with a status — and passing the slug
+would have dragged one of those vocabularies into the other.
+
+The pair is deliberately small and deliberately closed. `LEGAL[WAITING_APPROVAL]` is
+`{READY, HUMAN_ONLY, DONE}`, and the third is not here: an item a human closes by hand is closed in
+the forge, where the issue is, and the sweep reads it back. What a human decides *about an attempt*
+is only ever these two — let it try, or take it away from it.
+"""
+
+from __future__ import annotations
+
+from sqlalchemy.orm import Session
+
+from hullwork.models import Item, ItemState, Project
+from hullwork.states import IllegalTransitionError, transition
+
+
+class DecisionError(Exception):
+    """The decision cannot be made, and the message says which state was found instead.
+
+    Naming the state is the difference between a refusal and a puzzle: an item already `ready`, or
+    one a human closed last week, is the common case at this door rather than the exception.
+    """
+
+
+def _the_item(session: Session, project: Project, item_id: int) -> Item:
+    item = (
+        session.query(Item)
+        .filter(Item.id == item_id, Item.project_id == project.id)
+        .one_or_none()
+    )
+    if item is None:
+        msg = f"'{project.slug}' has no item {item_id}"
+        raise DecisionError(msg)
+    return item
+
+
+def _move(item: Item, target: ItemState, *, only_from: ItemState, verb: str) -> Item:
+    if item.state is not only_from:
+        msg = (
+            f"item {item.id} is '{item.state.value}', not '{only_from.value}' — "
+            f"only an item waiting for approval can be {verb}"
+        )
+        raise DecisionError(msg)
+    try:
+        transition(item, target)
+    except IllegalTransitionError as exc:
+        # Red reaches here only if a manifest was edited underneath a queued item. The state machine
+        # refuses it whatever this function thinks, which is the point of enforcing it there.
+        raise DecisionError(str(exc)) from exc
+    return item
+
+
+def approve(session: Session, project: Project, item_id: int) -> Item:
+    """Let an agent attempt one amber item. One item, named explicitly, by a human.
+
+    **There is deliberately no `--all` and no equivalent.** One approval is one attempt, which costs
+    money and opens a pull request somebody has to read; a button that approves a queue is a button
+    that spends a budget.
+    """
+    item = _move(
+        _the_item(session, project, item_id),
+        ItemState.READY,
+        only_from=ItemState.WAITING_APPROVAL,
+        verb="approved",
+    )
+    session.commit()
+    return item
+
+
+def hand_to_human(session: Session, project: Project, item_id: int) -> Item:
+    """Take an amber item away from the agent: a person will do this one.
+
+    **Not `rejected`, and the state machine is why.** `LEGAL[WAITING_APPROVAL]` does not contain
+    `REJECTED` — that state means *a reviewer closed a pull request*, and it feeds
+    `counted.rejected` keyed by the reason on that pull request's labels. Calling this "reject"
+    would file a decision about **whether to attempt** into the tally that counts **review**
+    decisions, and the number would drift with nobody able to see why.
+
+    `human-only` is the honest name and it already existed: it is what a lane says when the code
+    location is somewhere an agent is not allowed to go.
+    """
+    item = _move(
+        _the_item(session, project, item_id),
+        ItemState.HUMAN_ONLY,
+        only_from=ItemState.WAITING_APPROVAL,
+        verb="handed to a human",
+    )
+    session.commit()
+    return item