diff --git a/CHANGELOG.md b/CHANGELOG.md index 038bcf5..139ad49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,21 @@ Notable changes to Cloudfall. The format follows ### Added +- A decision record has two new statuses, added to + `operation-decision.schema.json` beside the old ones. `ran`: a `risk: read` + operation has run by the time it is recorded, so it is recorded with its + run's exit code (`ranExitCode`) instead of waiting as `proposed` forever. + `superseded`: when `operations propose` records an operation, an older + `proposed` record of the same operation, targets and inputs is closed and + names the new decision (`supersededBy`); an approved or finished record is + never touched. Since it reads the record first, `operations propose` now + stops with `RECORD_INVALID` on a record that does not load, before check + mode runs. `decisions approve` refuses a `ran` or `superseded` record with + `PRECONDITION` before asking for the decision id. `decisions list --status S` and + `cloudfall why --status S` (repeatable) keep only those statuses, and an + unknown status is refused. Records already on disk keep their status. One + disk-full incident left 47 records for `cloudfall why --host hz1`, most of + them reads and repeats (#26) - Every turn of `agent investigate` keeps its raw trace, in the stream's `turn` event (`trace`) and in the investigation record (`spec.trace`): the model's reasoning when it returns one (Nemotron's `reasoning_content`), diff --git a/config/schemas/v1/operation-decision.schema.json b/config/schemas/v1/operation-decision.schema.json index a8de463..e7f5dd4 100644 --- a/config/schemas/v1/operation-decision.schema.json +++ b/config/schemas/v1/operation-decision.schema.json @@ -192,9 +192,21 @@ "rejected", "executed", "verified", - "failed" + "failed", + "ran", + "superseded" ] }, + "ranExitCode": { + "description": "The exit code of a read operation's run, which happens when it is proposed. Set when the status is ran.", + "type": "integer", + "minimum": 0, + "maximum": 255 + }, + "supersededBy": { + "description": "The decision that proposed the same operation, targets and inputs again. Set when the status is superseded.", + "$ref": "common.schema.json#/$defs/resourceId" + }, "approval": { "type": "object", "properties": { @@ -363,6 +375,36 @@ "check", "status" ], + "allOf": [ + { + "if": { + "properties": { + "status": { + "const": "ran" + } + } + }, + "then": { + "required": [ + "ranExitCode" + ] + } + }, + { + "if": { + "properties": { + "status": { + "const": "superseded" + } + } + }, + "then": { + "required": [ + "supersededBy" + ] + } + } + ], "additionalProperties": false } }, diff --git a/examples/disk-full-incident/README.md b/examples/disk-full-incident/README.md index f1fcf0f..8fa2a61 100644 --- a/examples/disk-full-incident/README.md +++ b/examples/disk-full-incident/README.md @@ -32,8 +32,11 @@ The catalog: | `converge` | mutating | runs `site.yml` | | `postgresql-reinit` | destructive | drops the cluster with all its data and creates an empty one | -Read operations run when called and return what the hosts reported. The -others run in check mode only and leave a decision a person approves. +Read operations run when called, return what the hosts reported, and are +recorded as `ran`. The others run in check mode only and leave a decision a +person approves; proposing the same one again marks the older decision +`superseded`. `cloudfall decisions list --status proposed` shows what still +waits. ## Run it diff --git a/sdk/src/cloudfall/app.py b/sdk/src/cloudfall/app.py index 9ae77a7..da2d3dc 100644 --- a/sdk/src/cloudfall/app.py +++ b/sdk/src/cloudfall/app.py @@ -100,6 +100,7 @@ CHECK_TIMEOUT_SECONDS, DECISION_DIRECTORY, ERROR_DECISION_MISSING, + ERROR_RECORD_INVALID, ApprovalChannel, ApprovalRequest, CheckRunner, @@ -111,6 +112,7 @@ Targets, approve, propose, + refuse_unapprovable, ) from cloudfall.domain import ( ConnectionAddress, @@ -759,6 +761,16 @@ class DecisionPayload(Payload): command = "decisions show" +@dataclass(frozen=True, slots=True, kw_only=True) +class ListDecisionsArgs(DecisionsArgs): + """Arguments of ``decisions list``.""" + + status: tuple[DecisionStatus, ...] = Flag( + default=(), + description="Only decisions with this status (repeatable)", + ) + + @dataclass(frozen=True, slots=True, kw_only=True) class ShowDecisionArgs(DecisionsArgs): """Arguments of ``decisions show``.""" @@ -803,13 +815,23 @@ def operations_show(args: ShowOperationArgs, _ctx: Ctx) -> OperationPayload: description="List the decision records this repository holds, oldest first", danger_level="safe", exit_codes=["RECORD_INVALID"], - examples=[("List the decision records", "cloudfall decisions list")], + examples=[ + ("List the decision records", "cloudfall decisions list"), + ( + "List what still waits for an approval", + "cloudfall decisions list --status proposed", + ), + ], ) -def decisions_list(args: DecisionsArgs, _ctx: Ctx) -> DecisionsPayload: +def decisions_list(args: ListDecisionsArgs, _ctx: Ctx) -> DecisionsPayload: """Answer every decision record: the record outlives the catalog.""" store = args.store() try: - documents = [decision.as_document() for decision in store.list()] + documents = [ + decision.as_document() + for decision in store.list() + if not args.status or decision.status in args.status + ] except DecisionError as error: raise _record_invalid(error) from error return DecisionsPayload( @@ -1989,7 +2011,13 @@ def _decision_failed(error: DecisionError) -> Exception: "on the fleet changes" ), danger_level="mutating", - exit_codes=["CONFIG_INVALID", "NOT_FOUND", "PRECONDITION", "CHECK_FAILED"], + exit_codes=[ + "CONFIG_INVALID", + "NOT_FOUND", + "PRECONDITION", + "CHECK_FAILED", + "RECORD_INVALID", + ], timeout=None, subprocess=Subprocess("ansible-playbook"), required_tools={"ansible-playbook": "2.21.0"}, @@ -2030,6 +2058,10 @@ def _propose(args: ProposeArgs, run: CheckRunner) -> Decided: try: decision = propose(request, store, run=run) except DecisionError as error: + # A new proposal reads the open ones it may supersede, so a broken + # record stops it the way it stops every other reader of the record. + if error.code == ERROR_RECORD_INVALID: + raise _record_invalid(error) from error raise _decision_failed(error) from error output = ( (args.root / decision.check.diff.path).read_text(encoding="utf-8") @@ -2087,6 +2119,10 @@ def decisions_approve(args: ApproveDecisionArgs, ctx: Ctx) -> Decided: if error.code == ERROR_DECISION_MISSING: raise Exit.NOT_FOUND(error.detail, context={"code": error.code}) from error raise _record_invalid(error) from error + try: + refuse_unapprovable(decision) + except DecisionError as error: + raise _decision_failed(error) from error # Off a terminal this ends with PERSON_REQUIRED, and a mistyped id with # ATTESTATION_MISMATCH, before anything runs: --yes never approves. attestation = ctx.attest( @@ -3049,6 +3085,10 @@ class WhyArgs(DecisionsArgs): "Only decisions with a moment at or before this ISO 8601 time or date" ), ) + status: tuple[DecisionStatus, ...] = Flag( + default=(), + description="Only decisions with this status (repeatable)", + ) def __post_init__(self) -> None: """Refuse a time the question cannot be asked with.""" @@ -3065,6 +3105,7 @@ def query(self) -> WhyQuery: operation=self.operation.value if self.operation is not None else None, since=self.since, until=self.until, + status=tuple(entry.value for entry in self.status), ) @@ -3085,6 +3126,10 @@ class WhyPayload(Payload): renderers={"html": FormatRenderer(render_why_document, media_type="text/html")}, examples=[ ("Ask about one host", "cloudfall why --host h1"), + ( + "Only the decisions that verified or failed", + "cloudfall why --status verified --status failed", + ), ("One page for a person", "cloudfall why --since 2026-10-01 --format html"), ], ) @@ -3839,7 +3884,13 @@ def _provided_tools(args: McpServeArgs, ctx: Ctx) -> list[McpTool]: return _engine_tools(args, root, ctx) -_PROPOSE_EXIT_CODES = ("CONFIG_INVALID", "NOT_FOUND", "PRECONDITION", "CHECK_FAILED") +_PROPOSE_EXIT_CODES = ( + "CONFIG_INVALID", + "NOT_FOUND", + "PRECONDITION", + "CHECK_FAILED", + "RECORD_INVALID", +) def _operation_tool_schema(operation: Operation) -> dict[str, object]: diff --git a/sdk/src/cloudfall/decision.py b/sdk/src/cloudfall/decision.py index 827ffdb..7cf28d4 100644 --- a/sdk/src/cloudfall/decision.py +++ b/sdk/src/cloudfall/decision.py @@ -71,7 +71,7 @@ _ERROR_NOT_PROPOSED = "decision_not_proposed" _ERROR_APPROVER_UNKNOWN = "decision_approver_unknown" _ERROR_CHANNEL_UNKNOWN = "decision_approval_channel_unknown" -_ERROR_RECORD_INVALID = "decision_record_invalid" +ERROR_RECORD_INVALID = "decision_record_invalid" class DecisionError(RuntimeError): @@ -113,6 +113,11 @@ class DecisionStatus(StrEnum): EXECUTED = "executed" VERIFIED = "verified" FAILED = "failed" + RAN = "ran" + """A read operation: it ran when it was proposed; nothing waits on it.""" + + SUPERSEDED = "superseded" + """A newer proposal of the same operation, targets and inputs replaced it.""" def gate(risk: RiskLevel) -> tuple[Requirement, str]: @@ -287,6 +292,16 @@ class Decision: execution: RunRecord | None = None verification: RunRecord | None = None verdict: str | None = None + ran_exit_code: int | None = None + superseded_by: ResourceId | None = None + + def same_call(self, other: Decision) -> bool: + """Return whether both name the same operation, targets and inputs.""" + return ( + self.operation_id == other.operation_id + and self.targets == other.targets + and _canonical(self.inputs) == _canonical(other.inputs) + ) def as_document(self) -> dict[str, object]: """Serialize the decision as a schema-valid record document.""" @@ -315,6 +330,10 @@ def as_document(self) -> dict[str, object]: spec["verify"] = self.verification.as_dict() if self.verdict is not None: spec["verdict"] = self.verdict + if self.ran_exit_code is not None: + spec["ranExitCode"] = self.ran_exit_code + if self.superseded_by is not None: + spec["supersededBy"] = self.superseded_by.value return { "apiVersion": API_VERSION, "kind": DECISION_KIND, @@ -362,7 +381,7 @@ def load(self, decision_id: ResourceId) -> Decision: ) except (UnicodeDecodeError, json.JSONDecodeError) as error: message = f"decision record is not valid JSON: {path}: {error}" - raise DecisionError(_ERROR_RECORD_INVALID, message) from error + raise DecisionError(ERROR_RECORD_INVALID, message) from error try: self.catalog.validate_named(DECISION_SCHEMA, document) except ValidationError as error: @@ -371,7 +390,7 @@ def load(self, decision_id: ResourceId) -> Decision: f"decision record does not match its schema: {path}: " f"{field}: {error.message}" ) - raise DecisionError(_ERROR_RECORD_INVALID, message) from error + raise DecisionError(ERROR_RECORD_INVALID, message) from error return _decision_from_document(document) def list(self) -> tuple[Decision, ...]: @@ -418,10 +437,19 @@ def propose( Nothing on the fleet changes: check mode is the whole point of the step. The record it leaves is what an approval is given against. + + A read operation has run by the time it is recorded, so it is recorded + as ``ran`` with its exit code rather than left waiting. An older open + proposal of the same operation, targets and inputs is marked + ``superseded`` by the new one: approving the older one would run what + the newer check already replaced. """ operation = request.operation declared = _validated_inputs(operation, request.inputs) pattern = _validated_targets(operation, request.targets) + # Read before anything runs: a record that does not load stops the + # proposal here rather than after check mode ran. + recorded = store.list() moment = (now or _utc_now)() decision_id = _decision_identifier(operation.operation_id, moment) diff_path = store.directory / f"{decision_id.value}{_DIFF_SUFFIX}" @@ -439,6 +467,7 @@ def propose( diff_path.parent.mkdir(parents=True, exist_ok=True) exit_code = (run or _run_check)(argv, environment, diff_path) requirement, reason = gate(operation.risk) + ran = requirement is Requirement.RUNS_FREELY decision = Decision( decision_id=decision_id, proposed_at=_timestamp(moment), @@ -454,9 +483,28 @@ def propose( reason=reason, basis=read_basis(request.observations, request.repository), check=_preview(exit_code, tree, diff_path, request.repository), - status=DecisionStatus.PROPOSED, + status=DecisionStatus.RAN if ran else DecisionStatus.PROPOSED, + verdict=( + f"a read operation runs when it is proposed; it exited {exit_code}" + if ran + else None + ), + ran_exit_code=exit_code if ran else None, ) store.save(decision) + for older in recorded: + if older.status is DecisionStatus.PROPOSED and older.same_call(decision): + store.update( + replace( + older, + status=DecisionStatus.SUPERSEDED, + superseded_by=decision_id, + verdict=( + f"{decision_id.value} proposed the same operation, " + "targets and inputs again" + ), + ) + ) return decision @@ -504,12 +552,7 @@ def approve( human read and the run that follows it. """ decision = request.decision - if decision.status is not DecisionStatus.PROPOSED: - message = ( - f"decision {decision.decision_id} is {decision.status.value}, so " - "there is nothing left to approve" - ) - raise DecisionError(_ERROR_NOT_PROPOSED, message) + refuse_unapprovable(decision) if not request.approver.strip(): message = "an approval records who gave it; pass --approver or set USER" raise DecisionError(_ERROR_APPROVER_UNKNOWN, message) @@ -545,6 +588,40 @@ def approve( return approved +def refuse_unapprovable(decision: Decision) -> None: + """Refuse a decision that is not an open proposal, before anything runs. + + Callers that ask a person for the decision id check this first, so + nobody types an id only to hear there was nothing to approve. + """ + if decision.status is DecisionStatus.PROPOSED: + return + if decision.status is DecisionStatus.SUPERSEDED: + # A read is recorded as ran, so the decision that replaced an old + # read record has nothing left to approve either. + instead = ( + "it ran when it was proposed, so there is nothing to approve" + if decision.requirement is Requirement.RUNS_FREELY + else "approve that one instead" + ) + message = ( + f"decision {decision.decision_id} is superseded by " + f"{decision.superseded_by}, a newer proposal of the same operation, " + f"targets and inputs; {instead}" + ) + elif decision.status is DecisionStatus.RAN: + message = ( + f"decision {decision.decision_id} is a read operation that ran when " + "it was proposed, so there is nothing to approve" + ) + else: + message = ( + f"decision {decision.decision_id} is {decision.status.value}, so " + "there is nothing left to approve" + ) + raise DecisionError(_ERROR_NOT_PROPOSED, message) + + def _outcome( execution: RunRecord, verification: RunRecord | None, @@ -797,6 +874,11 @@ def _coerced( raise DecisionError(_ERROR_INPUT_TYPE, message) +def _canonical(inputs: Mapping[str, object]) -> str: + """Return the inputs as one comparable text: ``1`` and ``true`` differ.""" + return json.dumps(dict(inputs), sort_keys=True) + + def _decision_identifier(operation_id: ResourceId, moment: datetime) -> ResourceId: stamp = moment.strftime("%Y%m%d%H%M%S") return ResourceId.from_boundary(f"{operation_id.value}-{stamp}") @@ -887,6 +969,14 @@ def _decision_from_document(document: Mapping[str, object]) -> Decision: ), status=DecisionStatus(str(spec["status"])), verdict=str(spec["verdict"]) if "verdict" in spec else None, + ran_exit_code=( + int(cast("int", spec["ranExitCode"])) if "ranExitCode" in spec else None + ), + superseded_by=( + ResourceId.from_boundary(spec["supersededBy"]) + if "supersededBy" in spec + else None + ), execution=_run_from_document(spec.get("execution")), verification=_run_from_document(spec.get("verify")), approval=_approval_from_document(approval), diff --git a/sdk/src/cloudfall/why.py b/sdk/src/cloudfall/why.py index d3d7ce0..d6fc7f2 100644 --- a/sdk/src/cloudfall/why.py +++ b/sdk/src/cloudfall/why.py @@ -30,6 +30,7 @@ _ERROR_INSTANT_INVALID = "why_instant_invalid" _ERROR_WINDOW_INVERTED = "why_window_inverted" +_ERROR_STATUS_UNKNOWN = "why_status_unknown" class WhyError(RuntimeError): @@ -90,6 +91,7 @@ class WhyQuery: operation: ResourceId | None = None since: Instant | None = None until: Instant | None = None + statuses: frozenset[DecisionStatus] = frozenset() def __post_init__(self) -> None: """Refuse a window that ends before it starts.""" @@ -111,6 +113,7 @@ def from_boundary( operation: object = None, since: object = None, until: object = None, + status: Iterable[object] = (), ) -> WhyQuery: """Build a query from the primitives a CLI or tool call carries.""" return cls( @@ -120,6 +123,7 @@ def from_boundary( ), since=Instant.from_boundary(since) if _present(since) else None, until=Instant.from_boundary(until) if _present(until) else None, + statuses=frozenset(_status(value) for value in status), ) def as_dict(self) -> dict[str, object]: @@ -133,12 +137,16 @@ def as_dict(self) -> dict[str, object]: result["since"] = self.since.as_string() if self.until is not None: result["until"] = self.until.as_string() + if self.statuses: + result["status"] = sorted(status.value for status in self.statuses) return result def matches(self, decision: Decision) -> bool: """Return whether one decision is part of the answer.""" if self.operation is not None and decision.operation_id != self.operation: return False + if self.statuses and decision.status not in self.statuses: + return False if self.host is not None and self.host.value not in hosts_of(decision): return False if self.since is None and self.until is None: @@ -251,7 +259,7 @@ def render_why_document(document: Mapping[str, object]) -> str: '

The record holds no decision this question is about.

' ) filters = ", ".join( - f"{escape(key)} = {escape(str(value))}" + f"{escape(key)} = {escape(_filter_value(value))}" for key, value in query.items() ) asked = escape(filters) if filters else "Every decision the record holds" @@ -312,6 +320,7 @@ def _render_card(entry: Mapping[str, object]) -> str: spec = cast("Mapping[str, object]", decision["spec"]) operation = cast("Mapping[str, object]", spec["operation"]) status = DecisionStatus(str(spec["status"])) + ran_exit_code = spec.get("ranExitCode") story = cast("Sequence[str]", entry["story"]) steps = "".join(f"
  • {escape(sentence)}
    • " for sentence in story) hosts = ( @@ -323,7 +332,7 @@ def _render_card(entry: Mapping[str, object]) -> str: '
    ' f"

    {escape(str(operation['id']))}

    " f"{escape(str(entry['id']))}
    " - f'' + f'' f"{escape(status.value)}
    " f"
      {steps}
    " f'
    Hosts the record names: {hosts}
    ' @@ -331,16 +340,33 @@ def _render_card(entry: Mapping[str, object]) -> str: ) -def _tone(status: DecisionStatus) -> str: +def _tone(status: DecisionStatus, ran_exit_code: object = None) -> str: + if status is DecisionStatus.RAN: + return "healthy" if ran_exit_code == 0 else "critical" if status is DecisionStatus.VERIFIED: return "healthy" if status is DecisionStatus.FAILED: return "critical" - if status is DecisionStatus.REJECTED: + if status in {DecisionStatus.REJECTED, DecisionStatus.SUPERSEDED}: return "unknown" return "warning" +def _filter_value(value: object) -> str: + if isinstance(value, list): + return ", ".join(str(entry) for entry in cast("list[object]", value)) + return str(value) + + +def _status(value: object) -> DecisionStatus: + try: + return DecisionStatus(str(value)) + except ValueError as error: + known = ", ".join(status.value for status in DecisionStatus) + message = f"status {value!r} is not one of {known}" + raise WhyError(_ERROR_STATUS_UNKNOWN, message) from error + + def _story(decision: Decision) -> Iterable[str]: """Yield the record as sentences: saw, proposed, checked, approved, ended.""" yield _saw(decision) @@ -434,6 +460,16 @@ def _ran(subject: str, run: RunRecord, log: object) -> str: def _ended(decision: Decision) -> str: if decision.status is DecisionStatus.PROPOSED: return "It is still proposed: nobody has approved or rejected it." + if decision.status is DecisionStatus.RAN: + return ( + f"It ran when it was proposed and exited {decision.ran_exit_code}: " + "a read operation waits for no approval." + ) + if decision.status is DecisionStatus.SUPERSEDED: + return ( + f"It was superseded by {decision.superseded_by}, a newer proposal " + "of the same operation, targets and inputs; nobody approved it." + ) if decision.verdict is not None: return f"It ended {decision.status.value}: {decision.verdict}." return f"It ended {decision.status.value}." diff --git a/sdk/tests/test_decision.py b/sdk/tests/test_decision.py index 047e083..6d5ebec 100644 --- a/sdk/tests/test_decision.py +++ b/sdk/tests/test_decision.py @@ -27,6 +27,7 @@ gate, propose, read_basis, + refuse_unapprovable, run_command, ) from cloudfall.domain import ResourceId @@ -38,6 +39,8 @@ if TYPE_CHECKING: from collections.abc import Callable, Mapping, Sequence + from cloudfall.decision import Decision + SCHEMAS = default_schema_directory() MOMENT = datetime(2026, 9, 21, 14, 30, 12, tzinfo=UTC) TERMINAL = ApprovalChannel.TERMINAL @@ -52,6 +55,9 @@ playbook: playbooks/health.yml """ +LATER = datetime(2026, 9, 21, 14, 45, 0, tzinfo=UTC) +LATEST = datetime(2026, 9, 21, 15, 0, 0, tzinfo=UTC) + def _store(repository: Path) -> DecisionStore: return DecisionStore( @@ -89,6 +95,25 @@ def _proposal( ) +def _legacy_read( + repository: Path, store: DecisionStore, moment: datetime +) -> Decision: + """Record a facts read as a version before #26 did: ``proposed``, no exit code.""" + decision = propose( + _proposal(repository, "facts", target=None, inputs={}), + store, + lambda: moment, + _Runner(), + ) + path = repository / DECISION_DIRECTORY / f"{decision.decision_id.value}.json" + document = json.loads(path.read_text(encoding="utf-8")) + document["spec"]["status"] = "proposed" + del document["spec"]["ranExitCode"] + del document["spec"]["verdict"] + path.write_text(json.dumps(document), encoding="utf-8") + return store.load(decision.decision_id) + + class _Runner: """A runner that writes what Ansible would write, stage by stage. @@ -358,6 +383,260 @@ def test_a_read_operation_is_gated_by_nothing(tmp_path: Path) -> None: assert decision.verify_playbook is None +def test_a_read_operation_is_recorded_as_ran_with_its_exit_code( + tmp_path: Path, +) -> None: + """A read has run by the time it is recorded; it never waits (#26).""" + repository = _repository(tmp_path, deploy=DEPLOY, facts=FACTS) + store = _store(repository) + + decision = propose( + _proposal(repository, "facts", target=None, inputs={}), + store, + lambda: MOMENT, + _Runner(exit_code=2), + ) + + assert decision.status is DecisionStatus.RAN + assert decision.ran_exit_code == 2 + assert decision.check.exit_code == 2 + assert store.load(decision.decision_id) == decision + on_disk = json.loads( + (repository / DECISION_DIRECTORY / f"{decision.decision_id.value}.json") + .read_text(encoding="utf-8") + ) + assert on_disk["spec"]["status"] == "ran" + assert on_disk["spec"]["ranExitCode"] == 2 + + +def test_a_mutating_proposal_still_waits_and_has_no_run_exit_code( + tmp_path: Path, +) -> None: + repository = _repository(tmp_path) + + decision = propose( + _proposal(repository), _store(repository), lambda: MOMENT, _Runner() + ) + + assert decision.status is DecisionStatus.PROPOSED + assert decision.ran_exit_code is None + assert "ranExitCode" not in json.dumps(decision.as_document()) + + +def test_a_new_proposal_supersedes_the_open_one_of_the_same_call( + tmp_path: Path, +) -> None: + """Proposing the same thing twice leaves one open proposal, not two (#26).""" + repository = _repository(tmp_path) + store = _store(repository) + first = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) + + second = propose(_proposal(repository), store, lambda: LATER, _Runner()) + + older = store.load(first.decision_id) + assert older.status is DecisionStatus.SUPERSEDED + assert older.superseded_by == second.decision_id + assert older.approval is None + assert older.verdict is not None + assert second.decision_id.value in older.verdict + assert store.load(second.decision_id).status is DecisionStatus.PROPOSED + on_disk = json.loads( + (repository / DECISION_DIRECTORY / f"{first.decision_id.value}.json") + .read_text(encoding="utf-8") + ) + assert on_disk["spec"]["supersededBy"] == second.decision_id.value + + +@pytest.mark.parametrize( + ("target", "inputs"), + [ + ("web-2", {"version": "1.4.0"}), + ("web-1", {"version": "1.5.0"}), + ("web-1", {"version": "1.4.0", "force": "yes"}), + ], + ids=["other-target", "other-input", "extra-input"], +) +def test_a_proposal_of_another_call_supersedes_nothing( + tmp_path: Path, target: str, inputs: dict[str, object] +) -> None: + repository = _repository(tmp_path) + store = _store(repository) + first = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) + + propose( + _proposal(repository, target=target, inputs=inputs), + store, + lambda: LATER, + _Runner(), + ) + + assert store.load(first.decision_id).status is DecisionStatus.PROPOSED + + +def test_superseding_never_touches_a_decision_that_ran(tmp_path: Path) -> None: + """Only an open proposal is replaced: what ran stays what it was.""" + repository = _repository(tmp_path) + store = _store(repository) + first = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) + approved = approve( + ApprovalRequest( + decision=first, approver="roman", repository=repository, via=TERMINAL + ), + store, + lambda: MOMENT, + _Runner(), + ) + + propose(_proposal(repository), store, lambda: LATER, _Runner()) + + assert store.load(first.decision_id) == approved + assert approved.status is DecisionStatus.VERIFIED + + +def test_a_read_run_supersedes_a_read_recorded_before_ran_existed( + tmp_path: Path, +) -> None: + """A record on disk that says ``proposed`` for a read still loads (#26).""" + repository = _repository(tmp_path, deploy=DEPLOY, facts=FACTS) + store = _store(repository) + old = _legacy_read(repository, store, MOMENT) + assert old.status is DecisionStatus.PROPOSED + + new = propose( + _proposal(repository, "facts", target=None, inputs={}), + store, + lambda: LATER, + _Runner(), + ) + + assert new.status is DecisionStatus.RAN + assert store.load(old.decision_id).superseded_by == new.decision_id + + +def test_a_superseded_read_does_not_point_at_a_run_to_approve( + tmp_path: Path, +) -> None: + """The read that replaced it ran already; there is nothing to approve.""" + repository = _repository(tmp_path, deploy=DEPLOY, facts=FACTS) + store = _store(repository) + old = _legacy_read(repository, store, MOMENT) + new = propose( + _proposal(repository, "facts", target=None, inputs={}), + store, + lambda: LATER, + _Runner(), + ) + + with pytest.raises(DecisionError) as error: + refuse_unapprovable(store.load(old.decision_id)) + + assert error.value.code == "decision_not_proposed" + assert new.decision_id.value in error.value.detail + assert "approve that one" not in error.value.detail + assert "ran when it was proposed" in error.value.detail + + +@pytest.mark.parametrize("status", ["ran", "superseded"]) +def test_approving_a_decision_that_is_not_open_is_refused( + tmp_path: Path, status: str +) -> None: + repository = _repository(tmp_path, deploy=DEPLOY, facts=FACTS) + store = _store(repository) + if status == "ran": + decision = propose( + _proposal(repository, "facts", target=None, inputs={}), + store, + lambda: MOMENT, + _Runner(), + ) + else: + first = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) + propose(_proposal(repository), store, lambda: LATER, _Runner()) + decision = store.load(first.decision_id) + run = _Runner() + + with pytest.raises(DecisionError) as error: + approve( + ApprovalRequest( + decision=decision, + approver="roman", + repository=repository, + via=TERMINAL, + ), + store, + lambda: MOMENT, + run, + ) + + assert error.value.code == "decision_not_proposed" + assert run.recorded == [] + assert store.load(decision.decision_id) == decision + + +def test_the_issue_repro_lists_ran_superseded_and_proposed( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A read, then the same mutating proposal twice (#26).""" + repository = _repository(tmp_path, deploy=DEPLOY, facts=FACTS) + store = _store(repository) + propose( + _proposal(repository, "facts", target=None, inputs={}), + store, + lambda: MOMENT, + _Runner(), + ) + propose(_proposal(repository), store, lambda: LATER, _Runner()) + propose(_proposal(repository), store, lambda: LATEST, _Runner()) + + exit_code = main(["decisions", "list", "--repository", str(repository)]) + every = json.loads(capsys.readouterr().out) + open_only = main( + [ + *("decisions", "list", "--repository", str(repository)), + *("--status", "proposed"), + ] + ) + waiting = json.loads(capsys.readouterr().out) + settled = main( + [ + *("decisions", "list", "--repository", str(repository)), + *("--status", "ran", "--status", "superseded"), + ] + ) + closed = json.loads(capsys.readouterr().out) + + assert exit_code == open_only == settled == 0 + assert [entry["spec"]["status"] for entry in every["data"]["decisions"]] == [ + "ran", + "superseded", + "proposed", + ] + assert [entry["metadata"]["id"] for entry in waiting["data"]["decisions"]] == [ + "deploy-20260921150000" + ] + assert [entry["spec"]["status"] for entry in closed["data"]["decisions"]] == [ + "ran", + "superseded", + ] + + +def test_an_unknown_status_filter_is_refused( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + repository = _repository(tmp_path) + + exit_code = main( + [ + *("decisions", "list", "--repository", str(repository)), + *("--status", "stale"), + ] + ) + + payload = json.loads(capsys.readouterr().out) + assert exit_code == 2 + assert payload["error"]["code"] == "ARG_ERROR" + + def test_the_cli_proposes_and_then_lists_the_record( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: @@ -600,17 +879,13 @@ def __call__( assert approved.verification.exit_code == 1 -def test_a_read_operation_without_verify_is_executed_not_verified( +def test_an_operation_without_verify_is_executed_not_verified( tmp_path: Path, ) -> None: + """Only a read declares no verify step, and only one recorded before #26 waits.""" repository = _repository(tmp_path, deploy=DEPLOY, facts=FACTS) store = _store(repository) - proposed = propose( - _proposal(repository, "facts", target=None, inputs={}), - store, - lambda: MOMENT, - _Runner(), - ) + proposed = _legacy_read(repository, store, MOMENT) approved = approve( ApprovalRequest( @@ -776,6 +1051,30 @@ def test_an_approval_off_a_terminal_is_refused( assert store.load(proposed.decision_id).status is DecisionStatus.PROPOSED +def test_the_cli_refuses_a_superseded_decision_before_asking_for_its_id( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """Nobody types an id only to hear there was nothing to approve (#26).""" + repository = _repository(tmp_path) + store = _store(repository) + first = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) + second = propose(_proposal(repository), store, lambda: LATER, _Runner()) + + exit_code = main( + [ + *("decisions", "approve", first.decision_id.value), + *("--repository", str(repository), "--approver", "roman"), + ] + ) + + payload = json.loads(capsys.readouterr().out) + assert exit_code == 4 + assert payload["error"]["code"] == "PRECONDITION" + assert payload["error"]["context"]["code"] == "decision_not_proposed" + assert second.decision_id.value in payload["error"]["message"] + assert store.load(first.decision_id).status is DecisionStatus.SUPERSEDED + + def test_a_raw_payload_cannot_approve_off_a_terminal( tmp_path: Path, capsys: pytest.CaptureFixture[str] ) -> None: @@ -924,12 +1223,7 @@ def test_every_outcome_says_why_in_the_record(tmp_path: Path) -> None: ) executed = approve( ApprovalRequest( - decision=propose( - _proposal(repository, "facts", target=None, inputs={}), - store, - lambda: MOMENT, - _Runner(), - ), + decision=_legacy_read(repository, store, MOMENT), approver="roman", repository=repository, via=TERMINAL, diff --git a/sdk/tests/test_why.py b/sdk/tests/test_why.py index 8977f4e..ca8fa53 100644 --- a/sdk/tests/test_why.py +++ b/sdk/tests/test_why.py @@ -29,7 +29,7 @@ def _record(repository: Path) -> tuple[Decision, Decision]: - """One verified deploy on web-1, then a proposed facts read of the fleet.""" + """One verified deploy on web-1, then a facts read of the fleet that ran.""" store = _store(repository) deploy = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) deploy = approve( @@ -168,11 +168,66 @@ def test_every_sentence_of_the_story_cites_the_record(tmp_path: Path) -> None: "It ended verified: the run succeeded and the verify step changed nothing." ) - proposed = read.explanations[0].story - assert facts.status is DecisionStatus.PROPOSED - assert "was proposed against the whole fleet with no inputs." in proposed[1] - assert proposed[3].startswith("Nothing gated it:") - assert proposed[-1] == ("It is still proposed: nobody has approved or rejected it.") + ran = read.explanations[0].story + assert facts.status is DecisionStatus.RAN + assert "was proposed against the whole fleet with no inputs." in ran[1] + assert ran[3].startswith("Nothing gated it:") + assert ran[-1] == ( + "It ran when it was proposed and exited 0: a read operation waits for " + "no approval." + ) + + +def test_a_superseded_proposal_says_what_replaced_it(tmp_path: Path) -> None: + repository = _repository(tmp_path) + store = _store(repository) + first = propose(_proposal(repository), store, lambda: MOMENT, _Runner()) + second = propose(_proposal(repository), store, lambda: LATER, _Runner()) + + told = answer(store, WhyQuery.from_boundary(status=["superseded"])) + + assert [e.decision.decision_id for e in told.explanations] == [first.decision_id] + assert told.explanations[0].story[-1] == ( + f"It was superseded by {second.decision_id.value}, a newer proposal of " + "the same operation, targets and inputs; nobody approved it." + ) + + +def test_a_status_question_is_answered( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """``--status`` is repeatable and keeps only the statuses asked for (#26).""" + repository = _repository(tmp_path) + _record(repository) + + exit_code = main( + [ + *("why", "--repository", str(repository)), + *("--status", "ran", "--status", "proposed"), + ] + ) + + payload = json.loads(capsys.readouterr().out) + assert exit_code == 0 + assert payload["data"]["query"] == {"status": ["proposed", "ran"]} + assert [entry["id"] for entry in payload["data"]["answers"]] == [ + "facts-20260922080000" + ] + + +def test_an_unknown_status_question_is_refused( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + repository = _repository(tmp_path) + + exit_code = main(["why", "--repository", str(repository), "--status", "stale"]) + + payload = json.loads(capsys.readouterr().out) + assert exit_code == 2 + assert payload["error"]["code"] == "ARG_ERROR" + with pytest.raises(WhyError) as error: + WhyQuery.from_boundary(status=["stale"]) + assert error.value.code == "why_status_unknown" def test_the_cli_answers_in_json_without_a_catalog( @@ -217,7 +272,7 @@ def test_the_cli_answers_as_a_page( assert "Cloudfall Why" in page assert "2 decisions" in page assert 'class="badge healthy">verified' in page - assert 'class="badge warning">proposed' in page + assert 'class="badge healthy">ran' in page assert "roman approved it at 2026-09-21T14:30:12Z (via terminal)" in page