From 592d94dfbb689247a5904d6a1327040fc9cf16ad Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Wed, 21 Oct 2026 20:40:39 +0530 Subject: [PATCH 1/2] feat(trust): compact workspace status output --- CHANGELOG.md | 5 + cli/bash/commands/basectl/README.md | 6 +- cli/bash/commands/basectl/tests/help.bats | 2 +- cli/python/base_trust/engine.py | 63 ++++++++-- .../tests/test_command_error_paths.py | 118 +++++++++++++++++- docs/command-reference.md | 2 +- docs/manifest-command-trust.md | 7 ++ 7 files changed, 189 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ebeb31f9..ea41dd33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ numeric next development line is tracked in `DEVELOPMENT_VERSION`. ## [Unreleased] +### Changed + +- Make project-less terminal `basectl trust status` output a compact workspace + table with blocked projects first, short reasons, and an aggregate summary. + ### Fixed - Use the same readable blocked-trust explanations in workspace test preflight, diff --git a/cli/bash/commands/basectl/README.md b/cli/bash/commands/basectl/README.md index 87cdc97a..2954d188 100644 --- a/cli/bash/commands/basectl/README.md +++ b/cli/bash/commands/basectl/README.md @@ -93,9 +93,9 @@ such command directories exist. Optional utility CLIs such as `caff` and history index and supports table, JSON, privacy-conscious reports, chronological ordering, and bounded time-window filters. - `basectl config path/show/doctor` inspects Base's machine-local user config at `~/.base.d/config.yaml`. -- `basectl trust status [project]` inspects one project's manifest command - trust or all discovered command-bearing projects. `basectl trust - allow/revoke ` manages local approval under +- `basectl trust status [project]` shows detailed trust guidance for one + project or a compact, blocked-first table for discovered command-bearing + projects. `basectl trust allow/revoke ` manages local approval under `~/.base.d/trust/manifest-commands/`. - `basectl doctor [project]` diagnoses the local Base environment and, when provided, project manifest artifacts with suggested fixes. diff --git a/cli/bash/commands/basectl/tests/help.bats b/cli/bash/commands/basectl/tests/help.bats index 1e8ccdf3..0484e09a 100644 --- a/cli/bash/commands/basectl/tests/help.bats +++ b/cli/bash/commands/basectl/tests/help.bats @@ -404,7 +404,7 @@ PY @test "command reference documents trust commands" { local command_reference="$BASE_REPO_ROOT/docs/command-reference.md" - grep -Fqx -- "| \`basectl trust status [project]\` | Show one project's manifest trust status, or all discovered command-bearing projects. | \`--project \`, \`--workspace \`, \`--format \` |" "$command_reference" + grep -Fqx -- "| \`basectl trust status [project]\` | Show detailed trust guidance for one project, or a compact blocked-first table for discovered command-bearing projects. | \`--project \`, \`--workspace \`, \`--format \` |" "$command_reference" grep -Fqx -- "| \`basectl trust allow \` | Approve the current manifest command contract on this machine. | \`--project \`, \`--workspace \`, \`--manifest-sha256 \`, \`--test-requirements-sha256 \` |" "$command_reference" grep -Fqx -- "| \`basectl trust revoke \` | Remove local manifest command approval. | \`--project \`, \`--workspace \` |" "$command_reference" } diff --git a/cli/python/base_trust/engine.py b/cli/python/base_trust/engine.py index f4259f64..47b51e28 100644 --- a/cli/python/base_trust/engine.py +++ b/cli/python/base_trust/engine.py @@ -204,7 +204,10 @@ class TrustError(RuntimeError): def workspace_status_command(ctx: base_cli.Context, workspace: str | None, output_format: str) -> int: try: - projects = workspace_status_projects(ctx, workspace) + from base_projects.workspace_context import resolve_workspace_root + + workspace_root = resolve_workspace_root(ctx, workspace) + projects = workspace_status_projects(ctx, workspace, workspace_root) store = ManifestCommandTrustStore() statuses = [] for project in projects: @@ -237,21 +240,65 @@ def workspace_status_command(ctx: base_cli.Context, workspace: str | None, outpu return base_cli.ExitCode.SUCCESS if not statuses: - print("No discovered projects require manifest command trust.") + _print_workspace_status_text(workspace_root, ()) return base_cli.ExitCode.SUCCESS - for index, (trust_status, surfaces) in enumerate(statuses): - if index: - print() - print_status_text(trust_status, surfaces) + _print_workspace_status_text(workspace_root, tuple(trust_status for trust_status, _surfaces in statuses)) return base_cli.ExitCode.SUCCESS -def workspace_status_projects(ctx: base_cli.Context, workspace: str | None) -> tuple[Project, ...]: +def _print_workspace_status_text(workspace_root: Path, statuses: tuple[TrustStatus, ...]) -> None: + project_count = len(statuses) + project_label = "project" if project_count == 1 else "projects" + print(f"Workspace trust status: {workspace_root} ({project_count} command-bearing {project_label})") + + if not statuses: + print("\nNo command-bearing projects require manifest command trust.") + print("Summary: 0 blocked, 0 allowed.") + return + + display_statuses = sorted( + statuses, + key=lambda trust_status: (trust_status.is_allowed, trust_status.identity.project_name.casefold()), + ) + project_width = max(len("PROJECT"), *(len(item.identity.project_name) for item in display_statuses)) + print() + print(f"{'PROJECT':<{project_width}} {'STATUS':<7} REASON") + for trust_status in display_statuses: + project_name = trust_status.identity.project_name + status_label = "Allowed" if trust_status.is_allowed else "Blocked" + reason_label = _workspace_status_reason_label(trust_status.reason) + print(f"{project_name:<{project_width}} {status_label:<7} {reason_label}") + + blocked_count = sum(1 for trust_status in statuses if not trust_status.is_allowed) + allowed_count = project_count - blocked_count + print(f"\nSummary: {blocked_count} blocked, {allowed_count} allowed.") + print("Details: basectl trust status ") + print("Trust scope: approval binds manifest and declared test-requirements digests.") + print(" Referenced scripts, direct executables, and Git HEAD are not covered.") + print(" Uncommitted working-tree changes are not checked.") + + +def _workspace_status_reason_label(reason: str) -> str: + labels = { + "allowed": "Approved", + "not_allowed": "Not approved", + "manifest_changed": "Manifest changed", + "test_requirements_changed": "Test requirements changed", + } + return labels.get(reason, reason.replace("_", " ").capitalize()) + + +def workspace_status_projects( + ctx: base_cli.Context, + workspace: str | None, + workspace_root: Path | None = None, +) -> tuple[Project, ...]: from base_projects.project_discovery import discover_projects_cached, read_project from base_projects.workspace_context import resolve_workspace_root - workspace_root = resolve_workspace_root(ctx, workspace) + if workspace_root is None: + workspace_root = resolve_workspace_root(ctx, workspace) projects_by_name = {project.name: project for project in discover_projects_cached(ctx, workspace_root)} if workspace is None: diff --git a/cli/python/base_trust/tests/test_command_error_paths.py b/cli/python/base_trust/tests/test_command_error_paths.py index 8c77e32d..270861c1 100644 --- a/cli/python/base_trust/tests/test_command_error_paths.py +++ b/cli/python/base_trust/tests/test_command_error_paths.py @@ -64,7 +64,123 @@ def test_workspace_status_supports_yaml_and_empty_terminal_output( assert yaml.safe_load(yaml_result.stdout)["projects"][0]["project"]["name"] == "demo" assert text_result.exit_code == 0, text_result.output - assert text_result.stdout == "No discovered projects require manifest command trust.\n" + assert text_result.stdout == ( + f"Workspace trust status: {empty_workspace.resolve()} (0 command-bearing projects)\n" + "\nNo command-bearing projects require manifest command trust.\n" + "Summary: 0 blocked, 0 allowed.\n" + ) + + +def test_workspace_status_text_shows_allowed_projects_compactly( + tmp_path: Path, + manifest_factory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + home = tmp_path / "home" + workspace = tmp_path / "work" + manifests = [ + manifest_factory.write(workspace / name, name=name) + for name in ("zeta", "alpha") + ] + store = engine.ManifestCommandTrustStore(home=home) + for manifest_path in manifests: + store.allow(engine.compute_trust_identity_for_manifest(manifest_path), base_version="1.10.5") + + monkeypatch.setattr(engine.base_cli, "is_terminal", lambda: True) + result = invoke( + engine.app, + ["status", "--workspace", str(workspace)], + home=home, + env={"BASE_HOME": str(workspace / "base")}, + ) + + assert result.exit_code == 0, result.output + assert f"Workspace trust status: {workspace.resolve()} (2 command-bearing projects)" in result.stdout + assert "PROJECT STATUS REASON" in result.stdout + assert "alpha Allowed Approved" in result.stdout + assert "zeta Allowed Approved" in result.stdout + assert "Summary: 0 blocked, 2 allowed." in result.stdout + assert result.stdout.count("Trust scope:") == 1 + assert "Details: basectl trust status " in result.stdout + assert "SHA-256" not in result.stdout + assert "Allow after review" not in result.stdout + + +def test_workspace_status_text_summarizes_blocked_reasons_before_allowed_projects( + tmp_path: Path, + manifest_factory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + home = tmp_path / "home" + workspace = tmp_path / "work" + allowed_manifest = manifest_factory.write(workspace / "allowed", name="allowed") + changed_manifest = manifest_factory.write(workspace / "changed-manifest", name="changed-manifest") + requirements_manifest = manifest_factory.write(workspace / "changed-requirements", name="changed-requirements") + requirements_manifest.write_text( + requirements_manifest.read_text(encoding="utf-8").replace( + " command: pytest tests/\n", + " command: pytest tests/\n requirements: requirements-dev.txt\n", + ), + encoding="utf-8", + ) + requirements_path = requirements_manifest.parent / "requirements-dev.txt" + requirements_path.write_text("jsonschema==4.25.1\n", encoding="utf-8") + + store = engine.ManifestCommandTrustStore(home=home) + store.allow(engine.compute_trust_identity_for_manifest(allowed_manifest), base_version="1.10.5") + store.allow(engine.compute_trust_identity_for_manifest(changed_manifest), base_version="1.10.5") + changed_manifest.write_text( + changed_manifest.read_text(encoding="utf-8").replace("pytest tests/", "pytest -q"), + encoding="utf-8", + ) + store.allow(engine.compute_trust_identity_for_manifest(requirements_manifest), base_version="1.10.5") + requirements_path.write_text("jsonschema==4.25.2\n", encoding="utf-8") + manifest_factory.write(workspace / "fresh", name="fresh") + + monkeypatch.setattr(engine.base_cli, "is_terminal", lambda: True) + result = invoke( + engine.app, + ["status", "--workspace", str(workspace)], + home=home, + env={"BASE_HOME": str(workspace / "base")}, + ) + + assert result.exit_code == 0, result.output + assert "changed-manifest Blocked Manifest changed" in result.stdout + assert "changed-requirements Blocked Test requirements changed" in result.stdout + assert "fresh Blocked Not approved" in result.stdout + assert "allowed Allowed Approved" in result.stdout + assert result.stdout.index("changed-manifest Blocked") < result.stdout.index("allowed Allowed") + assert "Summary: 3 blocked, 1 allowed." in result.stdout + assert result.stdout.count("Trust scope:") == 1 + assert "Details: basectl trust status " in result.stdout + assert "SHA-256" not in result.stdout + assert "Allow after review" not in result.stdout + + +def test_workspace_status_text_summarizes_all_blocked_projects( + tmp_path: Path, + manifest_factory, + monkeypatch: pytest.MonkeyPatch, +) -> None: + home = tmp_path / "home" + workspace = tmp_path / "work" + manifest_factory.write(workspace / "zeta", name="zeta") + manifest_factory.write(workspace / "alpha", name="alpha") + + monkeypatch.setattr(engine.base_cli, "is_terminal", lambda: True) + result = invoke( + engine.app, + ["status", "--workspace", str(workspace)], + home=home, + env={"BASE_HOME": str(workspace / "base")}, + ) + + assert result.exit_code == 0, result.output + assert "alpha Blocked Not approved" in result.stdout + assert "zeta Blocked Not approved" in result.stdout + assert "Summary: 2 blocked, 0 allowed." in result.stdout + assert result.stdout.count("Trust scope:") == 1 def test_invalid_status_format_is_a_usage_error(tmp_path: Path) -> None: diff --git a/docs/command-reference.md b/docs/command-reference.md index e10b9729..d238f9eb 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -146,7 +146,7 @@ is requested on macOS, Base warns if `osascript` is not available. | `basectl demo [project]` | Run a project-owned demo script. | `--project `, `--workspace `, `--dry-run`, `-- ` | | `basectl devcontainer [project]` | Preview or write `.devcontainer/devcontainer.json` from a Base manifest. Dry-run is the default. | `--project `, `--workspace `, `--format `, `--write` | | `basectl devenv-report [project]` | Classify Base manifest fields for Nix/devenv planning without generating files or requiring Nix. | `--project `, `--workspace `, `--format ` | -| `basectl trust status [project]` | Show one project's manifest trust status, or all discovered command-bearing projects. | `--project `, `--workspace `, `--format ` | +| `basectl trust status [project]` | Show detailed trust guidance for one project, or a compact blocked-first table for discovered command-bearing projects. | `--project `, `--workspace `, `--format ` | | `basectl trust allow ` | Approve the current manifest command contract on this machine. | `--project `, `--workspace `, `--manifest-sha256 `, `--test-requirements-sha256 ` | | `basectl trust revoke ` | Remove local manifest command approval. | `--project `, `--workspace ` | diff --git a/docs/manifest-command-trust.md b/docs/manifest-command-trust.md index 88befa94..29ef9917 100644 --- a/docs/manifest-command-trust.md +++ b/docs/manifest-command-trust.md @@ -170,6 +170,13 @@ reports every discovered project whose manifest declares an executable test, run, build, demo, or activation surface. Metadata-only manifests are omitted so workspace onboarding does not suggest unnecessary approval. +In the default terminal text view without a project, `trust status` prints a +compact workspace table with blocked projects first, a short reason for each +status, and aggregate allowed/blocked counts. It states the trust-scope limit +once and points to `basectl trust status ` for the detailed identity, +review, and allow guidance. An explicit project keeps the detailed text report; +JSON/YAML/CSV/TSV workspace output keeps its existing schema and columns. + For schema-v1 compatibility, explicit single-project JSON continues to report the underlying allow-record state, including `status: blocked` and an `allow_command` for a metadata-only manifest. Human text reports that approval From 1664138a27aebe41093b3d047bf19a59604d86d1 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:17:52 +0530 Subject: [PATCH 2/2] fix(trust): address workspace status review findings --- .ai-context/COMMANDS.md | 5 +- cli/python/base_trust/engine.py | 46 +++++++++------ .../tests/test_command_error_paths.py | 58 +++++++++++++++++-- docs/basectl-onboard.md | 10 ++-- 4 files changed, 91 insertions(+), 28 deletions(-) diff --git a/.ai-context/COMMANDS.md b/.ai-context/COMMANDS.md index aebf7757..2b85392f 100644 --- a/.ai-context/COMMANDS.md +++ b/.ai-context/COMMANDS.md @@ -53,7 +53,10 @@ the v1.x compatibility window; new automation should use `--debug-wrapper`. `--output` explicitly selects another destination, including project directories. `--print` and `--list-files` do not write a bundle. - `basectl trust status [project]` - inspect one project's manifest command - trust or all discovered command-bearing projects. + trust or all discovered command-bearing projects. Its terminal workspace + view is a compact, blocked-first summary; use + `basectl trust status ` for identity digests and review/allow + guidance. - Generated onboarding trust commands include the canonical workspace and manifest digest; they retain their target when copied to another directory. - `basectl trust ` - add or remove local approval for diff --git a/cli/python/base_trust/engine.py b/cli/python/base_trust/engine.py index 47b51e28..b4a29404 100644 --- a/cli/python/base_trust/engine.py +++ b/cli/python/base_trust/engine.py @@ -239,10 +239,6 @@ def workspace_status_command(ctx: base_cli.Context, workspace: str | None, outpu ) return base_cli.ExitCode.SUCCESS - if not statuses: - _print_workspace_status_text(workspace_root, ()) - return base_cli.ExitCode.SUCCESS - _print_workspace_status_text(workspace_root, tuple(trust_status for trust_status, _surfaces in statuses)) return base_cli.ExitCode.SUCCESS @@ -261,22 +257,32 @@ def _print_workspace_status_text(workspace_root: Path, statuses: tuple[TrustStat statuses, key=lambda trust_status: (trust_status.is_allowed, trust_status.identity.project_name.casefold()), ) - project_width = max(len("PROJECT"), *(len(item.identity.project_name) for item in display_statuses)) - print() - print(f"{'PROJECT':<{project_width}} {'STATUS':<7} REASON") - for trust_status in display_statuses: - project_name = trust_status.identity.project_name - status_label = "Allowed" if trust_status.is_allowed else "Blocked" - reason_label = _workspace_status_reason_label(trust_status.reason) - print(f"{project_name:<{project_width}} {status_label:<7} {reason_label}") - + records = [ + { + "project": trust_status.identity.project_name, + "status": "Allowed" if trust_status.is_allowed else "Blocked", + "reason": _workspace_status_reason_label(trust_status.reason), + } + for trust_status in display_statuses + ] blocked_count = sum(1 for trust_status in statuses if not trust_status.is_allowed) allowed_count = project_count - blocked_count - print(f"\nSummary: {blocked_count} blocked, {allowed_count} allowed.") - print("Details: basectl trust status ") - print("Trust scope: approval binds manifest and declared test-requirements digests.") - print(" Referenced scripts, direct executables, and Git HEAD are not covered.") - print(" Uncommitted working-tree changes are not checked.") + footer = "\n".join( + ( + f"Summary: {blocked_count} blocked, {allowed_count} allowed.", + "Details: basectl trust status ", + "Trust scope: approval binds manifest and declared test-requirements digests.", + " Referenced scripts, direct executables, and Git HEAD are not covered.", + " Uncommitted working-tree changes are not checked.", + ) + ) + print() + base_cli.render_records( + records, + requested_format="text", + columns=(("PROJECT", "project"), ("STATUS", "status"), ("REASON", "reason")), + footer=footer, + ) def _workspace_status_reason_label(reason: str) -> str: @@ -286,7 +292,9 @@ def _workspace_status_reason_label(reason: str) -> str: "manifest_changed": "Manifest changed", "test_requirements_changed": "Test requirements changed", } - return labels.get(reason, reason.replace("_", " ").capitalize()) + if reason in labels: + return labels[reason] + return " ".join(word[:1].upper() + word[1:] for word in reason.replace("_", " ").split()) def workspace_status_projects( diff --git a/cli/python/base_trust/tests/test_command_error_paths.py b/cli/python/base_trust/tests/test_command_error_paths.py index 270861c1..ce139860 100644 --- a/cli/python/base_trust/tests/test_command_error_paths.py +++ b/cli/python/base_trust/tests/test_command_error_paths.py @@ -1,15 +1,22 @@ from __future__ import annotations import json +import unicodedata from pathlib import Path import pytest import yaml +from base_cli import output as base_cli_output from base_cli.testing import invoke from base_trust import engine +def _force_terminal_output(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(engine.base_cli, "is_terminal", lambda: True) + monkeypatch.setattr(base_cli_output, "is_terminal", lambda *_args, **_kwargs: True) + + @pytest.mark.parametrize("output_format", ["yaml", "csv", "tsv"]) def test_project_status_supports_documented_output_formats( tmp_path: Path, @@ -54,7 +61,7 @@ def test_workspace_status_supports_yaml_and_empty_terminal_output( empty_workspace = tmp_path / "empty" empty_workspace.mkdir() - monkeypatch.setattr(engine.base_cli, "is_terminal", lambda: True) + _force_terminal_output(monkeypatch) text_result = invoke( engine.app, ["status", "--workspace", str(empty_workspace)], @@ -86,7 +93,7 @@ def test_workspace_status_text_shows_allowed_projects_compactly( for manifest_path in manifests: store.allow(engine.compute_trust_identity_for_manifest(manifest_path), base_version="1.10.5") - monkeypatch.setattr(engine.base_cli, "is_terminal", lambda: True) + _force_terminal_output(monkeypatch) result = invoke( engine.app, ["status", "--workspace", str(workspace)], @@ -137,7 +144,7 @@ def test_workspace_status_text_summarizes_blocked_reasons_before_allowed_project requirements_path.write_text("jsonschema==4.25.2\n", encoding="utf-8") manifest_factory.write(workspace / "fresh", name="fresh") - monkeypatch.setattr(engine.base_cli, "is_terminal", lambda: True) + _force_terminal_output(monkeypatch) result = invoke( engine.app, ["status", "--workspace", str(workspace)], @@ -168,7 +175,7 @@ def test_workspace_status_text_summarizes_all_blocked_projects( manifest_factory.write(workspace / "zeta", name="zeta") manifest_factory.write(workspace / "alpha", name="alpha") - monkeypatch.setattr(engine.base_cli, "is_terminal", lambda: True) + _force_terminal_output(monkeypatch) result = invoke( engine.app, ["status", "--workspace", str(workspace)], @@ -183,6 +190,49 @@ def test_workspace_status_text_summarizes_all_blocked_projects( assert result.stdout.count("Trust scope:") == 1 +def test_workspace_status_text_uses_unicode_aware_truncating_table( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + _force_terminal_output(monkeypatch) + monkeypatch.setattr(base_cli_output, "_terminal_width", lambda _stream: 40) + project_names = ("a", "界" * 40) + statuses = tuple( + engine.TrustStatus( + status="blocked", + reason="not_allowed", + identity=engine.ManifestCommandTrustIdentity( + project_name=name, + project_root=tmp_path, + manifest_path=tmp_path / "base_manifest.yaml", + manifest_sha256="a" * 64, + identity_key=name, + ), + ) + for name in project_names + ) + + engine._print_workspace_status_text(tmp_path, statuses) # pylint: disable=protected-access + + output = capsys.readouterr().out + rows = [line for line in output.splitlines() if "Blocked" in line] + assert len(rows) == 2 + assert any("界" in line and "…" in line for line in rows) + + def display_width(value: str) -> int: + return sum( + 0 if unicodedata.combining(char) else 2 if unicodedata.east_asian_width(char) in {"W", "F"} else 1 + for char in value + ) + + assert len({display_width(line.split("Blocked", maxsplit=1)[0]) for line in rows}) == 1 + + +def test_workspace_status_reason_fallback_preserves_acronyms() -> None: + assert engine._workspace_status_reason_label("remote_HTTP_status") == "Remote HTTP Status" # pylint: disable=protected-access + + def test_invalid_status_format_is_a_usage_error(tmp_path: Path) -> None: result = invoke( engine.app, diff --git a/docs/basectl-onboard.md b/docs/basectl-onboard.md index 447a57da..16a4b95f 100644 --- a/docs/basectl-onboard.md +++ b/docs/basectl-onboard.md @@ -107,8 +107,9 @@ boundary. profiles - `basectl update-profile` for shell startup integration - `basectl projects list` to show discovered projects after setup -- `basectl trust status` to show digest-bound command trust for discovered - projects without granting approval +- `basectl trust status` to show read-only trust status for discovered + command-bearing projects without granting approval; use + `basectl trust status ` for detailed digest and review guidance - `basectl activate ` as the final suggested next step It does not duplicate Homebrew, Python, venv, manifest, or shell-profile logic. @@ -141,8 +142,9 @@ The shipped flow is simple and checklist-oriented: 8. Run `basectl doctor` to summarize the final state. 9. Print discovered projects with `basectl projects list` when available. 10. Run the read-only `basectl trust status` workspace view, which filters out - manifests without executable command surfaces and prints exact review and - digest-bound allow guidance for those that need it. + manifests without executable command surfaces and prints a compact, + blocked-first summary. Use `basectl trust status ` for exact + identity digests and digest-bound review and allow guidance. 11. Suggest `basectl` or `basectl activate base` after any required trust review.