Skip to content

Enhance provenance display and refine completions in numan - #118

Closed
tonythethompson wants to merge 5 commits into
masterfrom
feat/p1-commit-snapshot-intake
Closed

Enhance provenance display and refine completions in numan#118
tonythethompson wants to merge 5 commits into
masterfrom
feat/p1-commit-snapshot-intake

Conversation

@tonythethompson

@tonythethompson tonythethompson commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

PR Summary by Qodo

Show commit-snapshot provenance, gate fork status, and slim completions

✨ Enhancement 🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Add optional version provenance and show a commit-snapshot note in numan info.
• Gate “verified upstream artifact” status for numan-maintained fork packages.
• Shrink generated completion scripts by omitting help subcommands and fixing Nushell hints.
Diagram

graph TD
  R[("Registry index") ] --> V["VersionEntry (provenance)"] --> I["info formatter"] --> O["Terminal output"]
  C["clap CLI tree"] --> D["Completions cmd"] --> H["Help subcmds disabled"] --> S["Shell scripts"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Model provenance as an enum (with unknown passthrough)
  • ➕ Avoids stringly-typed checks scattered in UI code
  • ➕ Makes it explicit which values trigger user-facing messaging
  • ➕ Easier to evolve without accidental partial matches
  • ➖ Requires serde custom parsing (or untagged strategy) to remain forward-compatible
  • ➖ Adds a small amount of type/boilerplate complexity
2. Centralize provenance/status messaging in a dedicated presenter
  • ➕ Keeps format_info() smaller and reduces future conditional sprawl
  • ➕ Encourages consistent wording across commands (info/search/try)
  • ➖ May be over-architecting for a single message today
  • ➖ Adds indirection for contributors reading the info command
3. Filter completion commands via clap metadata instead of recursive mutation
  • ➕ Could leverage clap’s configuration knobs to reduce custom recursion
  • ➕ Potentially less code if clap supports the desired pruning directly
  • ➖ May not fully remove nested help subcommands across all command groups
  • ➖ Behavior could vary across clap versions; harder to test/guarantee

Recommendation: The current approach is a good incremental step: keep provenance as an optional string with an exact-match check to preserve forward compatibility, and prune help subcommands recursively to materially reduce completion size. Consider a follow-up to introduce a small Provenance enum (with Unknown(String)), if provenance values expand and multiple commands start depending on them.

Files changed (10) +120 / -7

Enhancement (2) +63 / -1
info.rsGate fork status and display commit-snapshot provenance note +36/-1

Gate fork status and display commit-snapshot provenance note

• Changes 'numan info' to label 'numan-maintained' packages as forks rather than always claiming “verified upstream artifact”. Adds a note for versions whose provenance is exactly 'commit-snapshot', plus tests ensuring other/absent provenance values never trigger the message.

src/cmd/info.rs

package.rsAdd optional provenance to VersionEntry and parse from registry JSON +27/-0

Add optional provenance to VersionEntry and parse from registry JSON

• Introduces an optional 'provenance' field on 'VersionEntry' with serde defaulting for backward compatibility. Adds unit tests verifying missing provenance defaults to 'None' and that 'commit-snapshot' parses correctly.

src/core/package.rs

Bug fix (1) +45 / -5
completions.rsPrune help subcommands from completions and fix Nushell hint +45/-5

Prune help subcommands from completions and fix Nushell hint

• Adds a dedicated clap command builder for completions that disables help subcommands recursively to avoid bloated completion scripts. Fixes the Nushell install hint to use 'mkdir' (no '--all') and prints a reminder to re-run completions after upgrading; extends tests to assert both behaviors across shells.

src/cmd/completions.rs

Tests (6) +11 / -0
search.rsUpdate search fixtures for new VersionEntry field +1/-0

Update search fixtures for new VersionEntry field

• Extends test fixtures to populate the new 'provenance' field (defaulting to 'None') so compilation and expectations remain consistent.

src/cmd/search.rs

try_cmd.rsUpdate try-command fixtures for new VersionEntry field +1/-0

Update try-command fixtures for new VersionEntry field

• Updates try-command test fixtures to include 'provenance: None' to match the expanded 'VersionEntry' struct.

src/cmd/try_cmd.rs

registry.rsUpdate registry tests for provenance field +1/-0

Update registry tests for provenance field

• Updates registry test fixtures to include 'provenance: None' for 'VersionEntry' literals after the struct expansion.

src/core/registry.rs

resolve.rsUpdate resolver tests for provenance field +3/-0

Update resolver tests for provenance field

• Adjusts resolver test data to provide 'provenance: None' in 'VersionEntry' instances used by resolution scenarios.

src/core/resolve.rs

install_test.rsUpdate install integration tests for provenance field +4/-0

Update install integration tests for provenance field

• Updates integration test registries and snapshots to include the new 'provenance' field (set to 'None') to keep builds and assertions aligned.

tests/install_test.rs

mod.rsUpdate active-update test support for provenance field +1/-0

Update active-update test support for provenance field

• Extends the test registry writer helper to populate 'provenance: None' on generated 'VersionEntry' values.

tests/support/active_update/mod.rs

Documentation (1) +1 / -1
cli.rsClarify 'use' command help text +1/-1

Clarify 'use' command help text

• Updates the CLI help description for 'numan use' to reflect supported values ('latest', 'list', or a specific version). This improves user guidance without changing behavior.

src/cli.rs

tonythethompson and others added 4 commits August 9, 2026 23:43
VersionEntry gains an optional provenance field parsed from the
registry index. numan info shows a note when a version's provenance
is "commit-snapshot" ("built from a commit snapshot, not a tagged
release"). Other fixture-only VersionEntry literals across
tests/cmd updated for the new field.

Ref: docs/plans/2026-08-09-intake-process-reform.md (P1)
Cover the forward-compatible string contract so future provenance values
do not accidentally trigger the commit-snapshot message.

Co-authored-by: Cursor <cursoragent@cursor.com>
Drop help subcommands from generated completion trees to cut script size,
fix the invalid mkdir --all Nushell hint, update use command docs, and
remind users to refresh completions after upgrading numan.

Co-authored-by: Cursor <cursoragent@cursor.com>
ADR 0001 treats maintained forks as distinct from verified upstream
artifacts; format_info() was printing the verified-upstream status
line unconditionally, contradicting the fork identity output.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@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

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 343ece16-8b46-4031-a1c6-4eee22421a66

📥 Commits

Reviewing files that changed from the base of the PR and between 2281b87 and 1ac335b.

📒 Files selected for processing (10)
  • src/cli.rs
  • src/cmd/completions.rs
  • 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
  • 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. (1)
  • GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (15)
**/*

📄 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/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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: Use &Path rather than &PathBuf in Rust function parameters.
Use anyhow::Result for application code, thiserror for library errors, and add context with .context(...) or ?.
Never panic in library code; return errors instead.
Test-first development is expected: write a failing test, implement the change, then verify it passes.
Format and lint Rust code with cargo fmt --check and cargo clippy -- -D warnings; no warnings are permitted.

**/*.rs: All CI gates must pass: cargo test, cargo clippy -- -D warnings, cargo fmt --check, MSRV cargo +1.88 check --locked --all-targets, cargo package --locked, cargo deny, and ignored real-Nu acceptance tests.
numan install must not invoke...

Files:

  • src/core/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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.

Files:

  • src/core/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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.

Files:

  • src/core/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.rs
src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Use compile-time #[cfg(target_env)] platform detection rather than std::env::consts.

Files:

  • src/core/package.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.rs
src/core/package.rs

📄 CodeRabbit inference engine (AGENTS.md)

A plugin is active only when PluginActivation's Nu executable SHA256, Nu version, and plugin registry path all match the current NuPaths; do not use a bare boolean.

Files:

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

📄 CodeRabbit inference engine (AGENTS.md)

Keep integration tests in tests/, unit tests inline with source modules, and use mock platforms for platform-specific behavior.

Files:

  • tests/install_test.rs
  • tests/support/active_update/mod.rs
**/*_test.rs

📄 CodeRabbit inference engine (REVIEW.md)

Tests must cover failure modes, not only successful execution.

Files:

  • tests/install_test.rs
**/tests/**/*.rs

📄 CodeRabbit inference engine (REVIEW.md)

Tests must cover failure modes, not only successful execution.

Files:

  • tests/install_test.rs
  • tests/support/active_update/mod.rs
src/core/registry.rs

📄 CodeRabbit inference engine (AGENTS.md)

Verify registry signatures over the exact index.json bytes using Ed25519; unsigned bypass is permitted only with NUMAN_ALLOW_UNSIGNED=1 for development.

Files:

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

📄 CodeRabbit inference engine (AGENTS.md)

Create a snapshot before mutations performed by install, update, remove, activate, deactivate, nupm import, and init --refresh.

Files:

  • src/cmd/try_cmd.rs
  • src/cmd/search.rs
  • src/cmd/completions.rs
  • src/cmd/info.rs
🔍 Remote MCP Context7, DeepWiki, GitHub Copilot

Additional review context

  • The commit-snapshot contract is cross-repository: the intake ADR requires source_commit, registry propagation of "provenance": "commit-snapshot", and the exact numan info note. PR #118 changes only the client, so verify the registry producer/schema emits this field.
  • The ADR requires maintained forks to be clearly distinct from upstream and never silently substituted. PR #118 labels numan-maintained packages as forks, but info still prints the generic “verified means provenance…” footer for them; review whether that disclaimer is misleading for forks.
  • VersionEntry is serialized/deserialized through the registry model and consumed by resolution/install paths; #[serde(default)] preserves compatibility with older indexes lacking provenance.
  • The numan use help text aligns with PR #67’s intended behavior: latest, list, or an installed specific version, without implicit downloading.
  • PR checks currently show CodeQL, CodeFactor, and several language analyses successful; Rust analysis and Greptile remain in progress. There are no review threads.
  • Context7 did not return matching documentation for the specific recursive Clap API query, so completion behavior is primarily supported by the PR’s cross-shell tests.
🛑 Comments failed to post (3)
src/cmd/completions.rs (2)

190-193: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not instruct users to overwrite the Nushell autoload file.

save -f overwrites $nu.data-dir/vendor/autoload/numan-completions.nu without checking OWNERSHIP_MARKER. A foreign file at that path can be destroyed by following the manual hint. Remove the manual redirect, or add an ownership check before writing.

Proposed fix
-# Or manually (mkdir creates parent dirs by default):
-mkdir ($nu.data-dir | path join vendor/autoload)
-numan completions nushell --print | save -f ($nu.data-dir | path join vendor/autoload/numan-completions.nu)
+# Use the managed command above. It refuses to overwrite a foreign autoload file.

As per coding guidelines, never overwrite foreign autoload files and respect OWNERSHIP_MARKER.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

# Use the managed command above. It refuses to overwrite a foreign autoload file.
"
🤖 Prompt for 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.

In `@src/cmd/completions.rs` around lines 190 - 193, Remove the manual Nushell
installation instructions that invoke save -f, or revise that path to check
OWNERSHIP_MARKER before writing. Ensure the generated completion guidance never
overwrites a foreign numan-completions.nu autoload file.

Source: Coding guidelines


441-461: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Cover nested help subcommands in this regression test.

The test only rejects the contiguous text numan help. It can pass while a nested entry such as numan registry help, numan snapshot help, or numan setup help remains in the generated script. Assert representative nested paths for every shell, or inspect the transformed command tree directly.

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

🤖 Prompt for 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.

In `@src/cmd/completions.rs` around lines 441 - 461, Expand
completions_omit_help_subcommands to assert that representative nested help
paths such as numan registry help, numan snapshot help, and numan setup help are
absent for every CompletionShell, while preserving the existing top-level check.

Source: Coding guidelines

src/cmd/info.rs (1)

38-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use fork-specific wording in the footer and test it.

For numan-maintained packages, this branch prints not a verified upstream artifact, but format_info still appends the generic verified footer and refers to the upstream source. This mixes fork and upstream trust claims.

Emit a fork-specific footer or suppress the upstream footer for maintained forks. Add a test for the complete numan-maintained output.

This follows the intake requirement that maintained forks remain distinct from upstream.

Also applies to: 207-232

🤖 Prompt for 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.

In `@src/cmd/info.rs` around lines 38 - 42, Update format_info so numan-maintained
packages use fork-specific footer wording or omit the upstream verification
footer, avoiding references to verified upstream artifacts or “the upstream
source.” Preserve the existing upstream footer for other packages, and add
coverage asserting the complete numan-maintained output.

Source: MCP tools

🔇 Additional comments (14)
src/cli.rs (1)

64-64: LGTM!

src/cmd/completions.rs (3)

54-54: LGTM!


198-210: LGTM!


319-326: LGTM!

src/core/package.rs (2)

115-118: 🗄️ Data Integrity & Integration

Verify the registry producer emits the exact provenance marker.

This client displays the warning only when VersionEntry.provenance equals "commit-snapshot". Verify that the registry producer maps the required source_commit metadata to this exact field and value in published index.json. Otherwise, numan info silently omits the warning.

Add a signed-index fixture or cross-repository acceptance check for this contract.

This relies on the cross-repository commit-snapshot contract provided for this review.

Source: MCP tools


276-298: LGTM!

src/cmd/search.rs (1)

254-254: LGTM!

src/cmd/try_cmd.rs (1)

471-471: LGTM!

src/core/registry.rs (1)

409-409: LGTM!

src/core/resolve.rs (1)

729-729: LGTM!

Also applies to: 747-747, 1060-1060

tests/install_test.rs (1)

140-140: LGTM!

Also applies to: 385-385, 468-468, 539-539

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

509-509: LGTM!

src/cmd/info.rs (2)

86-89: 📐 Maintainability & Code Quality

Document the new numan info output.

This change adds a user-visible provenance note. Verify that AGENTS.md, docs/, or command help explains the commit-snapshot label. No documentation or help update is included in the supplied changes.

As per coding guidelines, **/*.{rs,md} requires updating AGENTS.md, docs/, or command help when user-visible behavior changes.

Source: Coding guidelines


172-172: LGTM!


📝 Walkthrough

Walkthrough

The PR updates CLI help and shell completion generation. It adds optional package provenance metadata, updates package information output, and adjusts fixtures and tests for the new field.

Changes

CLI experience

Layer / File(s) Summary
Completion generation and CLI guidance
src/cli.rs, src/cmd/completions.rs
The use help text documents supported version forms. Completion installation guidance includes an upgrade step. Nushell setup syntax is corrected, help subcommands are removed recursively, and shell-specific tests cover the generated completions.

Package provenance

Layer / File(s) Summary
Provenance schema and fixture propagation
src/core/package.rs, src/cmd/search.rs, src/cmd/try_cmd.rs, src/core/registry.rs, src/core/resolve.rs, tests/install_test.rs, tests/support/active_update/mod.rs
VersionEntry now has an optional serde-defaulted provenance field. Deserialization and package fixtures cover absent and commit-snapshot values.
Provenance display
src/cmd/info.rs
Package information labels numan-maintained packages as forks and displays a warning for commit-snapshot provenance. Tests cover matching, absent, and unrelated values.

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

Possibly related PRs

Suggested reviewers: greptile-apps

🚥 Pre-merge checks | ✅ 8
✅ Passed checks (8 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the two primary changes: provenance display and completion refinement.
Description check ✅ Passed The description accurately covers provenance handling, fork status, completion changes, and related tests.
Docstring Coverage ✅ Passed Docstring coverage is 63.64% which is sufficient. The required threshold is 60.00%.
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 repository contains no SessionWorkflowStage enum or references to its specified members; no ordering, raw-integer comparison, or legacy-converter check applies.
Gpu/Cpu Runtime Boundary ✅ Passed The pull request changes only Rust CLI, completion, package, registry, resolver, and test files; no inference, runtime requirements, Python main.py, or C# files are modified.
Managed Host Restart Safety ✅ Passed PASS: The PR changes only CLI, completion, and package provenance code; no managed-host components, restart calls, lease trackers, or busy-state logic exist in the base or PR tree.
✨ Finishing Touches 💡 1
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch feat/p1-commit-snapshot-intake
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/p1-commit-snapshot-intake
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feat/p1-commit-snapshot-intake

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.

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR enhances package information with commit-snapshot and maintained-fork provenance labels while reducing generated completion scripts by omitting synthetic help subcommands.

  • Adds optional, backward-compatible provenance metadata to registry version entries.
  • Displays commit-snapshot provenance and distinguishes numan-maintained forks in numan info.
  • Refines completion installation guidance, Nushell hints, and generated command trees.
  • Updates test fixtures for the expanded version-entry schema.

Confidence Score: 5/5

The PR appears safe to merge with no concrete changed-code failures identified.

The optional provenance field remains backward compatible, reaches the information display intact, and completion generation removes only synthetic help subcommands while preserving the repository’s real command tree.

Important Files Changed

Filename Overview
src/core/package.rs Adds an optional serde-defaulted provenance field to version entries, preserving compatibility with existing registry data.
src/cmd/info.rs Displays commit-snapshot provenance and a distinct status for packages in the numan-maintained scope.
src/cmd/completions.rs Removes synthetic help subcommands from completion generation and improves installation and Nushell guidance.
src/core/registry.rs Updates registry test construction for the new optional version provenance field.
src/core/resolve.rs Updates resolver test fixtures for the expanded VersionEntry schema without changing resolution behavior.
tests/install_test.rs Updates installation fixtures to initialize the new optional provenance field.

Reviews (1): Last reviewed commit: "fix(info): gate verified-upstream status..." | Re-trigger Greptile

greptile-apps[bot]
greptile-apps Bot previously approved these changes Aug 10, 2026
@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

No reviewable files after applying ignore patterns.

@tonythethompson
tonythethompson deleted the feat/p1-commit-snapshot-intake branch August 10, 2026 17:41
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

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

@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo Fixer

No findings are available for this PR yet. Findings appear here once Qodo has reviewed the PR.

@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: 1ac335bd40

ℹ️ 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/cmd/info.rs Outdated
Comment on lines +38 to +39
if pkg.id.owner == "numan-maintained" {
out.push_str("Status: numan-maintained fork (not a verified upstream artifact)\n");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Authenticate the maintained-fork label

When a custom registry is configured as the default, its index can assign any package the numan-maintained owner, and find_package() returns that package without preserving registry or signer identity. This branch therefore presents a third party's package as a Numan-maintained fork solely from an unreserved string; only emit this ownership claim when the package came from the verified official trust root, or qualify it with the actual registry identity.

AGENTS.md reference: AGENTS.md:L148-L148

Useful? React with 👍 / 👎.

Comment thread src/cmd/info.rs Outdated
Comment on lines +38 to +39
if pkg.id.owner == "numan-maintained" {
out.push_str("Status: numan-maintained fork (not a verified upstream artifact)\n");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Surface the original upstream for maintained forks

For an actual numan-maintained package, this new output path identifies it as a fork but still prints only pkg.repo and source.git, both of which describe the fork. SourceInfo at src/core/package.rs:146-152 also discards the planned source.upstream field, so numan info cannot tell users which original author or repository was forked, contrary to the maintained-fork inspection contract in docs/plans/2026-08-09-intake-process-reform.md:263-277; deserialize and display that upstream identity with this status.

Useful? React with 👍 / 👎.

@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: 3

🤖 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/completions.rs`:
- Around line 441-461: Expand completions_omit_help_subcommands to assert that
representative nested help paths such as numan registry help, numan snapshot
help, and numan setup help are absent for every CompletionShell, while
preserving the existing top-level check.
- Around line 190-193: Remove the manual Nushell installation instructions that
invoke save -f, or revise that path to check OWNERSHIP_MARKER before writing.
Ensure the generated completion guidance never overwrites a foreign
numan-completions.nu autoload file.

In `@src/cmd/info.rs`:
- Around line 38-42: Update format_info so numan-maintained packages use
fork-specific footer wording or omit the upstream verification footer, avoiding
references to verified upstream artifacts or “the upstream source.” Preserve the
existing upstream footer for other packages, and add coverage asserting the
complete numan-maintained output.
🪄 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: 343ece16-8b46-4031-a1c6-4eee22421a66

📥 Commits

Reviewing files that changed from the base of the PR and between 2281b87 and 1ac335b.

📒 Files selected for processing (10)
  • src/cli.rs
  • src/cmd/completions.rs
  • 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
  • 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
🧰 Additional context used
📓 Path-based instructions (15)
**/*

📄 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/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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: Use &Path rather than &PathBuf in Rust function parameters.
Use anyhow::Result for application code, thiserror for library errors, and add context with .context(...) or ?.
Never panic in library code; return errors instead.
Test-first development is expected: write a failing test, implement the change, then verify it passes.
Format and lint Rust code with cargo fmt --check and cargo clippy -- -D warnings; no warnings are permitted.

**/*.rs: All CI gates must pass: cargo test, cargo clippy -- -D warnings, cargo fmt --check, MSRV cargo +1.88 check --locked --all-targets, cargo package --locked, cargo deny, and ignored real-Nu acceptance tests.
numan install must not invoke...

Files:

  • src/core/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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.

Files:

  • src/core/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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.

Files:

  • src/core/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.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/package.rs
  • tests/install_test.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • tests/support/active_update/mod.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.rs
src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Use compile-time #[cfg(target_env)] platform detection rather than std::env::consts.

Files:

  • src/core/package.rs
  • src/core/registry.rs
  • src/cmd/try_cmd.rs
  • src/core/resolve.rs
  • src/cmd/search.rs
  • src/cli.rs
  • src/cmd/completions.rs
  • src/cmd/info.rs
src/core/package.rs

📄 CodeRabbit inference engine (AGENTS.md)

A plugin is active only when PluginActivation's Nu executable SHA256, Nu version, and plugin registry path all match the current NuPaths; do not use a bare boolean.

Files:

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

📄 CodeRabbit inference engine (AGENTS.md)

Keep integration tests in tests/, unit tests inline with source modules, and use mock platforms for platform-specific behavior.

Files:

  • tests/install_test.rs
  • tests/support/active_update/mod.rs
**/*_test.rs

📄 CodeRabbit inference engine (REVIEW.md)

Tests must cover failure modes, not only successful execution.

Files:

  • tests/install_test.rs
**/tests/**/*.rs

📄 CodeRabbit inference engine (REVIEW.md)

Tests must cover failure modes, not only successful execution.

Files:

  • tests/install_test.rs
  • tests/support/active_update/mod.rs
src/core/registry.rs

📄 CodeRabbit inference engine (AGENTS.md)

Verify registry signatures over the exact index.json bytes using Ed25519; unsigned bypass is permitted only with NUMAN_ALLOW_UNSIGNED=1 for development.

Files:

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

📄 CodeRabbit inference engine (AGENTS.md)

Create a snapshot before mutations performed by install, update, remove, activate, deactivate, nupm import, and init --refresh.

Files:

  • src/cmd/try_cmd.rs
  • src/cmd/search.rs
  • src/cmd/completions.rs
  • src/cmd/info.rs
🔍 Remote MCP Context7, DeepWiki, GitHub Copilot

Additional review context

  • The commit-snapshot contract is cross-repository: the intake ADR requires source_commit, registry propagation of "provenance": "commit-snapshot", and the exact numan info note. PR #118 changes only the client, so verify the registry producer/schema emits this field.
  • The ADR requires maintained forks to be clearly distinct from upstream and never silently substituted. PR #118 labels numan-maintained packages as forks, but info still prints the generic “verified means provenance…” footer for them; review whether that disclaimer is misleading for forks.
  • VersionEntry is serialized/deserialized through the registry model and consumed by resolution/install paths; #[serde(default)] preserves compatibility with older indexes lacking provenance.
  • The numan use help text aligns with PR #67’s intended behavior: latest, list, or an installed specific version, without implicit downloading.
  • PR checks currently show CodeQL, CodeFactor, and several language analyses successful; Rust analysis and Greptile remain in progress. There are no review threads.
  • Context7 did not return matching documentation for the specific recursive Clap API query, so completion behavior is primarily supported by the PR’s cross-shell tests.
🔇 Additional comments (14)
src/cli.rs (1)

64-64: LGTM!

src/cmd/completions.rs (3)

54-54: LGTM!


198-210: LGTM!


319-326: LGTM!

src/core/package.rs (2)

115-118: 🗄️ Data Integrity & Integration

Verify the registry producer emits the exact provenance marker.

This client displays the warning only when VersionEntry.provenance equals "commit-snapshot". Verify that the registry producer maps the required source_commit metadata to this exact field and value in published index.json. Otherwise, numan info silently omits the warning.

Add a signed-index fixture or cross-repository acceptance check for this contract.

This relies on the cross-repository commit-snapshot contract provided for this review.

Source: MCP tools


276-298: LGTM!

src/cmd/search.rs (1)

254-254: LGTM!

src/cmd/try_cmd.rs (1)

471-471: LGTM!

src/core/registry.rs (1)

409-409: LGTM!

src/core/resolve.rs (1)

729-729: LGTM!

Also applies to: 747-747, 1060-1060

tests/install_test.rs (1)

140-140: LGTM!

Also applies to: 385-385, 468-468, 539-539

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

509-509: LGTM!

src/cmd/info.rs (2)

86-89: 📐 Maintainability & Code Quality

Document the new numan info output.

This change adds a user-visible provenance note. Verify that AGENTS.md, docs/, or command help explains the commit-snapshot label. No documentation or help update is included in the supplied changes.

As per coding guidelines, **/*.{rs,md} requires updating AGENTS.md, docs/, or command help when user-visible behavior changes.

Source: Coding guidelines


172-172: LGTM!

@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.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 3

🤖 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/completions.rs`:
- Around line 441-461: Expand completions_omit_help_subcommands to assert that
representative nested help paths such as numan registry help, numan snapshot
help, and numan setup help are absent for every CompletionShell, while
preserving the existing top-level check.
- Around line 190-193: Remove the manual Nushell installation instructions that
invoke save -f, or revise that path to check OWNERSHIP_MARKER before writing.
Ensure the generated completion guidance never overwrites a foreign
numan-completions.nu autoload file.

In `@src/cmd/info.rs`:
- Around line 38-42: Update format_info so numan-maintained packages use
fork-specific footer wording or omit the upstream verification footer, avoiding
references to verified upstream artifacts or “the upstream source.” Preserve the
existing upstream footer for other packages, and add coverage asserting the
complete numan-maintained output.
🪄 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: 343ece16-8b46-4031-a1c6-4eee22421a66

📥 Commits

Reviewing files that changed from the base of the PR and between 2281b87 and 1ac335b.

📒 Files selected for processing (10)
  • src/cli.rs
  • src/cmd/completions.rs
  • 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
  • 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
🔇 Additional comments (14)
src/cli.rs (1)

64-64: LGTM!

src/cmd/completions.rs (3)

54-54: LGTM!


198-210: LGTM!


319-326: LGTM!

src/core/package.rs (2)

115-118: 🗄️ Data Integrity & Integration

Verify the registry producer emits the exact provenance marker.

This client displays the warning only when VersionEntry.provenance equals "commit-snapshot". Verify that the registry producer maps the required source_commit metadata to this exact field and value in published index.json. Otherwise, numan info silently omits the warning.

Add a signed-index fixture or cross-repository acceptance check for this contract.

This relies on the cross-repository commit-snapshot contract provided for this review.

Source: MCP tools


276-298: LGTM!

src/cmd/search.rs (1)

254-254: LGTM!

src/cmd/try_cmd.rs (1)

471-471: LGTM!

src/core/registry.rs (1)

409-409: LGTM!

src/core/resolve.rs (1)

729-729: LGTM!

Also applies to: 747-747, 1060-1060

tests/install_test.rs (1)

140-140: LGTM!

Also applies to: 385-385, 468-468, 539-539

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

509-509: LGTM!

src/cmd/info.rs (2)

86-89: 📐 Maintainability & Code Quality

Document the new numan info output.

This change adds a user-visible provenance note. Verify that AGENTS.md, docs/, or command help explains the commit-snapshot label. No documentation or help update is included in the supplied changes.

As per coding guidelines, **/*.{rs,md} requires updating AGENTS.md, docs/, or command help when user-visible behavior changes.

Source: Coding guidelines


172-172: LGTM!

🛑 Comments failed to post (3)
src/cmd/completions.rs (2)

190-193: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not instruct users to overwrite the Nushell autoload file.

save -f overwrites $nu.data-dir/vendor/autoload/numan-completions.nu without checking OWNERSHIP_MARKER. A foreign file at that path can be destroyed by following the manual hint. Remove the manual redirect, or add an ownership check before writing.

Proposed fix
-# Or manually (mkdir creates parent dirs by default):
-mkdir ($nu.data-dir | path join vendor/autoload)
-numan completions nushell --print | save -f ($nu.data-dir | path join vendor/autoload/numan-completions.nu)
+# Use the managed command above. It refuses to overwrite a foreign autoload file.

As per coding guidelines, never overwrite foreign autoload files and respect OWNERSHIP_MARKER.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

# Use the managed command above. It refuses to overwrite a foreign autoload file.
"
🤖 Prompt for 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.

In `@src/cmd/completions.rs` around lines 190 - 193, Remove the manual Nushell
installation instructions that invoke save -f, or revise that path to check
OWNERSHIP_MARKER before writing. Ensure the generated completion guidance never
overwrites a foreign numan-completions.nu autoload file.

Source: Coding guidelines


441-461: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Cover nested help subcommands in this regression test.

The test only rejects the contiguous text numan help. It can pass while a nested entry such as numan registry help, numan snapshot help, or numan setup help remains in the generated script. Assert representative nested paths for every shell, or inspect the transformed command tree directly.

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

🤖 Prompt for 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.

In `@src/cmd/completions.rs` around lines 441 - 461, Expand
completions_omit_help_subcommands to assert that representative nested help
paths such as numan registry help, numan snapshot help, and numan setup help are
absent for every CompletionShell, while preserving the existing top-level check.

Source: Coding guidelines

src/cmd/info.rs (1)

38-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use fork-specific wording in the footer and test it.

For numan-maintained packages, this branch prints not a verified upstream artifact, but format_info still appends the generic verified footer and refers to the upstream source. This mixes fork and upstream trust claims.

Emit a fork-specific footer or suppress the upstream footer for maintained forks. Add a test for the complete numan-maintained output.

This follows the intake requirement that maintained forks remain distinct from upstream.

Also applies to: 207-232

🤖 Prompt for 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.

In `@src/cmd/info.rs` around lines 38 - 42, Update format_info so numan-maintained
packages use fork-specific footer wording or omit the upstream verification
footer, avoiding references to verified upstream artifacts or “the upstream
source.” Preserve the existing upstream footer for other packages, and add
coverage asserting the complete numan-maintained output.

Source: MCP tools

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