feat(major2): the hosted surface refuses the proposal review by name and points at the CLI [skip-docs]
Ordre 20260904T173146Z-8102814273-from-portfolio-optimiser, steg 7 av 10. En PRE-whitelist-sjekk paa raa payload, ETTER isinstance-vakten (en ikke-objekt-body skal beholde sin 400, ikke bli en 500) og FOER den generiske unknown-field-sjekken, som ellers ville svart foerst. Plasseringen er MAALT, ikke valgt: _CONSUMED_FIELDS maa vaere disjunkt fra run_projects parametre (Fase 4es negative halvdel) mens proposal_reviewer ER en av dem, saa navnet kan ikke bo i noen av de tre listene. F4-presedensen er IKKE analog - enable_plan_review er en NOESTET noekkel inne i det whitelistede explore_contract, som er derfor den kan navngis der. DISKRIMINATOREN ER TEKSTEN, IKKE STATUSEN: whitelisten svarer alt enhver ukjent nokkel med 400 "unknown field(s)", saa en detachet navngitt nekt ville fortsatt gitt 400 med feltnavnet. Den navngitte meldingen peker paa CLI-doera og paa /readiness, og kontrollen (et ordinaert ukjent felt) asserterer at den generiske meldingen deler ingenting av det. Null run_project-kall, ikke bare en 400 (oekt 57). _response_payload er IKKE utvidet: flaten nekter revieweren, saa feltet kunne kun vaert tomt. RODT foer impl: tre armer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
fedf9897c4
commit
bfc634d806
2 changed files with 80 additions and 2 deletions
|
|
@ -98,6 +98,15 @@ _OPTIONAL_FIELDS = (
|
||||||
#: a real ``run_project`` parameter, and every consumed field must not be.
|
#: a real ``run_project`` parameter, and every consumed field must not be.
|
||||||
_CONSUMED_FIELDS = ("explore_prompt", "explore_contract")
|
_CONSUMED_FIELDS = ("explore_prompt", "explore_contract")
|
||||||
_ALLOWED_FIELDS = frozenset(_REQUIRED_FIELDS + _OPTIONAL_FIELDS + _CONSUMED_FIELDS)
|
_ALLOWED_FIELDS = frozenset(_REQUIRED_FIELDS + _OPTIONAL_FIELDS + _CONSUMED_FIELDS)
|
||||||
|
#: Names refused BY NAME on the raw payload, before the generic unknown-field check (MAJOR-2).
|
||||||
|
#:
|
||||||
|
#: A pre-whitelist check rather than a fourth list entry, and the placement is MEASURED.
|
||||||
|
#: ``_CONSUMED_FIELDS`` must be disjoint from ``run_project``'s parameters (Fase 4e's negative
|
||||||
|
#: half) while ``proposal_reviewer`` IS one; and the generic ``unknown field(s)`` refusal below
|
||||||
|
#: fires FIRST, so a name left to it would return the same 400 with a message that says nothing
|
||||||
|
#: about where the door actually is. The F4 precedent is NOT analogous: ``enable_plan_review`` is
|
||||||
|
#: a NESTED key inside the whitelisted ``explore_contract``, which is why it can be named there.
|
||||||
|
_REFUSED_BY_NAME = ("proposal_review",)
|
||||||
_REASONS = {
|
_REASONS = {
|
||||||
200: "OK",
|
200: "OK",
|
||||||
400: "Bad Request",
|
400: "Bad Request",
|
||||||
|
|
@ -130,6 +139,15 @@ def _run_kwargs(payload: Any) -> tuple[str, dict[str, Any], dict[str, Any]]:
|
||||||
which the container answers as a 500 for what is really a wiring mistake."""
|
which the container answers as a 500 for what is really a wiring mistake."""
|
||||||
if not isinstance(payload, dict):
|
if not isinstance(payload, dict):
|
||||||
raise InvocationRefused("body must be a JSON object")
|
raise InvocationRefused("body must be a JSON object")
|
||||||
|
# AFTER the object guard (a non-object body must keep its 400 rather than become a 500) and
|
||||||
|
# BEFORE the generic unknown-field check, which would otherwise answer this one first.
|
||||||
|
if "proposal_review" in payload:
|
||||||
|
raise InvocationRefused(
|
||||||
|
"proposal_review: this surface has no terminal to answer a proposal review — the "
|
||||||
|
"synchronous door would block the request on nobody, and would block the event loop "
|
||||||
|
"that answers /readiness while doing it. The operator door is the CLI's "
|
||||||
|
"--proposal-review"
|
||||||
|
)
|
||||||
unknown = sorted(set(payload) - _ALLOWED_FIELDS)
|
unknown = sorted(set(payload) - _ALLOWED_FIELDS)
|
||||||
if unknown:
|
if unknown:
|
||||||
raise InvocationRefused(f"unknown field(s): {', '.join(unknown)}")
|
raise InvocationRefused(f"unknown field(s): {', '.join(unknown)}")
|
||||||
|
|
|
||||||
|
|
@ -24,6 +24,7 @@ to today, golden transcript included.
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import inspect
|
||||||
import io
|
import io
|
||||||
import json
|
import json
|
||||||
import subprocess
|
import subprocess
|
||||||
|
|
@ -35,7 +36,7 @@ from typing import Any
|
||||||
import pytest
|
import pytest
|
||||||
from spikes._harness import FakeChatClient, message_texts
|
from spikes._harness import FakeChatClient, message_texts
|
||||||
|
|
||||||
from portfolio_optimiser import hitl
|
from portfolio_optimiser import hitl, hosting
|
||||||
from portfolio_optimiser import proposal_review as pr
|
from portfolio_optimiser import proposal_review as pr
|
||||||
from portfolio_optimiser.budget import Budget, BudgetExceeded, TokenMeter
|
from portfolio_optimiser.budget import Budget, BudgetExceeded, TokenMeter
|
||||||
from portfolio_optimiser.generate import ParseFailure, _build_messages, generate_via_llm
|
from portfolio_optimiser.generate import ParseFailure, _build_messages, generate_via_llm
|
||||||
|
|
@ -49,7 +50,8 @@ from portfolio_optimiser.validator import Rejection, ValidatedProposal, proposal
|
||||||
from portfolio_optimiser.verdicts import VerdictStore
|
from portfolio_optimiser.verdicts import VerdictStore
|
||||||
|
|
||||||
_REPO = Path(__file__).resolve().parents[1]
|
_REPO = Path(__file__).resolve().parents[1]
|
||||||
_BUNDLE_DIR = _REPO / "shared" / "examples" / "bygg-energi-mikro"
|
_EXAMPLES = _REPO / "shared" / "examples"
|
||||||
|
_BUNDLE_DIR = _EXAMPLES / "bygg-energi-mikro"
|
||||||
_PID = "BYGG-KONTOR-NORD"
|
_PID = "BYGG-KONTOR-NORD"
|
||||||
_RUN_ID = "proposal-review-door"
|
_RUN_ID = "proposal-review-door"
|
||||||
|
|
||||||
|
|
@ -1395,3 +1397,61 @@ def test_a_run_with_no_reviewer_prints_no_review_line(
|
||||||
argv = [a for a in _cli_argv(tmp_path, outbox="outbox") if a != "--proposal-review"]
|
argv = [a for a in _cli_argv(tmp_path, outbox="outbox") if a != "--proposal-review"]
|
||||||
assert run.main(argv) == 0
|
assert run.main(argv) == 0
|
||||||
assert "proposal review" not in capsys.readouterr().out
|
assert "proposal review" not in capsys.readouterr().out
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------------------------
|
||||||
|
# Group D (Steps 7 and 8) — the hosted refusal, and the library dispatcher.
|
||||||
|
# ---------------------------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
class _RunProjectRecorder:
|
||||||
|
def __init__(self) -> None:
|
||||||
|
self.calls: list[dict[str, Any]] = []
|
||||||
|
|
||||||
|
async def __call__(self, *args: Any, **kwargs: Any) -> Any:
|
||||||
|
self.calls.append(kwargs)
|
||||||
|
raise AssertionError("run_project must not be reached on a refused invocation")
|
||||||
|
|
||||||
|
|
||||||
|
async def test_t21_the_hosted_surface_refuses_the_door_by_name(
|
||||||
|
monkeypatch: pytest.MonkeyPatch,
|
||||||
|
) -> None:
|
||||||
|
"""T21 — the hosted refusal. Detach points: deleting the named refusal (M22), forwarding the
|
||||||
|
field instead (M23).
|
||||||
|
|
||||||
|
**The discriminator is the TEXT, not the status.** The whitelist already answers any unknown
|
||||||
|
field with ``400 unknown field(s): …``, so a detached named refusal would STILL return 400
|
||||||
|
naming the field — the arm would pass while the message said nothing about where the door
|
||||||
|
actually is. So the named message points at the CLI and at ``/readiness``, and the control
|
||||||
|
(an ordinary unknown field) asserts the generic message shares none of it.
|
||||||
|
|
||||||
|
Zero ``run_project`` calls, not merely a 400: at the status code a refusal after the spend
|
||||||
|
looks identical to one before it (økt 57's rule)."""
|
||||||
|
recorder = _RunProjectRecorder()
|
||||||
|
monkeypatch.setattr(hosting, "run_project", recorder)
|
||||||
|
payload = {"project_id": _RUN_PID, "docs_dir": str(_BUNDLE_DIR)}
|
||||||
|
|
||||||
|
with pytest.raises(hosting.InvocationRefused) as excinfo:
|
||||||
|
await hosting.invoke({**payload, "proposal_review": True})
|
||||||
|
message = str(excinfo.value)
|
||||||
|
assert "--proposal-review" in message
|
||||||
|
assert "/readiness" in message
|
||||||
|
assert recorder.calls == []
|
||||||
|
|
||||||
|
with pytest.raises(hosting.InvocationRefused) as generic:
|
||||||
|
await hosting.invoke({**payload, "outbox_dir": "/x"})
|
||||||
|
assert "--proposal-review" not in str(generic.value)
|
||||||
|
assert "outbox_dir" in str(generic.value)
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_refused_name_enters_neither_half_of_the_whitelist() -> None:
|
||||||
|
"""The partition arm. Fase 4e's two asserts (every forwarded field IS a ``run_project``
|
||||||
|
parameter, every consumed field is NOT) must stay untouched, which is why this is a
|
||||||
|
PRE-whitelist check on the raw payload rather than a fourth list entry.
|
||||||
|
|
||||||
|
The F4 precedent is measured as NOT analogous: ``enable_plan_review`` is a NESTED key inside
|
||||||
|
the whitelisted ``explore_contract``, which is why it can be named at all."""
|
||||||
|
assert set(hosting._REFUSED_BY_NAME).isdisjoint(hosting._ALLOWED_FIELDS)
|
||||||
|
assert set(hosting._REFUSED_BY_NAME).isdisjoint(inspect.signature(run_project).parameters)
|
||||||
|
# ...and the parameter it corresponds to is a real one, so the refusal names a door that exists.
|
||||||
|
assert "proposal_reviewer" in inspect.signature(run_project).parameters
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue