[codex] Add manifest verified-load planning helper

[Fieldnote-Echo]

Summary

• add verify_for_load(manifest_path, VerifyOptions) and verify_document_for_load(&ManifestDocument, VerifyOptions) verifier-only helpers for Rust callers
• return captured canonical load-plan paths, probed metadata, row-identity summary, auxiliary artifact states, and the full verification report without calling ordvec loaders
• fail closed with VerifiedLoadPlanError::VerificationFailed(report) when verification reports errors, preserving report access for caller policy/logging
• document the mutable-path boundary: this plans verified loads but does not pin file descriptors or make shared storage immutable

Closes #146.

Stack / Scope

• Stacked on [codex] Add manifest auxiliary artifact verification #158 (codex/auxiliary-artifact-manifest) so auxiliary sidecar states are part of the plan surface.
• Depends on [codex] Bound manifest verifier resources #157/[codex] Add manifest auxiliary artifact verification #158 landing or this branch being rebased through their final shapes.
• Does not make ordvec-manifest publishable (manifest: publishable verifier crate and stable library API #143), expose programmatic cache APIs (manifest: expose programmatic verification cache API #147), unify report schemas (manifest: unify index, row identity, and auxiliary verification report #148), add signing/key/network policy, or call Rank::load / RankQuant::load / Bitmap::load / SignBitmap::load.

Validation

• cargo fmt --all --check
• cargo test -p ordvec-manifest --test manifest verify_for_load
• cargo test -p ordvec-manifest --test manifest
• cargo test -p ordvec-manifest --test manifest --features sqlite
• cargo check -p ordvec-manifest --no-default-features
• cargo check -p ordvec-manifest --all-features
• cargo clippy -p ordvec-manifest --all-targets --features sqlite -- -D warnings
• git diff --check

Adversarial Review

• Read-only scouts split issue scope, current manifest API, and trust-boundary risks before implementation.
• Initial adversarial diff review found a blocker: load-plan paths were rebuilt from lossy report strings. The implementation now carries non-UTF-8-safe PathBufs captured during verification into the plan, and verify_for_load_preserves_non_utf8_base_paths covers the regression.
• Bot review then flagged the remaining same-call re-resolution window and requested a public document-based helper. Commit 3f80ba2 addresses both by using verification-time path capture and exposing verify_document_for_load.
• Final targeted adversarial review found no blockers; residual mutable-path/TOCTOU risk after the plan is returned is explicitly documented as outside a planning helper that does not pin file descriptors.

[qodo-code-review]

Review Summary by Qodo

Add manifest verified-load planning helper for Rust callers

✨ Enhancement

Walkthroughs

Description

• Add verify_for_load() helper for Rust callers to verify manifests and obtain typed load plans
• Return canonical artifact paths, probed metadata, row-identity summary, and auxiliary artifact
  states
• Fail closed with VerifiedLoadPlanError::VerificationFailed(report) preserving full verification
  report
• Document mutable-path boundary: planning helper does not pin descriptors or make storage immutable

Diagram

flowchart LR
  A["manifest_path + VerifyOptions"] -->|verify_for_load| B["VerifiedLoadPlan"]
  B --> C["canonical artifact_path"]
  B --> D["metadata report"]
  B --> E["row_identity plan"]
  B --> F["auxiliary_artifacts plan"]
  B --> G["full verification report"]
  H["VerificationFailed"] -.->|fail closed| B

Loading

File Changes

1. ordvec-manifest/src/lib.rs ✨ Enhancement +335/-0

Implement verified load plan API and supporting types

• Add public verify_for_load() function that loads manifest and returns VerifiedLoadPlan
• Add internal verify_document_for_load() helper to construct plan from verification report
• Implement VerifiedLoadPlan struct with accessors for manifest path, artifact path, metadata, row
 identity, and auxiliary artifacts
• Implement VerifiedRowIdentityPlan struct capturing kind, path, row count, validated rows, and
 SHA256
• Implement VerifiedAuxiliaryArtifactPlan struct with name, path, state, and integrity metadata
• Add VerifiedLoadPlanError enum with variants for manifest errors, verification failures, and
 incomplete plans
• Add helper functions resolve_load_plan_path(), plan_resolution_message(), and
 report_issue_summary() for path resolution and error reporting

ordvec-manifest/src/lib.rs

---

2. ordvec-manifest/tests/manifest.rs 🧪 Tests +253/-4

Add comprehensive tests for verified load plan helper

• Add import for verify_for_load, VerifiedLoadPlanError, and ManifestIndexKind
• Add test verify_for_load_returns_resolved_plan_and_report() validating plan structure with
 auxiliary artifacts
• Add test verify_for_load_uses_explicit_index_override() verifying index override behavior
• Add test verify_for_load_returns_row_map_path_and_optional_absent_auxiliary() for JSONL row
 identity and optional sidecars
• Add Unix-specific test verify_for_load_preserves_non_utf8_base_paths() for non-UTF8 path
 handling
• Add test verify_for_load_fails_closed_with_report_for_default_path_policy() validating path
 escape rejection
• Add test verify_for_load_fails_closed_with_report_for_corrupted_artifact() validating corruption
 detection

ordvec-manifest/tests/manifest.rs

---

3. CHANGELOG.md 📝 Documentation +3/-0

Document verified load plan feature addition

• Document addition of VerifiedLoadPlan helper to ordvec-manifest for Rust callers
• Highlight ability to verify manifest, retain typed report, and load from resolved paths without
 re-resolving

CHANGELOG.md

---

View more (2)

4. docs/INDEX_PROVENANCE.md 📝 Documentation +9/-2

Document verified load plan API and usage patterns

• Add section documenting verify_for_load() API for Rust callers
• Explain plan contents: canonical paths, metadata, row-identity summary, auxiliary states, and full
 report
• Clarify boundary: helper does not call loaders, pin descriptors, or make storage immutable
• Update recipe guidance to mention plan helper as alternative to verifier

docs/INDEX_PROVENANCE.md

---

5. ordvec-manifest/README.md 📝 Documentation +14/-2

Document library API for verified load planning

• Add library usage section for verify_for_load() and VerifiedLoadPlan
• Explain fail-closed behavior and plan contents
• Clarify that helper does not call ordvec loaders or manage file descriptors
• Update artifact hashing documentation to include auxiliary artifacts

ordvec-manifest/README.md

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🔗 Cross-repo conflicts (0)

Context used

✅ Tickets: 🎫 manifest: publishable verifier crate and stable library API 🎫 manifest: add verified-load planning helper for callers 🎫 manifest: unify index, row identity, and auxiliary verification report +1 more

1. Re-resolved plan paths ✓ Resolved 🐞 Bug ⛨ Security

Description

verify_for_load verifies artifact bytes (hash/probe) and then VerifiedLoadPlan::from_report
re-runs path resolution (fs::canonicalize) to build the load plan, so a symlink/directory-entry
change between these steps can make the returned artifact_path/sidecar paths refer to different
files than were verified. This undermines the verify-then-load guarantee by enabling a same-call
path substitution attack even when report.ok is true.

Code

ordvec-manifest/src/lib.rs[R1941-1952]

+        let artifact_source_path = options
+            .index_override
+            .as_deref()
+            .unwrap_or_else(|| Path::new(&document.manifest.artifact.path));
+        let artifact_path = resolve_load_plan_path(
+            document,
+            options,
+            &report,
+            artifact_source_path,
+            "artifact",
+            "verified artifact path could not be resolved for load planning",
+        )?;

Evidence

The verifier hashes/probes a path resolved by resolve_existing_path, but the load plan re-resolves
paths again via resolve_load_plan_path → resolve_existing_path (another fs::canonicalize).
Because fs::canonicalize can yield a different target if symlinks/entries change, the plan can
return a different file path than the one whose bytes were verified, despite report.ok being true.

ordvec-manifest/src/lib.rs[176-237]
ordvec-manifest/src/lib.rs[1921-1967]
ordvec-manifest/src/lib.rs[2198-2213]
ordvec-manifest/src/lib.rs[1560-1629]

Agent prompt

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

## Issue description
`verify_for_load()` currently verifies files (hash/probe) and then constructs `VerifiedLoadPlan` by resolving paths again. Because `resolve_existing_path()` uses `fs::canonicalize()` (follows symlinks) each time, a symlink or directory entry can be swapped between verification and plan construction in the *same call*, causing the plan to return paths that were not the ones verified.

## Issue Context
- `verify_manifest()` resolves a path and hashes/probes `resolved.resolved_path`.
- `VerifiedLoadPlan::from_report()` later calls `resolve_load_plan_path()`, which calls `resolve_existing_path()` again to produce the `PathBuf` returned to the caller.
- This is separate from the documented “between verification and load” mutability boundary; this is an avoidable extra TOCTOU window inside the helper itself.

## Fix approach
Refactor the verification path used by `verify_for_load` so the *exact resolved/canonical `PathBuf`s used for hashing/probing* are captured and reused to build the load plan (artifact, row identity JSONL if present, and verified auxiliary artifacts). Options:
1) Add an internal verifier variant that returns both `(VerificationReport, VerifiedResolvedPaths)` where `VerifiedResolvedPaths` contains `PathBuf`s (non-UTF8-safe) for artifact/row-identity/aux paths used.
2) Or refactor `verify_manifest` internals to accept optional pre-resolved `ResolvedPath`s and/or to expose them to the caller (keeping the serializable `VerificationReport` unchanged).
3) Ensure `VerifiedLoadPlan` construction does not call `fs::canonicalize` again on those paths.

## Fix Focus Areas
- ordvec-manifest/src/lib.rs[159-174]
- ordvec-manifest/src/lib.rs[176-237]
- ordvec-manifest/src/lib.rs[1921-1976]
- ordvec-manifest/src/lib.rs[2198-2213]
- ordvec-manifest/src/lib.rs[1560-1629]

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

---

Previous review results

Review updated until commit 9ae2ab4

Results up to commit 80e5313

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0)

1. Re-resolved plan paths ✓ Resolved 🐞 Bug ⛨ Security

Description

verify_for_load verifies artifact bytes (hash/probe) and then VerifiedLoadPlan::from_report
re-runs path resolution (fs::canonicalize) to build the load plan, so a symlink/directory-entry
change between these steps can make the returned artifact_path/sidecar paths refer to different
files than were verified. This undermines the verify-then-load guarantee by enabling a same-call
path substitution attack even when report.ok is true.

Code

ordvec-manifest/src/lib.rs[R1941-1952]

+        let artifact_source_path = options
+            .index_override
+            .as_deref()
+            .unwrap_or_else(|| Path::new(&document.manifest.artifact.path));
+        let artifact_path = resolve_load_plan_path(
+            document,
+            options,
+            &report,
+            artifact_source_path,
+            "artifact",
+            "verified artifact path could not be resolved for load planning",
+        )?;

Evidence

The verifier hashes/probes a path resolved by resolve_existing_path, but the load plan re-resolves
paths again via resolve_load_plan_path → resolve_existing_path (another fs::canonicalize).
Because fs::canonicalize can yield a different target if symlinks/entries change, the plan can
return a different file path than the one whose bytes were verified, despite report.ok being true.

ordvec-manifest/src/lib.rs[176-237]
ordvec-manifest/src/lib.rs[1921-1967]
ordvec-manifest/src/lib.rs[2198-2213]
ordvec-manifest/src/lib.rs[1560-1629]

Agent prompt

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

## Issue description
`verify_for_load()` currently verifies files (hash/probe) and then constructs `VerifiedLoadPlan` by resolving paths again. Because `resolve_existing_path()` uses `fs::canonicalize()` (follows symlinks) each time, a symlink or directory entry can be swapped between verification and plan construction in the *same call*, causing the plan to return paths that were not the ones verified.

## Issue Context
- `verify_manifest()` resolves a path and hashes/probes `resolved.resolved_path`.
- `VerifiedLoadPlan::from_report()` later calls `resolve_load_plan_path()`, which calls `resolve_existing_path()` again to produce the `PathBuf` returned to the caller.
- This is separate from the documented “between verification and load” mutability boundary; this is an avoidable extra TOCTOU window inside the helper itself.

## Fix approach
Refactor the verification path used by `verify_for_load` so the *exact resolved/canonical `PathBuf`s used for hashing/probing* are captured and reused to build the load plan (artifact, row identity JSONL if present, and verified auxiliary artifacts). Options:
1) Add an internal verifier variant that returns both `(VerificationReport, VerifiedResolvedPaths)` where `VerifiedResolvedPaths` contains `PathBuf`s (non-UTF8-safe) for artifact/row-identity/aux paths used.
2) Or refactor `verify_manifest` internals to accept optional pre-resolved `ResolvedPath`s and/or to expose them to the caller (keeping the serializable `VerificationReport` unchanged).
3) Ensure `VerifiedLoadPlan` construction does not call `fs::canonicalize` again on those paths.

## Fix Focus Areas
- ordvec-manifest/src/lib.rs[159-174]
- ordvec-manifest/src/lib.rs[176-237]
- ordvec-manifest/src/lib.rs[1921-1976]
- ordvec-manifest/src/lib.rs[2198-2213]
- ordvec-manifest/src/lib.rs[1560-1629]

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

---

[gemini-code-assist]

Code Review

This pull request introduces a new verifier-only helper VerifiedLoadPlan and a corresponding verify_for_load function to ordvec-manifest. This allows Rust callers to verify a manifest, retain a typed report, and load from resolved artifact and sidecar paths without re-resolving manifest strings. The changes also include comprehensive documentation updates and unit tests. The reviewer suggested making the internal helper function verify_document_for_load public to allow library callers with in-memory ManifestDocuments to plan verified loads directly without disk I/O.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 80e531316c

ℹ️ 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".

[Fieldnote-Echo]

Rebased #163 onto current main after #160 merged and preserved the earlier verified-load review fix that reuses captured resolved paths instead of re-resolving during plan construction.

Current branch commits on top of main:

• d1e24c4 Add manifest verified load plan helper
• ba5d170 Address verified load plan review findings
• 9ae2ab4 Ignore local ordgrep store

Validation on rebased head 9ae2ab4:

• cargo fmt --all --check
• git diff --check origin/main...HEAD
• cargo test -p ordvec-manifest
• cargo test -p ordvec-manifest --features sqlite
• cargo check -p ordvec-manifest --no-default-features

Before the rebase, Qodo's only previous finding (Re-resolved plan paths) was marked resolved and there were no unresolved review threads.

[Fieldnote-Echo]

/agentic_review

[qodo-code-review]

Code review by qodo was updated up to the latest commit 9ae2ab4