Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/plainweave/mcp_surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -1252,7 +1252,10 @@ def _append_preflight_fact(
ErrorCode.INTERNAL,
"preflight severity is not supported",
recoverable=True,
hint="Refresh local Plainweave state and retry.",
hint=(
f"Internal defect: preflight severity must be one of {sorted(PREFLIGHT_SEVERITIES)}; "
"refreshing local state will not help."
),
)
facts.append(
{
Expand Down
99 changes: 89 additions & 10 deletions src/plainweave/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,38 @@
from plainweave.store import connect, read_schema_meta
from plainweave.wardline_adapter import WardlineAdapter

#: Cause-appropriate default hints, one per :class:`ErrorCode` member. An error
#: must SAY WHAT IT KNOWS: a hint that does not point at the real fix actively
#: misdirects, so VALIDATION/NOT_FOUND must NEVER inherit a stale-state
#: "refresh and retry" hint (their cause is bad input / a missing id, not local
#: staleness). CONFLICT legitimately points at refetch-and-retry because reused
#: or superseded state IS its genuine cause. Sites that know more (e.g. the
#: version guard) override with a precise per-call hint. Advisory only — no
#: allow/block/approved/verdict/gate tokens.
_DEFAULT_ERROR_HINTS: dict[ErrorCode, str] = {
ErrorCode.VALIDATION: "Correct the invalid input described in the message and retry.",
ErrorCode.NOT_FOUND: "Confirm the referenced id exists (list or search to find it) and retry.",
ErrorCode.CONFLICT: (
"Refetch the current state and retry - the value you sent no longer matches (it changed or was already used)."
),
ErrorCode.POLICY_REQUIRED: "Satisfy the precondition named in the message before retrying.",
ErrorCode.PEER_ABSENT: (
"The named peer facts are absent; enable or supply the sibling tool, or proceed treating enrichment as absent."
),
ErrorCode.PEER_STALE: (
"The peer's facts are stale relative to the current tree; re-run the peer's analysis to refresh them."
),
ErrorCode.PEER_CONTRACT: (
"The peer returned data that does not match the expected contract; check the peer's version and output."
),
ErrorCode.LOCKED: ("The resource is held by another operation; wait for it to be freed or retry shortly."),
ErrorCode.UNSUPPORTED: "Use one of the supported operations or values described in the message.",
ErrorCode.INTERNAL: (
"Internal defect: this should not occur from normal input and refreshing "
"local state will not help; report it with the message and details."
),
}


class PlainweaveService:
#: Actor kinds permitted to record external/manual attestation authority
Expand Down Expand Up @@ -526,9 +558,19 @@ def update_draft(
if not isinstance(draft_id, str):
raise self._error(ErrorCode.POLICY_REQUIRED, "requirement has no active draft")
draft = self._draft_row(connection, draft_id)
if expected_draft_revision is not None and int(draft["draft_revision"]) != expected_draft_revision:
raise self._error(ErrorCode.CONFLICT, "draft revision conflict")
next_revision = int(draft["draft_revision"]) + 1
current_draft_revision = int(draft["draft_revision"])
if expected_draft_revision is not None and current_draft_revision != expected_draft_revision:
raise self._error(
ErrorCode.CONFLICT,
f"expected draft revision {expected_draft_revision} does not match "
f"current draft revision {current_draft_revision}",
hint=f"Retry with --expected-draft-revision {current_draft_revision}.",
details={
"expected_draft_revision": expected_draft_revision,
"current_draft_revision": current_draft_revision,
},
)
next_revision = current_draft_revision + 1
next_title = title if title is not None else str(draft["title"])
next_statement = statement if statement is not None else str(draft["statement"])
connection.execute(
Expand Down Expand Up @@ -2101,7 +2143,12 @@ def _draft_row(self, connection: sqlite3.Connection, draft_id: str) -> sqlite3.R
def _require_current_version(self, requirement: sqlite3.Row, expected_version: int) -> None:
current_version = int(requirement["current_version"])
if current_version != expected_version:
raise self._error(ErrorCode.CONFLICT, "expected version does not match current version")
raise self._error(
ErrorCode.CONFLICT,
f"expected version {expected_version} does not match current version {current_version}",
hint=f"Retry with --expected-version {current_version}.",
details={"expected_version": expected_version, "current_version": current_version},
)

def _project_key(self, connection: sqlite3.Connection) -> str:
metadata = read_schema_meta(connection)
Expand Down Expand Up @@ -2785,7 +2832,14 @@ def _validate_trace_relation(self, from_ref: TraceRef, relation: str, to_ref: Tr
("legis_attestation", "attests", "requirement_version"),
}
if (from_ref.kind, relation, to_ref.kind) not in allowed:
raise self._error(ErrorCode.VALIDATION, "trace relation is not canonical")
raise self._error(
ErrorCode.VALIDATION,
"trace relation is not canonical",
hint=(
"Use a canonical (from-kind, relation, to-kind) triple, "
"e.g. loomweave_entity satisfies requirement_version."
),
)

def _validate_trace_transition(self, current: str, target: str) -> None:
allowed = {
Expand Down Expand Up @@ -2975,15 +3029,30 @@ def _require_accepted_trace_targets(self, connection: sqlite3.Connection, to_ref

def _require_actor(self, actor: str) -> None:
if not actor:
raise self._error(ErrorCode.VALIDATION, "actor is required")
raise self._error(
ErrorCode.VALIDATION,
"actor is required",
hint=(
"Pass --actor <acting-identity> (e.g. --actor claude); "
"it need not be pre-registered for req/criterion commands."
),
)

def _validate_verification_method(self, method: str) -> None:
if method not in {"test", "analysis", "inspection", "manual"}:
raise self._error(ErrorCode.VALIDATION, "verification method is not supported")
raise self._error(
ErrorCode.VALIDATION,
"verification method is not supported",
hint="Pass --method as one of: test, analysis, inspection, manual.",
)

def _validate_evidence_status(self, status: str) -> None:
if status not in {"passing", "failing", "inconclusive", "waived"}:
raise self._error(ErrorCode.VALIDATION, "verification evidence status is not supported")
raise self._error(
ErrorCode.VALIDATION,
"verification evidence status is not supported",
hint="Pass --status as one of: passing, failing, inconclusive, waived.",
)

def _actor_kind(self, connection: sqlite3.Connection, actor: str) -> str | None:
"""Return the registered kind for ``actor`` or ``None`` if unregistered."""
Expand Down Expand Up @@ -3025,8 +3094,18 @@ def _evidence_authority(self, connection: sqlite3.Connection, method: str, statu
return "human_attested"
return "agent_reported"

def _error(self, code: ErrorCode, message: str) -> PlainweaveError:
return PlainweaveError(code, message, recoverable=True, hint="Refresh local Plainweave state and retry.")
def _error(
self,
code: ErrorCode,
message: str,
*,
hint: str | None = None,
details: dict[str, object] | None = None,
) -> PlainweaveError:
resolved_hint = (
hint if hint is not None else _DEFAULT_ERROR_HINTS.get(code, "Review the error message and retry.")
)
return PlainweaveError(code, message, recoverable=True, hint=resolved_hint, details=details)

def _optional_int(self, value: object) -> int | None:
return int(str(value)) if value is not None else None
Expand Down
12 changes: 12 additions & 0 deletions tests/contracts/test_cli_contract_outputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ def test_cli_error_output_matches_contract_fixture(
expected_status=2,
)
assert_matches_fixture(error, load_fixture("cli/error-validation-json.json"))
# The missing-actor hint must point at --actor, not at stale local state,
# and must not silently grow details away from the pinned {}.
assert "--actor" in error["error"]["hint"]
assert "Refresh local Plainweave state" not in error["error"]["hint"]
assert error["error"]["details"] == {}


def test_cli_conflict_error_output_matches_contract_fixture(
Expand Down Expand Up @@ -193,6 +198,13 @@ def test_cli_conflict_error_output_matches_contract_fixture(
expected_status=2,
)
assert_matches_fixture(error, load_fixture("cli/error-conflict-json.json"))
# Say what you know: both version numbers surface in the message and details,
# the hint names the version to retry with, and it is not the stale-state blanket.
assert "expected version 0" in error["error"]["message"]
assert "current version 1" in error["error"]["message"]
assert error["error"]["details"] == {"expected_version": 0, "current_version": 1}
assert "--expected-version 1" in error["error"]["hint"]
assert "Refresh local Plainweave state" not in error["error"]["hint"]


def test_criterion_cli_output_matches_contract_fixture(
Expand Down
9 changes: 6 additions & 3 deletions tests/fixtures/contracts/cli/error-conflict-json.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@
"ok": false,
"error": {
"code": "CONFLICT",
"message": "expected version does not match current version",
"message": "expected version 0 does not match current version 1",
"recoverable": true,
"hint": "Refresh local Plainweave state and retry.",
"details": {}
"hint": "Retry with --expected-version 1.",
"details": {
"expected_version": 0,
"current_version": 1
}
},
"warnings": [],
"meta": {
Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/contracts/cli/error-missing-actor.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"code": "VALIDATION",
"message": "actor is required",
"recoverable": true,
"hint": "Refresh local Plainweave state and retry.",
"hint": "Pass --actor <acting-identity> (e.g. --actor claude); it need not be pre-registered for req/criterion commands.",
"details": {}
},
"warnings": [],
Expand Down
2 changes: 1 addition & 1 deletion tests/fixtures/contracts/cli/error-validation-json.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"code": "VALIDATION",
"message": "actor is required",
"recoverable": true,
"hint": "Refresh local Plainweave state and retry.",
"hint": "Pass --actor <acting-identity> (e.g. --actor claude); it need not be pre-registered for req/criterion commands.",
"details": {}
},
"warnings": [],
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/contracts/envelopes/error-conflict.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
"ok": false,
"error": {
"code": "CONFLICT",
"message": "expected version 1 but current version is 2",
"message": "expected version 1 does not match current version 2",
"recoverable": true,
"hint": "Refetch the requirement and retry with the current version.",
"hint": "Retry with --expected-version 2.",
"details": {
"expected_version": 1,
"current_version": 2
Expand Down
151 changes: 151 additions & 0 deletions tests/test_error_hints.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
"""Anti-vacuous guards for the honest error-hint layer (functional honesty).

An error must SAY WHAT IT KNOWS. A hint that does not point at the real fix
actively misdirects, so these tests pin three properties that the frozen
``weft.plainweave.error.v1`` envelope must keep honouring additively:

1. Every :class:`ErrorCode` has a non-empty default hint, and VALIDATION /
NOT_FOUND never inherit the stale-state "Refresh local Plainweave state and
retry." blanket (their cause is bad input / a missing id, not staleness).
2. ``_error`` resolves an omitted hint from that map, threads an explicit hint
verbatim, and always coerces ``details`` to a dict.
3. The two dogfood findings (CONFLICT version guard; missing-actor VALIDATION)
emit precise, cause-appropriate hints and details end-to-end.
"""

from __future__ import annotations

from pathlib import Path

import pytest

from plainweave.errors import ErrorCode, PlainweaveError
from plainweave.service import _DEFAULT_ERROR_HINTS, PlainweaveService
from plainweave.store import migrate

_BLANKET_HINT = "Refresh local Plainweave state and retry."


def service_for(tmp_path: Path) -> PlainweaveService:
db_path = tmp_path / ".plainweave" / "plainweave.db"
migrate(db_path, project_key="AUTH")
return PlainweaveService(db_path)


def test_default_hint_map_covers_every_error_code_non_empty() -> None:
for code in ErrorCode:
assert code in _DEFAULT_ERROR_HINTS, f"missing default hint for {code}"
hint = _DEFAULT_ERROR_HINTS[code]
assert hint and hint.strip(), f"empty default hint for {code}"


def test_validation_and_not_found_defaults_are_never_stale_state_hints() -> None:
for code in (ErrorCode.VALIDATION, ErrorCode.NOT_FOUND):
hint = _DEFAULT_ERROR_HINTS[code].lower()
assert _BLANKET_HINT.lower() not in hint
# No refresh/refetch-local-state phrasing either: the cause is the input,
# not local staleness. (CONFLICT may legitimately say "refetch".)
assert "refresh" not in hint
assert "refetch" not in hint


def test_error_resolves_default_hint_when_none(tmp_path: Path) -> None:
service = service_for(tmp_path)

error = service._error(ErrorCode.NOT_FOUND, "requirement not found: REQ-X")

assert error.hint == _DEFAULT_ERROR_HINTS[ErrorCode.NOT_FOUND]
assert error.hint
assert error.details == {}


def test_error_threads_explicit_hint_and_details_verbatim(tmp_path: Path) -> None:
service = service_for(tmp_path)

error = service._error(
ErrorCode.CONFLICT,
"boom",
hint="Retry with --expected-version 7.",
details={"expected_version": 3, "current_version": 7},
)

assert error.hint == "Retry with --expected-version 7."
assert error.details == {"expected_version": 3, "current_version": 7}
assert isinstance(error.details, dict)


def test_conflict_version_guard_says_what_it_knows(tmp_path: Path) -> None:
# add -> current version 0; approving with expected_version 1 trips the guard.
service = service_for(tmp_path)
draft = service.create_requirement(
"Reject expired bearer tokens", "The API shall reject expired tokens.", "human:john"
)

with pytest.raises(PlainweaveError) as exc_info:
service.approve_requirement(draft.id, actor="human:john", expected_version=1, idempotency_key="approve-1")

error = exc_info.value
assert error.code == ErrorCode.CONFLICT
assert error.details["current_version"] == 0
assert error.details["expected_version"] == 1
assert "current version 0" in error.message
assert "--expected-version 0" in error.hint
assert _BLANKET_HINT not in error.hint


def test_draft_revision_guard_says_what_it_knows(tmp_path: Path) -> None:
# The same optimistic-concurrency defect as the requirement-version guard, on
# a sibling field: updating a draft with a wrong expected_draft_revision must
# disclose the actual revision (message + details + a --expected-draft-revision hint).
service = service_for(tmp_path)
draft = service.create_requirement(
"Reject expired bearer tokens", "The API shall reject expired tokens.", "human:john"
)

with pytest.raises(PlainweaveError) as exc_info:
service.update_draft(draft.id, actor="human:john", statement="revised", expected_draft_revision=99)

error = exc_info.value
assert error.code == ErrorCode.CONFLICT
current = error.details["current_draft_revision"]
assert error.details["expected_draft_revision"] == 99
assert f"current draft revision {current}" in error.message
assert f"--expected-draft-revision {current}" in error.hint
assert _BLANKET_HINT not in error.hint


def test_missing_actor_error_points_at_actor_not_stale_state(tmp_path: Path) -> None:
service = service_for(tmp_path)

with pytest.raises(PlainweaveError) as exc_info:
service.create_requirement("Reject expired bearer tokens", "The API shall reject expired tokens.", "")

error = exc_info.value
assert error.code == ErrorCode.VALIDATION
assert "--actor" in error.hint
assert _BLANKET_HINT not in error.hint
# The missing-actor golden pins details:{}; enrichment must not silently grow it.
assert error.details == {}


def _assert_validation_without_blanket_hint(exc_info: pytest.ExceptionInfo[PlainweaveError]) -> None:
assert exc_info.value.code == ErrorCode.VALIDATION
assert exc_info.value.hint != _BLANKET_HINT
assert _BLANKET_HINT not in exc_info.value.hint


def test_no_service_path_validation_error_carries_the_blanket_hint(tmp_path: Path) -> None:
"""Guard: a VALIDATION error must never inherit the stale-state blanket hint."""
service = service_for(tmp_path)

with pytest.raises(PlainweaveError) as missing_actor:
service.create_requirement("t", "s", "")
_assert_validation_without_blanket_hint(missing_actor)

with pytest.raises(PlainweaveError) as bad_method:
service._validate_verification_method("nonsense")
_assert_validation_without_blanket_hint(bad_method)

with pytest.raises(PlainweaveError) as bad_status:
service._validate_evidence_status("nonsense")
_assert_validation_without_blanket_hint(bad_status)
5 changes: 5 additions & 0 deletions tests/test_mcp_read_surface.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ def assert_error(envelope: dict[str, Any], code: str) -> None:
assert envelope["error"]["code"] == code
assert envelope["error"]["recoverable"] is True
assert envelope["error"]["hint"]
assert isinstance(envelope["error"]["details"], dict)
# Functional honesty: bad-input / missing-id errors must never inherit the
# stale-state "refresh and retry" blanket hint — it would misdirect the agent.
if code in {"VALIDATION", "NOT_FOUND"}:
assert envelope["error"]["hint"] != "Refresh local Plainweave state and retry."


_VERDICT_KEYS = {"allow", "allowed", "block", "blocked", "verdict", "decision", "gate", "enforcement"}
Expand Down
Loading