Skip to content

feat(client): display and gate on provisional evidence tier (P6) - #115

Merged
tonythethompson merged 2 commits into
masterfrom
feat/p6-provisional-tier
Aug 10, 2026
Merged

feat(client): display and gate on provisional evidence tier (P6)#115
tonythethompson merged 2 commits into
masterfrom
feat/p6-provisional-tier

Conversation

@tonythethompson

@tonythethompson tonythethompson commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Implements the client side of P6 (Provisional Tier) from the intake process reform ADR.
  • VersionEntry gains optional evidence_tier: Option<String> and deferral_reason: Option<String> (both #[serde(default)], backward compatible), plus an is_provisional() helper (evidence_tier == Some("provisional"); absent means proven).
  • numan info: shows evidence: provisional — <reason> under a provisional version (reason not recorded if deferral_reason is empty/absent).
  • numan search: appends a [provisional] marker to the displayed version's row (extracted as a small pure provisional_marker() helper for testability, mirroring format_row_status).
  • numan install: prints a one-time notice ("This package has not been lifecycle-tested. It passed integrity checks.") right after a provisional package installs successfully — not on the already-installed no-op short-circuit.

Test plan

  • cargo build — clean
  • cargo test — 668+ passed across lib and all integration binaries, 0 failed
  • cargo fmt --check — clean
  • New tests: VersionEntry::is_provisional() (absent/proven/provisional), numan info evidence line (with reason / without reason / omitted when proven), provisional_marker() (Some-provisional / Some-proven / None)

Not separately tested: the numan install println itself (a straightforward guard on the already-tested is_provisional() predicate; the codebase doesn't capture stdout for install_package's other status lines either).

Ref: docs/plans/2026-08-09-intake-process-reform.md (P6, in numan-registry/ADR PR #113 context).

🤖 Generated with Claude Code

Review in cubic

VersionEntry gains optional evidence_tier/deferral_reason, parsed from
the registry index (absence means proven, per the two-tier model).

- numan info: shows "evidence: provisional — <reason>" under a
  provisional version (falls back to "reason not recorded" if
  deferral_reason is empty/missing).
- numan search: appends a " [provisional]" marker to the displayed
  version's row.
- numan install: prints a one-time notice ("has not been
  lifecycle-tested... passed integrity checks") right after a
  provisional package installs successfully (not on the
  already-installed no-op path).

Ref: docs/plans/2026-08-09-intake-process-reform.md (P6)
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 90a6ef3e-78db-4fff-b453-abef457d0461

📥 Commits

Reviewing files that changed from the base of the PR and between 6d3894d and e3b5675.

📒 Files selected for processing (9)
  • src/cmd/info.rs
  • src/cmd/search.rs
  • src/cmd/try_cmd.rs
  • src/core/package.rs
  • src/core/registry.rs
  • src/core/resolve.rs
  • src/install/transaction.rs
  • tests/install_test.rs
  • tests/support/active_update/mod.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • tonythethompson/QuickShell (manual)
  • tonythethompson/numan (manual)
  • tonythethompson/dependency-chain-substrate (manual)
📜 Recent review details
⏰ Context from checks skipped due to timeout. (7)
  • GitHub Check: Greptile Review
  • GitHub Check: Real-Nu acceptance (macos-latest)
  • GitHub Check: Test (ubuntu-latest)
  • GitHub Check: Real-Nu acceptance (ubuntu-latest)
  • GitHub Check: Real-Nu acceptance (windows-latest)
  • GitHub Check: Test (windows-latest)
  • GitHub Check: Analyze (rust)
🧰 Additional context used
📓 Path-based instructions (13)
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: Use Serena's semantic, symbol-aware tools as the primary tools for reading and editing code; use built-in Read, Glob, Grep, and Edit only under the stated exceptions.
Before editing a code file, inspect its symbol overview, read the specific symbols being changed, and edit them with Serena's symbol-aware tools.
Understand the existing code before changing it and make the smallest change that satisfies the request; avoid unrelated cleanup, premature abstractions, unnecessary error handling, feature flags, and compatibility shims.
Prefer editing existing files over creating new ones, and never create Markdown or README files unless explicitly requested.
For exploratory questions, provide a 2–3 sentence recommendation with the main tradeoff and do not implement until the user agrees.
For UI or frontend changes that cannot be tested in a browser, explicitly state that browser testing was not performed rather than claiming success.
Address security issues when discovered, including injection, XSS, SQL injection, path traversal, and secret leaks.
Pause and obtain confirmation before destructive, hard-to-reverse, externally visible, or third-party-upload actions, including deleting files or branches, dropping tables, force-pushing, modifying CI/CD, posting externally, or uploading content.
When blocked, investigate the root cause instead of bypassing it with --no-verify, --force, or deletion; investigate unfamiliar files, branches, and configuration before deleting them.
Only commit when explicitly asked; do not proactively update git configuration or push changes.
Do not skip Git hooks unless explicitly asked; if a pre-commit hook fails, fix the issue, re-stage, and create a new commit rather than amending.
Stage files by name rather than using git add -A or git add ., to avoid accidentally including secrets or large binaries.
Use a HEREDOC for commit messages, and add co-author attribution only when the user explicitly requests the exact trailer.
Do not force-pus...

Files:

  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/info.rs
  • src/cmd/search.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/core/package.rs
  • src/install/transaction.rs
**/*.{js,jsx,ts,tsx,py,java,go,rs,rb,php,c,cpp,h,hpp,cs,swift,kt,kts}

📄 CodeRabbit inference engine (CLAUDE.md)

Add comments only when the WHY is non-obvious; do not narrate what the code does, reference the current task, or reference the PR in comments.

Files:

  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/info.rs
  • src/cmd/search.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/core/package.rs
  • src/install/transaction.rs
!**/.env,!**/credentials.json,!**/*.pem

📄 CodeRabbit inference engine (CLAUDE.md)

Do not commit files that appear to contain secrets, including .env, credentials.json, and PEM files; warn before doing so even if explicitly requested.

Files:

  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/info.rs
  • src/cmd/search.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/core/package.rs
  • src/install/transaction.rs
**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.rs: Use the Rust 2021 edition.
Use anyhow::Result with .context(...) in application code; use thiserror for library error types that callers match on.
Use clap derive macros for CLI definitions.
Use serde with serde_json or toml for serialization.
Function parameters must use &Path, not &PathBuf.
Library code must not panic; error paths should return anyhow::Result with context where appropriate.
Add or update tests for behavior changes, including relevant failure paths.
New mutating code paths must acquire the mutation lock via acquire_mutation_lock(root) and snapshot the lockfile before writes.
Lockfile, journal, and state-file JSON writes must use write_json_atomic.
numan install must write only to $NUMAN_ROOT; it must not invoke Nu or register plugins/autoloads.
Only activate and deactivate may modify Nu integration state.
Treat the lockfile as the authoritative source of truth; derived projections such as autoload state must not be authoritative.
Install payloads under versioned, content-addressed paths and never overwrite them in place.
Never overwrite foreign autoload files; respect OWNERSHIP_MARKER.
Pass plugin paths through environment variables only; do not use runtime interpolation in Nu program strings.

**/*.rs: All CI gates must pass: cargo test, cargo clippy -- -D warnings, and cargo fmt --check.
Every mutating command—including install, remove, update, gc, and future nupm import—must call acquire_mutation_lock(root).
Lockfiles, journals, and state files must use write_json_atomic; partial writes are not allowed.
Pending activation, autoload, and lifecycle journals must be stored under $NUMAN_ROOT/state/.
Module autoload identity must match all four fields: Nu executable hash, Nu version, vendor autoload directory, and managed file path; the lockfile module_activation value is authoritative.
Never overwrite foreign autoload files; respect OWNERSHIP_MARKER.
Pass paths to Nu only throu...

Files:

  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/info.rs
  • src/cmd/search.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/core/package.rs
  • src/install/transaction.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run and keep cargo fmt/rustfmt clean, and ensure cargo clippy -- -D warnings passes.

Files:

  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/info.rs
  • src/cmd/search.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/core/package.rs
  • src/install/transaction.rs
**/*.{rs,nu}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{rs,nu}: Real-Nu acceptance tests must be marked #[ignore] and should be run when changes affect activation or nupm import; unit tests must not spawn real nu and should use injectable seams such as FakeCandidateRunner or registrars.
The nupm integration must be read-only toward NUPM_HOME, must not execute build.nu, and must not perform bidirectional synchronization.

Unit tests must use FakeCandidateRunner or injectable registrars and must not spawn a real nu process.

Files:

  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/info.rs
  • src/cmd/search.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/core/package.rs
  • src/install/transaction.rs
**/*.{rs,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Match existing naming, module layout, and documentation level in the file being edited; update AGENTS.md, docs/, or command help when structure, conventions, or user-visible behavior changes.

Tests must cover failure modes, not only successful execution.

Files:

  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/info.rs
  • src/cmd/search.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/core/package.rs
  • src/install/transaction.rs
**/*.{rs,md,toml}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use the repository's established serialization and module conventions rather than introducing unrelated refactors.

Files:

  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/info.rs
  • src/cmd/search.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/core/package.rs
  • src/install/transaction.rs
src/cmd/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Pass Nu paths and names through environment variables (NUMAN_PLUGIN_BINARY, NUMAN_PLUGIN_CONFIG, and NUMAN_PLUGIN_NAME); the Nu program string must be a compile-time constant with no runtime interpolation.

Files:

  • src/cmd/try_cmd.rs
  • src/cmd/info.rs
  • src/cmd/search.rs
src/core/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Derive the platform triple from compile-time #[cfg(target_env)] values, not std::env::consts; LIBC must be a compile-time constant.

Files:

  • src/core/resolve.rs
  • src/core/registry.rs
  • src/core/package.rs
tests/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Test platform-specific behavior with mock platforms, and use injected fakes rather than real Nu for unit tests.

Files:

  • tests/support/active_update/mod.rs
  • tests/install_test.rs
src/core/{trust.rs,official_registry.rs,registry.rs}

📄 CodeRabbit inference engine (AGENTS.md)

Verify Ed25519 signatures over the exact index.json bytes; use the built-in production trust root for the official registry. Unsigned bypass is allowed only with NUMAN_ALLOW_UNSIGNED=1 for development.

Files:

  • src/core/registry.rs
src/install/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

src/install/**/*.rs: Plugin binary artifacts must include a SHA256; the install transaction must fail when sha256 is missing.
Installation must remain inert: it may write only to $NUMAN_ROOT and must not perform Nu integration.

Files:

  • src/install/transaction.rs
🔍 Remote MCP DeepWiki, GitHub Copilot

Additional review context

  • VersionEntry is deserialized as part of signed RegistryIndex loading; info, search, and install all consume the resolved registry entries. This makes the new fields part of the registry/client compatibility contract.
  • The project’s P6 ADR specifies:
    • Missing evidence_tier means proven for backward compatibility.
    • "provisional" entries should carry deferral_reason, but missing/empty reasons are explicitly expected to display as "reason not recorded".
    • Signed indexes and SHA-256 integrity guarantees remain unchanged.
  • The actual diff confirms unknown evidence-tier strings fail deserialization, while absent fields default safely; provisional status is displayed in info, marked in search, and warned after a successful install.
  • Current PR checks include successful formatting, Clippy, MSRV, package, CodeQL language analyses, and several tests; Ubuntu/Windows tests, real-Nu acceptance, Rust analysis, and Greptile review were still in progress when queried.
🔇 Additional comments (10)
tests/install_test.rs (2)

140-142: Add coverage for provisional install reporting.

This fixture only tests a proven entry. Add coverage for a newly installed provisional entry and the already-installed no-op path. Assert that the warning includes the sanitized fallback reason and is emitted only after a successful new installation.

As per coding guidelines, “Add or update tests for behavior changes, including relevant failure paths.”

Source: Coding guidelines


387-389: LGTM!

Also applies to: 472-474, 545-547

src/install/transaction.rs (1)

411-416: Verify cache hits before reporting integrity success.

A cache hit bypasses integrity::verify_and_report, but this warning states that integrity checks passed. Verify cache_file against artifact_sha256 before extraction on every install path. Print this warning only after that verification succeeds.

src/core/package.rs (1)

115-162: LGTM!

Also applies to: 320-412

src/core/registry.rs (1)

409-411: LGTM!

src/core/resolve.rs (1)

729-731: LGTM!

Also applies to: 749-751, 1064-1066

src/cmd/try_cmd.rs (1)

471-473: LGTM!

src/cmd/info.rs (1)

82-89: LGTM!

Also applies to: 173-175, 236-270

src/cmd/search.rs (1)

270-272: LGTM!

Also applies to: 358-374

tests/support/active_update/mod.rs (1)

509-511: LGTM!


📝 Walkthrough

Walkthrough

Changes

Provisional Version Evidence

Layer / File(s) Summary
Evidence metadata contract
src/core/package.rs, src/core/registry.rs, src/core/resolve.rs, src/cmd/try_cmd.rs, tests/install_test.rs, tests/support/active_update/mod.rs
VersionEntry now stores typed optional evidence tier and deferral reason metadata. is_provisional() identifies provisional entries. Deferral reasons use sanitized fallback text. Fixtures and deserialization tests cover the new fields.
CLI evidence display
src/cmd/info.rs, src/cmd/search.rs
info reports provisional evidence and deferral reasons. Search results append [provisional]. Tests cover provisional, proven, absent, missing, and blank metadata.
Installation evidence reporting
src/install/transaction.rs
Successful installation reports provisional packages with a warning, integrity confirmation, and a recorded or fallback deferral reason.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 7 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 60.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the client-side provisional evidence tier changes described in the pull request.
Description check ✅ Passed The description directly explains the provisional tier implementation, affected commands, compatibility, tests, and known scope limits.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Pipeline Stage Enum Ordering ✅ Passed The patch does not modify or reference SessionWorkflowStage or its members; repository-wide search found no target enum or comparisons, so ordering and legacy-mapping checks are not applicable.
Gpu/Cpu Runtime Boundary ✅ Passed The PR changes only Rust CLI/core/install and test files. No inference/, requirements, main.py, or C# provider/registry files are modified, so this boundary check is not applicable.
Managed Host Restart Safety ✅ Passed The PR changes only numan registry metadata, CLI output, completions, and package installation; no managed-host, container readiness, lease, health, stop, or restart code exists or changed.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/p6-provisional-tier
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feat/p6-provisional-tier

Warning

Review ran into problems

🔥 Problems

Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. Analyzed tonythethompson/QuickShell, tonythethompson/numan, tonythethompson/dependency-chain-substrate, skipped Trackdubllc/Trackdub.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@tonythethompson
tonythethompson marked this pull request as ready for review August 10, 2026 08:01

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @tonythethompson, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Add provisional evidence tier support (P6) to info/search/install

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Parse optional evidence tier + deferral reason on registry version entries (backward compatible).
• Surface provisional status in numan info and numan search output.
• Emit a post-install warning when installing a provisional package successfully.
Diagram

graph TD
  R[(Registry index JSON)] --> VE["VersionEntry (evidence_tier, deferral_reason)"] --> H{"is_provisional()?"}
  H --> INFO["cmd/info: evidence line"] --> OUT["CLI output"]
  H --> SEARCH["cmd/search: [provisional] marker"] --> OUT
  H --> INSTALL["install/transaction: post-install notice"] --> OUT

  subgraph Legend
    direction LR
    _db[(Data source)] ~~~ _core["Core model"] ~~~ _dec{"Decision"} ~~~ _cmd["CLI command"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Typed evidence tier enum (with Unknown)
  • ➕ Prevents typos like "provisonal" silently disabling gating
  • ➕ Centralizes tier semantics (e.g., future tiers beyond provisional/proven)
  • ➕ Enables exhaustiveness checking when behavior differs by tier
  • ➖ Slightly more code/ceremony; requires serde mapping and possibly a custom deserializer for forward compatibility
  • ➖ May require deciding how to handle unknown tiers (warn vs treat as proven)
2. Single structured evidence object (e.g., `evidence: { tier, deferral_reason }`)
  • ➕ Avoids invalid states (e.g., deferral_reason without tier)
  • ➕ Easier to extend with future fields (links to evidence, timestamps, etc.)
  • ➖ Registry schema change is larger; would require coordinating client + registry producers
  • ➖ More migration considerations for existing index entries

Recommendation: The current approach (optional evidence_tier/deferral_reason strings with #[serde(default)] and an is_provisional() helper) is a pragmatic, backward-compatible first step for P6. If additional tiers are expected soon, consider upgrading to a typed enum (keeping forward compatibility via an Unknown(String) variant) to reduce accidental string drift and to make tier-specific behavior explicit.

Files changed (9) +164 / -3

Enhancement (4) +144 / -3
info.rsShow evidence tier line for provisional versions +37/-0

Show evidence tier line for provisional versions

• Adds an 'evidence: provisional — <reason>' line in 'format_info' when the displayed version is provisional, falling back to "reason not recorded". Extends unit tests to cover provisional-with-reason, provisional-without-reason, and proven (no evidence line) cases.

src/cmd/info.rs

search.rsAppend '[provisional]' marker in search results +40/-3

Append '[provisional]' marker in search results

• Introduces a small 'provisional_marker()' helper that returns a row suffix for provisional displayed versions and wires it into the printed search row. Adds unit tests for provisional/proven/None cases and updates test fixtures for the expanded 'VersionEntry' struct.

src/cmd/search.rs

package.rsAdd evidence tier fields + 'is_provisional()' to VersionEntry +56/-0

Add evidence tier fields + 'is_provisional()' to VersionEntry

• Extends 'VersionEntry' with 'evidence_tier' and 'deferral_reason', both '#[serde(default)]' for backward-compatible deserialization. Adds an 'is_provisional()' helper and new serde-based unit tests covering absent tier, provisional tier, and explicit proven tier.

src/core/package.rs

transaction.rsWarn after successful install of provisional packages +11/-0

Warn after successful install of provisional packages

• After a successful install (and not on the already-installed short-circuit path), prints a one-time warning for provisional packages indicating they are not lifecycle-tested and includes the deferral reason or a default message.

src/install/transaction.rs

Tests (5) +20 / -0
try_cmd.rsUpdate command test fixtures for new VersionEntry fields +2/-0

Update command test fixtures for new VersionEntry fields

• Extends in-file test 'VersionEntry' constructors with 'evidence_tier' and 'deferral_reason' fields set to 'None' to keep tests compiling after the model change.

src/cmd/try_cmd.rs

registry.rsAdjust registry tests for VersionEntry schema expansion +2/-0

Adjust registry tests for VersionEntry schema expansion

• Updates 'VersionEntry' test instances to include 'evidence_tier'/'deferral_reason' as 'None', reflecting the new struct fields without altering behavior.

src/core/registry.rs

resolve.rsAdjust resolver tests for VersionEntry schema expansion +6/-0

Adjust resolver tests for VersionEntry schema expansion

• Updates resolver test fixtures to populate 'evidence_tier' and 'deferral_reason' with 'None' for all constructed 'VersionEntry' values.

src/core/resolve.rs

install_test.rsUpdate install integration fixtures for new VersionEntry fields +8/-0

Update install integration fixtures for new VersionEntry fields

• Extends integration test package/version fixtures with 'evidence_tier: None' and 'deferral_reason: None' to match the updated 'VersionEntry' struct.

tests/install_test.rs

mod.rsUpdate active-update registry fixture for new VersionEntry fields +2/-0

Update active-update registry fixture for new VersionEntry fields

• Updates the helper that writes a dual-version registry to include the new 'VersionEntry' fields (set to 'None') in generated fixtures.

tests/support/active_update/mod.rs

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR adds typed provisional-evidence metadata to registry versions and consistently exposes it in package information, search results, and successful installation output.

  • Adds a closed EvidenceTier model with backward-compatible defaults.
  • Sanitizes missing, blank, and control-character-containing deferral reasons for terminal display.
  • Marks provisional versions in numan info and numan search.
  • Prints a provisional-evidence notice after successful installation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/core/package.rs Adds typed evidence tiers and a sanitized deferral-reason formatter; the previous blank-reason issue is fixed for missing, empty, whitespace-only, and control-character-containing values.
src/cmd/info.rs Displays provisional evidence using the shared sanitized fallback, completing the fix requested by the previous review thread.
src/cmd/search.rs Adds a provisional marker derived from the same version entry used for the displayed version label.
src/install/transaction.rs Prints the sanitized provisional notice after a successful installation while preserving the intentional already-installed short circuit.

Reviews (2): Last reviewed commit: "Merge branch 'master' into feat/p6-provi..." | Re-trigger Greptile

Comment thread src/cmd/info.rs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d3894dd88

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/core/package.rs Outdated
Comment thread src/cmd/info.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cmd/search.rs`:
- Line 151: Remove the narrative comment above the row-suffix definition in the
search command; keep the suffix implementation and surrounding behavior
unchanged.

In `@src/core/package.rs`:
- Around line 115-130: The VersionEntry evidence_tier field currently accepts
arbitrary strings and silently treats unsupported values as proven. Introduce an
EvidenceTier enum with serde representations for “proven” and “provisional”,
change VersionEntry.evidence_tier to Option<EvidenceTier>, and update
is_provisional() accordingly while preserving None as the legacy proven value.
Add a deserialization test confirming an unsupported tier is rejected.

In `@src/install/transaction.rs`:
- Around line 411-419: Ensure every install path, including cache hits, verifies
cache_file against artifact_sha256 before extraction by routing through the
existing integrity verification flow or equivalent comparison. Only print the
lifecycle-tested warning in the resolved.is_provisional() block after
verification succeeds, and prevent corrupted artifacts from reaching extraction.

In `@tests/install_test.rs`:
- Around line 140-141: Extend the install integration tests around the
evidence_tier and deferral_reason fixture to cover a newly installed provisional
entry and the already-installed no-op path. Assert the provisional warning
includes the fallback reason and is emitted only after a successful new
installation, while the no-op path produces no warning.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6aa9f7da-5349-4e84-9c86-ecb9149685bd

📥 Commits

Reviewing files that changed from the base of the PR and between 2c6bb5e and 6d3894d.

📒 Files selected for processing (9)
  • src/cmd/info.rs
  • src/cmd/search.rs
  • src/cmd/try_cmd.rs
  • src/core/package.rs
  • src/core/registry.rs
  • src/core/resolve.rs
  • src/install/transaction.rs
  • tests/install_test.rs
  • tests/support/active_update/mod.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • tonythethompson/QuickShell (manual)
  • tonythethompson/numan (manual)
  • tonythethompson/dependency-chain-substrate (manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Greptile Review
⚠️ CI failures not shown inline (2)

GitHub Actions: Trigger Kiro Crew / 0_trigger _ trigger-agent.txt: feat(client): display and gate on provisional evidence tier (P6)

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m: "${***REDACTED_SECRET_ASSIGNMENT*** missing}"�[0m
 �[36;1m: "${***REDACTED_SECRET_ASSIGNMENT*** missing}"�[0m
 �[36;1m: "${GATEWAY_URL:?KIRO_GATEWAY_URL missing}"�[0m
 �[36;1m�[0m
 �[36;1mif [ -n "${GATEWAY_ORIGIN:-}" ]; then�[0m
 �[36;1m  ORIGIN="$GATEWAY_ORIGIN"�[0m
 �[36;1melse�[0m
 �[36;1m  ORIGIN=$(printf '%s' "$GATEWAY_URL" | sed -E 's#(https?://[^/]+).*#\1#')�[0m
 �[36;1mfi�[0m
 �[36;1mREPO_SHORT="${REPO##*/}"�[0m
 �[36;1mSESSION_KEY="hook:github:${REPO_SHORT}:${EVENT}:${RUN_ID}"�[0m
 �[36;1m�[0m
 �[36;1mONE_OFF=""�[0m
 �[36;1mif [ -n "${ONE_OFF_RAW// }" ]; then�[0m
 �[36;1m  ONE_OFF=$(printf '%s' "$ONE_OFF_RAW" | sed -E 's|^[[:space:]]*/[Kk][Ii][Rr][Oo][Cc][Rr][Ee][Ww][[:space:]]*||')�[0m
 �[36;1m  ONE_OFF=$(printf '%s' "$ONE_OFF" | sed -E 's|/kirocrew||Ig' | sed -E 's/^[[:space:]]+//;s/[[:space:]]+$//')�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mif [ -n "${PR_NUMBER}" ]; then�[0m
 �[36;1m  COMMENT_TARGET="Leave a single GitHub PR comment on ${REPO}#${PR_NUMBER} (gh pr comment ${PR_NUMBER})."�[0m
 �[36;1m  PR_LABEL="PR #${PR_NUMBER}"�[0m
 �[36;1melse�[0m
 �[36;1m  COMMENT_TARGET="If an open PR contains commit ${SHA}, comment on that PR; otherwise leave a commit comment on ${SHA}."�[0m
 �[36;1m  PR_LABEL=""�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mMSG=$(jq -nr \�[0m
 �[36;1m  --arg prefix "$MESSAGE_PREFIX" \�[0m
 �[36;1m  --arg event "$EVENT" \�[0m
 �[36;1m  --arg repo "$REPO" \�[0m
 �[36;1m  --arg ref "$REF" \�[0m
 �[36;1m  --arg sha "$SHA" \�[0m
 �[36;1m  --arg run_id "$RUN_ID" \�[0m
 �[36;1m  --arg run_url "$RUN_URL" \�[0m
 �[36;1m  --arg pr_number "$PR_NUMBER" \�[0m
 �[36;1m  --arg pr_label "$PR_LABEL" \�[0m
 �[36;1m  --arg daytona "$DAYTONA_OS" \�[0m
 �[36;1m  --arg one_off "$ONE_OFF" \�[0m
 �[36;1m  --arg comment_url "$COMMENT_URL" \�[0m
 �[36;1m  --arg comment_target "$COMMENT_TARGET" \�[0m
 �[36;1m  '�[0m
 �[36;1m  def nl: "\n";�[0m
 �[36;1m  (if ($prefix | length) > 0 then $prefix ...

GitHub Actions: Trigger Kiro Crew / trigger _ trigger-agent: feat(client): display and gate on provisional evidence tier (P6)

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m: "${***REDACTED_SECRET_ASSIGNMENT*** missing}"�[0m
 �[36;1m: "${***REDACTED_SECRET_ASSIGNMENT*** missing}"�[0m
 �[36;1m: "${GATEWAY_URL:?KIRO_GATEWAY_URL missing}"�[0m
 �[36;1m�[0m
 �[36;1mif [ -n "${GATEWAY_ORIGIN:-}" ]; then�[0m
 �[36;1m  ORIGIN="$GATEWAY_ORIGIN"�[0m
 �[36;1melse�[0m
 �[36;1m  ORIGIN=$(printf '%s' "$GATEWAY_URL" | sed -E 's#(https?://[^/]+).*#\1#')�[0m
 �[36;1mfi�[0m
 �[36;1mREPO_SHORT="${REPO##*/}"�[0m
 �[36;1mSESSION_KEY="hook:github:${REPO_SHORT}:${EVENT}:${RUN_ID}"�[0m
 �[36;1m�[0m
 �[36;1mONE_OFF=""�[0m
 �[36;1mif [ -n "${ONE_OFF_RAW// }" ]; then�[0m
 �[36;1m  ONE_OFF=$(printf '%s' "$ONE_OFF_RAW" | sed -E 's|^[[:space:]]*/[Kk][Ii][Rr][Oo][Cc][Rr][Ee][Ww][[:space:]]*||')�[0m
 �[36;1m  ONE_OFF=$(printf '%s' "$ONE_OFF" | sed -E 's|/kirocrew||Ig' | sed -E 's/^[[:space:]]+//;s/[[:space:]]+$//')�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mif [ -n "${PR_NUMBER}" ]; then�[0m
 �[36;1m  COMMENT_TARGET="Leave a single GitHub PR comment on ${REPO}#${PR_NUMBER} (gh pr comment ${PR_NUMBER})."�[0m
 �[36;1m  PR_LABEL="PR #${PR_NUMBER}"�[0m
 �[36;1melse�[0m
 �[36;1m  COMMENT_TARGET="If an open PR contains commit ${SHA}, comment on that PR; otherwise leave a commit comment on ${SHA}."�[0m
 �[36;1m  PR_LABEL=""�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mMSG=$(jq -nr \�[0m
 �[36;1m  --arg prefix "$MESSAGE_PREFIX" \�[0m
 �[36;1m  --arg event "$EVENT" \�[0m
 �[36;1m  --arg repo "$REPO" \�[0m
 �[36;1m  --arg ref "$REF" \�[0m
 �[36;1m  --arg sha "$SHA" \�[0m
 �[36;1m  --arg run_id "$RUN_ID" \�[0m
 �[36;1m  --arg run_url "$RUN_URL" \�[0m
 �[36;1m  --arg pr_number "$PR_NUMBER" \�[0m
 �[36;1m  --arg pr_label "$PR_LABEL" \�[0m
 �[36;1m  --arg daytona "$DAYTONA_OS" \�[0m
 �[36;1m  --arg one_off "$ONE_OFF" \�[0m
 �[36;1m  --arg comment_url "$COMMENT_URL" \�[0m
 �[36;1m  --arg comment_target "$COMMENT_TARGET" \�[0m
 �[36;1m  '�[0m
 �[36;1m  def nl: "\n";�[0m
 �[36;1m  (if ($prefix | length) > 0 then $prefix ...
🧰 Additional context used
📓 Path-based instructions (13)
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: Use Serena's semantic, symbol-aware tools as the primary tools for reading and editing code; use built-in Read, Glob, Grep, and Edit only under the stated exceptions.
Before editing a code file, inspect its symbol overview, read the specific symbols being changed, and edit them with Serena's symbol-aware tools.
Understand the existing code before changing it and make the smallest change that satisfies the request; avoid unrelated cleanup, premature abstractions, unnecessary error handling, feature flags, and compatibility shims.
Prefer editing existing files over creating new ones, and never create Markdown or README files unless explicitly requested.
For exploratory questions, provide a 2–3 sentence recommendation with the main tradeoff and do not implement until the user agrees.
For UI or frontend changes that cannot be tested in a browser, explicitly state that browser testing was not performed rather than claiming success.
Address security issues when discovered, including injection, XSS, SQL injection, path traversal, and secret leaks.
Pause and obtain confirmation before destructive, hard-to-reverse, externally visible, or third-party-upload actions, including deleting files or branches, dropping tables, force-pushing, modifying CI/CD, posting externally, or uploading content.
When blocked, investigate the root cause instead of bypassing it with --no-verify, --force, or deletion; investigate unfamiliar files, branches, and configuration before deleting them.
Only commit when explicitly asked; do not proactively update git configuration or push changes.
Do not skip Git hooks unless explicitly asked; if a pre-commit hook fails, fix the issue, re-stage, and create a new commit rather than amending.
Stage files by name rather than using git add -A or git add ., to avoid accidentally including secrets or large binaries.
Use a HEREDOC for commit messages, and add co-author attribution only when the user explicitly requests the exact trailer.
Do not force-pus...

Files:

  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/cmd/info.rs
  • src/install/transaction.rs
  • src/core/resolve.rs
  • src/core/package.rs
  • tests/install_test.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
**/*.{js,jsx,ts,tsx,py,java,go,rs,rb,php,c,cpp,h,hpp,cs,swift,kt,kts}

📄 CodeRabbit inference engine (CLAUDE.md)

Add comments only when the WHY is non-obvious; do not narrate what the code does, reference the current task, or reference the PR in comments.

Files:

  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/cmd/info.rs
  • src/install/transaction.rs
  • src/core/resolve.rs
  • src/core/package.rs
  • tests/install_test.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
!**/.env,!**/credentials.json,!**/*.pem

📄 CodeRabbit inference engine (CLAUDE.md)

Do not commit files that appear to contain secrets, including .env, credentials.json, and PEM files; warn before doing so even if explicitly requested.

Files:

  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/cmd/info.rs
  • src/install/transaction.rs
  • src/core/resolve.rs
  • src/core/package.rs
  • tests/install_test.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.rs: Use the Rust 2021 edition.
Use anyhow::Result with .context(...) in application code; use thiserror for library error types that callers match on.
Use clap derive macros for CLI definitions.
Use serde with serde_json or toml for serialization.
Function parameters must use &Path, not &PathBuf.
Library code must not panic; error paths should return anyhow::Result with context where appropriate.
Add or update tests for behavior changes, including relevant failure paths.
New mutating code paths must acquire the mutation lock via acquire_mutation_lock(root) and snapshot the lockfile before writes.
Lockfile, journal, and state-file JSON writes must use write_json_atomic.
numan install must write only to $NUMAN_ROOT; it must not invoke Nu or register plugins/autoloads.
Only activate and deactivate may modify Nu integration state.
Treat the lockfile as the authoritative source of truth; derived projections such as autoload state must not be authoritative.
Install payloads under versioned, content-addressed paths and never overwrite them in place.
Never overwrite foreign autoload files; respect OWNERSHIP_MARKER.
Pass plugin paths through environment variables only; do not use runtime interpolation in Nu program strings.

**/*.rs: All CI gates must pass: cargo test, cargo clippy -- -D warnings, and cargo fmt --check.
Every mutating command—including install, remove, update, gc, and future nupm import—must call acquire_mutation_lock(root).
Lockfiles, journals, and state files must use write_json_atomic; partial writes are not allowed.
Pending activation, autoload, and lifecycle journals must be stored under $NUMAN_ROOT/state/.
Module autoload identity must match all four fields: Nu executable hash, Nu version, vendor autoload directory, and managed file path; the lockfile module_activation value is authoritative.
Never overwrite foreign autoload files; respect OWNERSHIP_MARKER.
Pass paths to Nu only throu...

Files:

  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/cmd/info.rs
  • src/install/transaction.rs
  • src/core/resolve.rs
  • src/core/package.rs
  • tests/install_test.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run and keep cargo fmt/rustfmt clean, and ensure cargo clippy -- -D warnings passes.

Files:

  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/cmd/info.rs
  • src/install/transaction.rs
  • src/core/resolve.rs
  • src/core/package.rs
  • tests/install_test.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
**/*.{rs,nu}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{rs,nu}: Real-Nu acceptance tests must be marked #[ignore] and should be run when changes affect activation or nupm import; unit tests must not spawn real nu and should use injectable seams such as FakeCandidateRunner or registrars.
The nupm integration must be read-only toward NUPM_HOME, must not execute build.nu, and must not perform bidirectional synchronization.

Unit tests must use FakeCandidateRunner or injectable registrars and must not spawn a real nu process.

Files:

  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/cmd/info.rs
  • src/install/transaction.rs
  • src/core/resolve.rs
  • src/core/package.rs
  • tests/install_test.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
**/*.{rs,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Match existing naming, module layout, and documentation level in the file being edited; update AGENTS.md, docs/, or command help when structure, conventions, or user-visible behavior changes.

Tests must cover failure modes, not only successful execution.

Files:

  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/cmd/info.rs
  • src/install/transaction.rs
  • src/core/resolve.rs
  • src/core/package.rs
  • tests/install_test.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
**/*.{rs,md,toml}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use the repository's established serialization and module conventions rather than introducing unrelated refactors.

Files:

  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/cmd/info.rs
  • src/install/transaction.rs
  • src/core/resolve.rs
  • src/core/package.rs
  • tests/install_test.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
src/core/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Derive the platform triple from compile-time #[cfg(target_env)] values, not std::env::consts; LIBC must be a compile-time constant.

Files:

  • src/core/registry.rs
  • src/core/resolve.rs
  • src/core/package.rs
src/core/{trust.rs,official_registry.rs,registry.rs}

📄 CodeRabbit inference engine (AGENTS.md)

Verify Ed25519 signatures over the exact index.json bytes; use the built-in production trust root for the official registry. Unsigned bypass is allowed only with NUMAN_ALLOW_UNSIGNED=1 for development.

Files:

  • src/core/registry.rs
src/cmd/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Pass Nu paths and names through environment variables (NUMAN_PLUGIN_BINARY, NUMAN_PLUGIN_CONFIG, and NUMAN_PLUGIN_NAME); the Nu program string must be a compile-time constant with no runtime interpolation.

Files:

  • src/cmd/try_cmd.rs
  • src/cmd/info.rs
  • src/cmd/search.rs
src/install/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

src/install/**/*.rs: Plugin binary artifacts must include a SHA256; the install transaction must fail when sha256 is missing.
Installation must remain inert: it may write only to $NUMAN_ROOT and must not perform Nu integration.

Files:

  • src/install/transaction.rs
tests/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Test platform-specific behavior with mock platforms, and use injected fakes rather than real Nu for unit tests.

Files:

  • tests/install_test.rs
  • tests/support/active_update/mod.rs
🔍 Remote MCP DeepWiki, GitHub Copilot

Additional review context

  • The client contract is paired with open numan-registry#64: the registry schema adds evidence_tier (proven/provisional) and deferral_reason; add-package.py --provisional emits both fields. The current main schema does not yet contain these properties and uses additionalProperties: false.
  • Registry signatures cover the canonical JSON for the entire index. Adding these fields changes signed bytes, so published indexes must be re-signed; the client verifier still accepts schema_version: 1.
  • search marks the resolver-selected displayed version, while install resolves the version and returns before the warning on the already-installed no-op path.
  • The registry-side tooling treats a missing provisional reason as a non-fatal warning, and the client displays reason not recorded; the schema description calls the reason required but does not conditionally enforce that requirement.
  • PR checks show tests, formatting, Clippy, MSRV, CodeQL, and platform acceptance succeeding. A trigger / trigger-agent check failed, and Greptile review was still in progress. The PR explicitly states that install-warning output lacks a dedicated test.
🔇 Additional comments (7)
src/cmd/info.rs (1)

82-88: LGTM!

Also applies to: 172-173, 208-234

src/cmd/try_cmd.rs (1)

471-472: LGTM!

src/core/registry.rs (1)

409-410: LGTM!

src/core/resolve.rs (1)

729-730: LGTM!

Also applies to: 748-749, 1062-1063

tests/install_test.rs (1)

386-387: LGTM!

Also applies to: 470-471, 542-543

tests/support/active_update/mod.rs (1)

509-510: LGTM!

src/cmd/search.rs (1)

2-2: LGTM!

Also applies to: 104-115, 152-158, 271-272, 358-375

Comment thread src/cmd/search.rs Outdated
Comment thread src/core/package.rs
Comment thread src/install/transaction.rs
Comment thread tests/install_test.rs
@qodo-code-review

qodo-code-review Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Empty deferral reason shown ✓ Resolved 🐞 Bug ≡ Correctness
Description
format_info() and install_package() only fall back to "reason not recorded" when
deferral_reason is None, so Some("") renders as a blank reason (e.g., "provisional — "). This
contradicts the intended behavior and produces misleading/malformed CLI output for provisional
entries with empty reasons.
Code

src/cmd/info.rs[R84-87]

+                .deferral_reason
+                .as_deref()
+                .unwrap_or("reason not recorded");
+            out.push_str(&format!("    evidence:    provisional — {reason}\n"));
Relevance

●●● Strong

Matches PR intent (“empty/absent”); likely to treat empty string as missing to avoid blank CLI
output.

PR-#103

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Both call sites use unwrap_or("reason not recorded") which only triggers on None, not on
Some(""). Because deferral_reason is an unconstrained optional string with serde default, empty
strings are a valid decoded state and will bypass the fallback.

src/cmd/info.rs[82-88]
src/install/transaction.rs[411-420]
src/core/package.rs[100-131]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The provisional evidence message uses `as_deref().unwrap_or("reason not recorded")`, which does not treat an empty string as “missing”. As a result, `deferral_reason: ""` prints an empty reason instead of the intended fallback.

## Issue Context
`deferral_reason` is `Option<String>` with `#[serde(default)]`, so registry JSON can decode to `Some("")` without any validation.

## Fix Focus Areas
- src/cmd/info.rs[82-88]
- src/install/transaction.rs[411-420]
- src/core/package.rs[100-131]

## Suggested fix
Use a shared helper (or inline) that treats empty/whitespace-only reasons as absent:

```rust
let reason = ver
 .deferral_reason
 .as_deref()
 .map(str::trim)
 .filter(|s| !s.is_empty())
 .unwrap_or("reason not recorded");
```

Apply the same logic in `install_package()` for consistency.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Redundant provisional_marker comment ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new doc comment on provisional_marker() restates what is already obvious from the function
name and immediate code. This adds maintenance overhead and violates the guideline to keep comments
focused on non-obvious rationale.
Code

src/cmd/search.rs[R151-153]

+/// Row suffix marking a provisional (not lifecycle-tested) displayed version.
+fn provisional_marker(entry: Option<&VersionEntry>) -> &'static str {
+    if entry.map(|v| v.is_provisional()).unwrap_or(false) {
Relevance

●●● Strong

Team has precedent removing/reworking redundant doc comments that restate behavior.

PR-#108

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2452624 requires avoiding comments that merely restate behavior. The comment
immediately above provisional_marker() describes the same behavior the function name and return
branches already make clear.

Rule 2452624: Restrict code comments to non-obvious rationale, not restating behavior
src/cmd/search.rs[151-156]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new comment was added that restates the adjacent code behavior, rather than documenting non-obvious rationale.

## Issue Context
Compliance requires comments to capture rationale/constraints and avoid repeating what the code already clearly expresses.

## Fix Focus Areas
- src/cmd/search.rs[151-156]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Unsanitized reason breaks output ✓ Resolved 🐞 Bug ⛨ Security
Description
The new provisional messaging prints deferral_reason verbatim; if it contains newlines or terminal
control sequences, it can spoof additional lines or corrupt terminal output. Since the value comes
from registry metadata, it should be rendered as a single safe line (or control characters
stripped/escaped) before printing.
Code

src/install/transaction.rs[R416-419]

+        println!(
+            "{} This package has not been lifecycle-tested. It passed integrity checks. ({reason})",
+            console::style("!").yellow()
+        );
Relevance

●● Moderate

Security hardening of CLI output is plausible, but escaping policy may be debated without clear
precedent.

PR-#95
PR-#83

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
install_package() loads a registry index, resolves a VersionEntry, and prints
resolved.deferral_reason directly inside println!. deferral_reason is part of VersionEntry
deserialized from registry JSON, and there is no sanitization step before emitting it.

src/install/transaction.rs[48-92]
src/install/transaction.rs[411-420]
src/core/package.rs[100-123]
src/cmd/info.rs[82-88]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`deferral_reason` is interpolated directly into user-visible CLI output. If the registry value contains `\n`, `\r`, or ANSI/control characters, the terminal output can be manipulated (extra fake warnings/success lines) or become unreadable.

## Issue Context
This PR introduces new output paths that include `deferral_reason` in both `numan info` and `numan install`.

## Fix Focus Areas
- src/install/transaction.rs[411-420]
- src/cmd/info.rs[82-88]

## Suggested fix
Before printing, normalize to a single line and remove/escape control characters, for example:
- replace `\r`/`\n` with spaces
- drop other `char::is_control()` characters (optionally allow `\t`)

Consider implementing a small shared utility like `fn sanitize_cli_value(s: &str) -> String` and use it for `deferral_reason` (and potentially other registry-provided fields).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 31 rules
✅ REVIEW.md
Review mode: ⚖️ Balanced: This is a behavior-changing client feature spanning the data model, resolution/install flow, and multiple CLI presentation paths; it has meaningful blast radius, but the logic is coherent enough for one careful review rather than redundant passes.

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/cmd/search.rs Outdated
Comment thread src/cmd/info.rs Outdated
Comment thread src/install/transaction.rs
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo Fixer

No findings are within the configured fix scope. To change which findings are fixed, adjust the setting on your Qodo configuration page.

@tonythethompson

Copy link
Copy Markdown
Collaborator Author

Addressed in e3b5675 (pushed): merged latest master (resolved struct-literal conflicts with P1's provenance field), fixed empty/blank/whitespace-only deferral_reason falling back to "reason not recorded" (was previously showing blank), stripped control characters from deferral_reason before printing (terminal-injection risk), converted evidence_tier from an open Option<String> to a closed EvidenceTier enum so unrecognized values now fail deserialization instead of silently being treated as proven, and removed the narrative comment on provisional_marker().

Not fixed, flagged as out of scope for this PR: the cache-hit integrity-bypass comment on transaction.rs (pre-existing behavior, unrelated to the evidence-tier feature this PR adds) and the request for new integration-test fixtures covering the full install-notice path (unit tests already cover is_provisional() and deferral_reason_display() directly; adding install-flow fixtures is a larger lift better done separately).

@tonythethompson
tonythethompson merged commit b9e5d4e into master Aug 10, 2026
23 checks passed
@tonythethompson
tonythethompson deleted the feat/p6-provisional-tier branch August 10, 2026 12:22
@linear-code

linear-code Bot commented Aug 10, 2026

Copy link
Copy Markdown

NUM-96

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant