Repository navigation
Conversation
Signed-off-by: Pybsama <294532706+Pybsama@users.noreply.github.com>
PR Review SummarySize
Affected crates
Blast radius — ModerateThis PR touches: source code,documentation Updated automatically on each push to this PR. |
| visited.push(base_name.clone()); | ||
|
|
||
| let resolved = load_base_profile_raw(base_name, context_dir, source_file)?; | ||
| let resolved = if from_cli && is_file_path_ref(base_name) { |
There was a problem hiding this comment.
Warning
🐛 [MEDIUM · bug] The check is_file_path_ref(base_name) only matches slashes / or .json / .jsonc extensions. On Windows, backslashes \ are standard path separators, but are not checked. Consequently, relative paths using backslashes (e.g., --extends .\overlay) will not be recognized as file paths, triggering profile-not-found errors. Update is_file_path_ref to also check for backslash characters \ or use std::path::is_separator to ensure cross-platform compatibility.
| .map(|name| (name, true)) | ||
| .chain(base_names.iter().map(|name| (name, false))) | ||
| { | ||
| if visited.contains(base_name) { |
There was a problem hiding this comment.
Note
🏗️ [LOW · design] The cycle detector checks visited.contains(base_name) using raw string comparisons on file paths. If a file is extended using different string representations (e.g. ./foo.json vs foo.json or an absolute path), the cycle check will fail to recognize the duplicate. While infinite recursion is ultimately bounded by MAX_INHERITANCE_DEPTH, canonicalizing file paths before inserting/comparing them in visited would provide robust cycle detection.
Linked Issue
Closes #1851
Summary
--extends ./overlay.jsonis currently rejected as an invalid profile name, although--profileaccepts the same file. Keep CLI-selected bases separate from JSON-authored inheritance and load CLI file arguments through the existing comment-aware, canonicalizing file parser. Their bases resolve before merging beneath the selected profile, preserving the existing left-to-right order.Paths remain forbidden in JSON
extends, including inside a CLI-selected file. Canonical sibling lookup, draft-directory isolation, pack provenance, defaults, and merge rules remain intact. Help and both authoring/flag references document file support and./for unambiguous local paths.Added name/path parity regressions for the reported multi-base case, including explicit null, empty credentials, and
open_urlsreplacement. They pass without changing merge semantics; this PR does not assert that the older-version discrepancy still exists on current main.Agent Disclosure
Prepared by OpenAI Codex, an AI coding agent, on behalf of Pybsama. Identity, intent, scope, and risks were disclosed in the issue before editing.
Consulted
AGENTS.md,CONTRIBUTING.md,SECURITY.md,GOVERNANCE.md,neps/README.md, the profile loader/resolver and tests, CLI definitions, and authoring/flag documentation. This contribution complies with the applicable repository requirements. No third-party implementation was reused. This is the scoped CLI input addition requested by the issue; no profile-format or policy-merge redesign is introduced.Test Plan
make cipassed locally on macOS: all-feature strict Clippy, formatting,cargo test --workspace(3,818 passed / 11 ignored, including 8 doc tests), the separate doc-test step (8 passed),cargo audit, and docs lint.git diff --checkpassed. Independent agent review found no actionable correctness or security issues. The subsequent upstream CI and SPIRE runs passed on this PR head, including the Linux/macOS test and integration jobs.Checklist
Agent Compliance Check
NonoErrorpropagation is retainedCompatibility scope
The file classifier is shared unchanged with
--profile; native Windows is not currently supported, and WSL2 uses the Linux build. File paths are accepted only in the top-level CLI base list. Recursive resolution passes an empty CLI list, so JSON-authored paths remain rejected. Repeating a file via separate CLI arguments creates sibling base layers, not a recursive cycle; named-cycle regression coverage remains in place.