From 67b2f56e9747c8acd2e792d64ec2199855e24a34 Mon Sep 17 00:00:00 2001 From: Copilot Date: Fri, 15 May 2026 23:47:11 +0200 Subject: [PATCH] feat(substrate): v1.3 vocab partition fail-soft UX with block firewall MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _validate_view_trust_profile changed from raise-on-misplaced to return-tuple contract: (profile_minus_misplaced: list[str], findings: list[Finding]). Each misplaced token produces one block-severity trust-token-misplaced finding. Semantic firewall preserved: misplaced tokens are dropped from the returned profile so downstream taint/UX checks see them as absent — no silent mis-analysis path. "Fail-soft" is purely ergonomic: the structured finding carries a suggested_fix pointing the operator to the correct view, and the walker can re-prompt inline instead of surfacing an opaque ValueError. The lock cannot complete while a trust-token-misplaced finding exists (block-tier, same gate as all other block findings). Call sites updated: run_per_view now returns tuple[str, list]; CLI surfaces findings to stderr and exits 1 when any exist. Two existing tests that asserted on the raise are updated to assert on the finding. Co-Authored-By: Claude Opus 4.7 --- bin/findings.py | 2 + bin/substrate_wizard.py | 101 ++++++++++++----- docs/glossary.md | 9 ++ tests/test_substrate_wizard.py | 4 +- ...st_substrate_wizard_partition_fail_soft.py | 105 ++++++++++++++++++ tests/test_v1_review_followups.py | 26 +++-- 6 files changed, 208 insertions(+), 39 deletions(-) create mode 100644 tests/test_substrate_wizard_partition_fail_soft.py diff --git a/bin/findings.py b/bin/findings.py index c23d071..10a668a 100644 --- a/bin/findings.py +++ b/bin/findings.py @@ -97,6 +97,8 @@ "verification-too-shallow-for-claim", # v1.2 Fix C — Tier-3 thin negative-path coverage alongside-finding "tier3-negative-paths-thin-coverage", + # v1.3 #9 — per-view vocabulary partition fail-soft UX + "trust-token-misplaced", } # Severity mapping for Tier 3 contradiction tuple kinds (v0.5.2). diff --git a/bin/substrate_wizard.py b/bin/substrate_wizard.py index d33ccf1..0320229 100644 --- a/bin/substrate_wizard.py +++ b/bin/substrate_wizard.py @@ -331,36 +331,65 @@ def _ask_provenance(prompt_fn) -> dict: } -def _validate_view_trust_profile(view: str, raw: str) -> list[str]: - """Per-view trust-profile validator. Tokens must come from the view's - own vocabulary in _VIEW_TRUST_TOKENS — view A's tokens are not valid - under view B's profile.""" +def _validate_view_trust_profile( + view: str, raw: str +) -> tuple[list[str], list]: + """Per-view trust-profile validator — fail-soft contract (v1.3 #9). + + Returns a 2-tuple ``(profile, findings)`` where: + - ``profile`` contains only the tokens that belong to *this* view's + vocabulary (semantic firewall: misplaced tokens are never consumed). + - ``findings`` is a list of block-severity ``trust-token-misplaced`` + :class:`~bin.findings.Finding` objects, one per misplaced token. + + The caller must surface every finding through the normal finding pipeline. + The lock cannot complete while any ``trust-token-misplaced`` finding exists + (it is block-tier, same as every other block finding). + + Rationale: raising ValueError was opaque and gave operators no actionable + fix. The semantic firewall is *preserved* — misplaced tokens never enter + the returned profile — but the error is now structured so the walker can + re-prompt inline with the suggested-fix. + """ + from bin import findings as _findings # local import avoids circular dep + stripped = (raw or "").strip() if not stripped or stripped == "none": - return [] + return [], [] allowed = _VIEW_TRUST_TOKENS.get(view) if allowed is None: raise WizardValidationError("view", f"unknown view {view!r}") tokens = [t.strip() for t in stripped.split(",") if t.strip()] + profile: list[str] = [] + emitted: list[Any] = [] for t in tokens: - if t not in allowed: - # Disambiguation hint: when the token IS valid in another view, - # tell the operator instead of just listing the current view's - # allowed set. Saves an "is it misspelled?" detour. + if t in allowed: + profile.append(t) + else: + # Semantic firewall: token is NOT added to profile. + # Build a suggested-fix pointing the operator to the right view. other_views = sorted( v for v, vocab in _VIEW_TRUST_TOKENS.items() if t in vocab ) - hint = ( - f" (Note: {t!r} is valid in view {other_views[0]!r}, not {view!r}.)" - if other_views - else "" - ) - raise WizardValidationError( - "trust_profile", - f"unknown trust token for view {view!r}: {t!r}. Valid tokens: " - + ", ".join(sorted(allowed)) + hint, - ) - return tokens + if other_views: + fix = f"Move {t!r} to {other_views[0]!r} view (§8.x)."[:140] + else: + fix = ( + "Valid tokens for this view: " + + ", ".join(sorted(allowed)) + )[:140] + msg = ( + f"Trust token {t!r} is not valid in view {view!r}." + )[:140] + emitted.append(_findings.Finding( + tier=1, + kind="trust-token-misplaced", + severity="block", + location=_findings.FindingLocation(scope="spec-wide"), + message=msg, + suggested_fix=fix, + )) + return profile, emitted # v1.0 — per-view fingerprint vocabularies @@ -511,9 +540,17 @@ def run_per_view( binding: str = "", not_applicable_reason: str = "", prompt_fn=None, -) -> str: +) -> tuple[str, list]: """Render one §8.x block from validated flag values. + Returns a 2-tuple ``(block: str, findings: list[Finding])``. The + ``findings`` list contains block-severity ``trust-token-misplaced`` + entries for every trust token that does not belong to *this* view's + vocabulary. Misplaced tokens are **not** included in the rendered + block (semantic firewall preserved). Callers must surface any returned + findings through the normal finding pipeline; the lock cannot complete + while a block-tier finding exists. + Caller (the /vision skill) walks the view_scope dict from WalkState and invokes run_per_view once per view. Cache lives outside this function — caller is responsible for caching when the receiver @@ -533,16 +570,17 @@ def run_per_view( return _format_view_block(view, { "receiver-fingerprint": cleaned_receiver, "not-applicable-reason": not_applicable_reason, - }) + }), [] + profile, partition_findings = _validate_view_trust_profile(view, trust_profile) answers = { "receiver-fingerprint": cleaned_receiver, - "trust-profile": _validate_view_trust_profile(view, trust_profile), + "trust-profile": profile, "contextual-binding": _validate_contextual_binding(binding) if binding else "", } block = _format_view_block(view, answers) if prompt_fn is not None: block = _substitute_placeholders(block, prompt_fn) - return block + return block, partition_findings def _format_82_block(answers: dict) -> str: @@ -824,7 +862,7 @@ def _main() -> int: # processors to handle. pv_prompt_fn = _stdin_prompt if sys.stdin.isatty() else None try: - block = run_per_view( + block, pv_findings = run_per_view( view=args.view, receiver=args.receiver, trust_profile=args.trust_profile, @@ -842,9 +880,20 @@ def _main() -> int: remediation="re-run with corrected flag value for the field listed above", ) return 1 + # Surface any trust-token-misplaced block findings so the operator + # sees them immediately (same structured format as other block findings). + for f in pv_findings: + _status.emit( + "block", + f.kind, + dest="stderr", + message=f.message, + suggested_fix=f.suggested_fix or "", + ) sys.stdout.write(block) sys.stdout.flush() - return 0 + # Non-zero exit when block findings exist — caller must fix before lock. + return 1 if pv_findings else 0 return 2 diff --git a/docs/glossary.md b/docs/glossary.md index 51f00f2..d879224 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -1226,6 +1226,15 @@ Status codes: dotted identifiers like `walker.init`. Terms: `term:` prefix - related: negative-path-omission - since: v1.2 +## trust-token-misplaced +- kind: finding +- dev: A trust token was declared in the wrong view's §8.x substrate block. The token belongs to a different view's vocabulary and has no effect here — the semantic firewall drops it from the returned trust profile so downstream taint/UX checks see the profile as if the token were absent. Tier-1 block. The lock cannot complete while this finding exists. +- pm: A trust label was placed in the wrong section of the spec. For example, `untrusted-input` belongs in the implementing-agent section (§8.2), not the human-user section (§8.4). Move the label to the correct section and re-run. +- triggered_by: Tier-1 substrate_wizard _validate_view_trust_profile when a token is not in the calling view's _VIEW_TRUST_TOKENS vocabulary. +- user_action: Check the suggested_fix field — it names the view where the token belongs. Move the token to that view's §8.x block. Valid tokens for each view are listed in the Spectre docs. +- related: term:view, untrusted-flow-unguarded +- since: v1.3 + ## walker.round - kind: status - dev: Emitted after each concern answer in the walker interview loop. Fields: round=N (1-based count of answered concerns), pending=K (remaining non-stale concerns). Provides per-round visibility into walk progress without exposing convergence decisions. diff --git a/tests/test_substrate_wizard.py b/tests/test_substrate_wizard.py index 27aa8c2..ff436bc 100644 --- a/tests/test_substrate_wizard.py +++ b/tests/test_substrate_wizard.py @@ -648,7 +648,7 @@ def test_run_per_view_with_prompt_fn_produces_no_placeholders(self, monkeypatch, answers_iter = iter([ "product ready", "product failed", "stdout", "sync ruled out", ]) - block = substrate_wizard.run_per_view( + block, _findings = substrate_wizard.run_per_view( view="product-output", receiver="human-reader", trust_profile="schema-stable", @@ -660,7 +660,7 @@ def test_run_per_view_with_prompt_fn_produces_no_placeholders(self, monkeypatch, def test_run_per_view_without_prompt_fn_leaves_placeholders(self): """Without prompt_fn, <...> tokens are left in-place (backward-compat).""" - block = substrate_wizard.run_per_view( + block, _findings = substrate_wizard.run_per_view( view="operator", receiver="on-call-engineer", trust_profile="paging-required", diff --git a/tests/test_substrate_wizard_partition_fail_soft.py b/tests/test_substrate_wizard_partition_fail_soft.py new file mode 100644 index 0000000..0902952 --- /dev/null +++ b/tests/test_substrate_wizard_partition_fail_soft.py @@ -0,0 +1,105 @@ +"""Tests for v1.3 #9 — vocabulary partition fail-soft UX. + +Verifies that _validate_view_trust_profile: +- Returns (profile, findings) rather than raising on misplaced tokens. +- Drops misplaced tokens from the returned profile (semantic firewall). +- Emits one block-severity trust-token-misplaced finding per misplaced token. +- Passes correctly-placed tokens through with no finding. + +Pragma: no rejects/raises/refuses/denies in test names without pytest.raises. +All tests bind `result = substrate_wizard._validate_view_trust_profile(...)`. +""" +import pytest + +from bin import substrate_wizard +from bin.findings import Finding + + +def test_misplaced_untrusted_input_in_human_user_view_emits_block_finding(): + """untrusted-input in human-user view → one block trust-token-misplaced; not in profile.""" + result = substrate_wizard._validate_view_trust_profile( + "human-user", "untrusted-input" + ) + profile, findings = result + assert "untrusted-input" not in profile + assert len(findings) == 1 + f = findings[0] + assert isinstance(f, Finding) + assert f.kind == "trust-token-misplaced" + assert f.severity == "block" + assert f.tier == 1 + # suggested_fix must point the operator to the correct view + assert f.suggested_fix is not None + assert "implementing-agent" in f.suggested_fix + + +def test_misplaced_accessibility_required_in_implementing_agent_view_emits_block_finding(): + """accessibility-required in implementing-agent view → block finding; not in profile.""" + result = substrate_wizard._validate_view_trust_profile( + "implementing-agent", "accessibility-required" + ) + profile, findings = result + assert "accessibility-required" not in profile + assert len(findings) == 1 + f = findings[0] + assert f.kind == "trust-token-misplaced" + assert f.severity == "block" + # suggested_fix must reference the human-user view + assert f.suggested_fix is not None + assert "human-user" in f.suggested_fix + + +def test_two_misplaced_tokens_in_one_view_produce_two_block_findings(): + """Two misplaced tokens in one call → two separate block findings; neither in profile.""" + result = substrate_wizard._validate_view_trust_profile( + "operator", "untrusted-input,accessibility-required" + ) + profile, findings = result + assert "untrusted-input" not in profile + assert "accessibility-required" not in profile + assert len(findings) == 2 + for f in findings: + assert f.kind == "trust-token-misplaced" + assert f.severity == "block" + + +def test_correctly_placed_token_produces_no_finding_and_appears_in_profile(): + """untrusted-input in implementing-agent view → no findings; token IS in profile.""" + result = substrate_wizard._validate_view_trust_profile( + "implementing-agent", "untrusted-input" + ) + profile, findings = result + assert "untrusted-input" in profile + assert findings == [] + + +def test_mixed_valid_and_misplaced_tokens_partitions_correctly(): + """One valid + one misplaced → valid in profile, misplaced dropped, one finding.""" + result = substrate_wizard._validate_view_trust_profile( + "implementing-agent", "untrusted-input,accessibility-required" + ) + profile, findings = result + assert "untrusted-input" in profile + assert "accessibility-required" not in profile + assert len(findings) == 1 + assert findings[0].kind == "trust-token-misplaced" + + +def test_none_token_returns_empty_profile_and_no_findings(): + """'none' sentinel → empty profile and no findings (clean path).""" + result = substrate_wizard._validate_view_trust_profile( + "implementing-agent", "none" + ) + profile, findings = result + assert profile == [] + assert findings == [] + + +def test_empty_raw_returns_empty_profile_and_no_findings(): + """Empty string → empty profile, no findings.""" + result = substrate_wizard._validate_view_trust_profile( + "human-user", "" + ) + profile, findings = result + assert profile == [] + assert findings == [] diff --git a/tests/test_v1_review_followups.py b/tests/test_v1_review_followups.py index d0b9d47..bf90bdc 100644 --- a/tests/test_v1_review_followups.py +++ b/tests/test_v1_review_followups.py @@ -127,20 +127,24 @@ def test_empty_state_file_emits_recovery_hint(tmp_path): def test_trust_token_wrong_view_includes_disambiguation_hint(): """`untrusted-input` is valid for implementing-agent, not human-user. - The error message must say so explicitly.""" - with pytest.raises(substrate_wizard.WizardValidationError) as exc_info: - substrate_wizard._validate_view_trust_profile("human-user", "untrusted-input") - msg = str(exc_info.value) - assert "implementing-agent" in msg - assert "human-user" in msg + The finding's suggested_fix must reference the correct view.""" + result = substrate_wizard._validate_view_trust_profile("human-user", "untrusted-input") + profile, findings = result + assert "untrusted-input" not in profile + assert len(findings) == 1 + fix = findings[0].suggested_fix or "" + assert "implementing-agent" in fix def test_trust_token_genuine_typo_omits_hint(): - """Unknown token that doesn't exist in any view → no misleading hint.""" - with pytest.raises(substrate_wizard.WizardValidationError) as exc_info: - substrate_wizard._validate_view_trust_profile("human-user", "bogus-token") - msg = str(exc_info.value) - assert "Note:" not in msg + """Unknown token that doesn't exist in any view → block finding with valid-tokens fix.""" + result = substrate_wizard._validate_view_trust_profile("human-user", "bogus-token") + profile, findings = result + assert "bogus-token" not in profile + assert len(findings) == 1 + # No other-view hint; fix should list valid tokens for this view + fix = findings[0].suggested_fix or "" + assert "implementing-agent" not in fix # ── Issue #11: user-overlay shadowing surfaced via validate_catalog ──────────