Skip to content

Commit 3d5664f

Browse files
authored
Merge pull request #3 from NodeDB-Lab/fix/implicit-root-and-omission-cap
fix(cli): refuse an implicit home root and cap omissions
2 parents e7c9501 + 2fe6b13 commit 3d5664f

11 files changed

Lines changed: 506 additions & 37 deletions

File tree

‎.github/workflows/test.yml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ jobs:
138138
set -euo pipefail
139139
INSTALL_ROOT="$(mktemp -d)"
140140
cargo install --path cli --root "$INSTALL_ROOT"
141-
"$INSTALL_ROOT/bin/code2graph" --help
141+
for CLI_BIN in "$INSTALL_ROOT"/bin/*; do
142+
"$CLI_BIN" --help
143+
done
142144
143145
bindings:
144146
if: ${{ !inputs.skip_bindings }}

‎README.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,11 @@ c2g callers helper
111111
c2g impact helper --depth 3
112112
```
113113

114-
By default the CLI rejects an incomplete index. `--allow-partial` explicitly permits
115-
publishing and querying a partial source set; inspect the reported omissions before
116-
relying on its results.
114+
By default the CLI rejects an incomplete index. `--allow-partial` explicitly permits publishing and querying a partial source set. Inspect the reported omissions before relying on its results. Each reported omission list is capped at 256 entries. Project metadata retains the full count in `omittedFiles` and full reason totals in `omissionReasons`. Index results use `omitted_files` and `omission_reasons`. Status inventory retains the full count in `inventory.omitted_files`. The `omissionsTruncated` project flag and `omissions_truncated` index flag identify capped lists.
115+
116+
Without `--root`, the selected project is the working directory. A working directory
117+
that is a home directory or the filesystem root is refused: walking one costs minutes
118+
and describes no project. Name the project (`--root <DIR>`) to proceed.
117119

118120
Driving the CLI from a coding agent: [`docs/agent-integration.md`](docs/agent-integration.md) carries a copy-pasteable rule block for `CLAUDE.md` / `AGENTS.md` and explains why a mechanical trigger is the only kind an agent reliably follows.
119121

‎cli/src/config.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,13 @@ pub const DEFAULT_MAX_TOTAL_BYTES: usize = 256 * 1_024 * 1_024;
9595
pub const DEFAULT_MAX_DEPTH: u32 = 32;
9696
/// Default number of rows rendered by a command.
9797
pub const DEFAULT_LIMIT: usize = 50;
98+
/// Default maximum number of individual omission entries reported in any one
99+
/// list. An over-broad root (a home directory, a parent of many repositories)
100+
/// omits tens of thousands of files, and a JSON envelope carrying one entry per
101+
/// omitted file grows to megabytes. The entry lists are diagnostics: each list
102+
/// is capped to this many entries while the totals (`omittedFiles`,
103+
/// `inventory.omitted_files`) and the rendered reason counts stay complete.
104+
pub const DEFAULT_MAX_OMISSIONS: usize = 256;
98105
/// Default reverse-reachability depth for `impact`.
99106
pub const DEFAULT_IMPACT_DEPTH: u32 = 2;
100107

‎cli/src/execution/lifecycle.rs‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1878,13 +1878,15 @@ fn graph_from_snapshot(
18781878
})
18791879
}
18801880

1881-
fn project_output(
1881+
pub(super) fn project_output(
18821882
selection: &crate::ProjectSelection,
18831883
snapshot: &LoadedSnapshot,
18841884
tier: crate::ResolverTier,
18851885
freshness: Freshness,
18861886
cache: CacheDisposition,
18871887
) -> ProjectOutput {
1888+
let omission_reasons = crate::result::cache_omission_reasons(&snapshot.omissions);
1889+
let (omissions, omissions_truncated) = crate::result::capped_omissions(&snapshot.omissions);
18881890
ProjectOutput {
18891891
root: selection.canonical_root.to_string_lossy().into_owned(),
18901892
snapshot: snapshot.candidate_id.to_string(),
@@ -1893,7 +1895,9 @@ fn project_output(
18931895
cache,
18941896
completeness: snapshot.completeness.into(),
18951897
omitted_files: snapshot.omissions.len(),
1896-
omissions: snapshot.omissions.iter().map(Into::into).collect(),
1898+
omissions,
1899+
omission_reasons,
1900+
omissions_truncated,
18971901
// Only the paths that actually refreshed against a store can observe a
18981902
// recovery; they fill this in from the store afterwards.
18991903
cache_recovery: None,
@@ -2675,11 +2679,11 @@ mod tests {
26752679

26762680
// One project root disappears; one cache is left on an older schema.
26772681
fs::remove_dir_all(temp.path().join("deleted")).expect("remove project");
2678-
let stale_key = crate::cache::CacheLocation::for_project(
2679-
Some(cache.as_path()),
2680-
&temp.path().join("stale"),
2681-
)
2682-
.expect("stale location");
2682+
let stale_root =
2683+
fs::canonicalize(temp.path().join(".").join("stale")).expect("canonical stale project");
2684+
let stale_key =
2685+
crate::cache::CacheLocation::for_project(Some(cache.as_path()), &stale_root)
2686+
.expect("stale location");
26832687
let connection =
26842688
rusqlite::Connection::open(&stale_key.database_path).expect("open stale cache");
26852689
connection
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
use crate::cache::{
4+
CacheCompleteness, CacheOmission, CandidateId, CompatibilityFingerprint, CompatibilityRecord,
5+
LanguageFeatureFingerprint, LoadedSnapshot, PackageFingerprint, ProjectInputDigest,
6+
};
7+
use crate::config::{DEFAULT_MAX_OMISSIONS, ResourceLimits};
8+
use crate::result::{CacheReasonCountOutput, IndexOutput, PlanDecisionCountsOutput};
9+
use crate::{
10+
CacheDisposition, Freshness, OutputEnvelope, OutputStatus, ProjectSelection, ResolverTier,
11+
SelectionProvenance, StatusOutput,
12+
};
13+
14+
use super::super::lifecycle::{CommandOutput, project_output};
15+
use super::render_human;
16+
17+
fn mixed_snapshot() -> LoadedSnapshot {
18+
let omissions = (0..265)
19+
.rev()
20+
.map(|index| CacheOmission {
21+
path: format!("src/file{index:04}.rs"),
22+
reason: match index {
23+
0..250 => "file-count-limit",
24+
250..260 => "file-too-large",
25+
_ => "read-error:other",
26+
}
27+
.into(),
28+
detail: "resource limit".into(),
29+
})
30+
.collect::<Vec<_>>();
31+
let language = LanguageFeatureFingerprint::current();
32+
let package = PackageFingerprint::from_normalized(["test"]);
33+
let compatibility = CompatibilityFingerprint::new(language, package);
34+
let digest = ProjectInputDigest::from_inputs([] as [(&str, &str, [u8; 32]); 0]);
35+
LoadedSnapshot {
36+
candidate_id: CandidateId::new(
37+
compatibility,
38+
digest,
39+
CacheCompleteness::Partial,
40+
&omissions,
41+
),
42+
compatibility: CompatibilityRecord {
43+
id: compatibility,
44+
language_fingerprint: language,
45+
package_fingerprint: package,
46+
created_at_ns: 1,
47+
},
48+
input_digest: digest,
49+
completeness: CacheCompleteness::Partial,
50+
omissions,
51+
created_at_ns: 2,
52+
inventory_file_count: 3,
53+
inventory_total_bytes: 42,
54+
files: Vec::new(),
55+
tier_graphs: Vec::new(),
56+
}
57+
}
58+
59+
#[test]
60+
fn index_and_cached_status_count_reasons_outside_capped_entries() {
61+
let snapshot = mixed_snapshot();
62+
let expected = vec![
63+
CacheReasonCountOutput {
64+
reason: "file-count-limit".into(),
65+
count: 250,
66+
},
67+
CacheReasonCountOutput {
68+
reason: "file-too-large".into(),
69+
count: 10,
70+
},
71+
CacheReasonCountOutput {
72+
reason: "read-error:other".into(),
73+
count: 5,
74+
},
75+
];
76+
let index = IndexOutput::from_loaded_snapshot(
77+
&snapshot,
78+
ResolverTier::Scope,
79+
0,
80+
0,
81+
0,
82+
1,
83+
PlanDecisionCountsOutput::default(),
84+
);
85+
let selection = ProjectSelection {
86+
canonical_root: "/project".into(),
87+
canonical_source: None,
88+
provenance: SelectionProvenance::RootArgument,
89+
};
90+
let project = project_output(
91+
&selection,
92+
&snapshot,
93+
ResolverTier::Scope,
94+
Freshness::Frozen,
95+
CacheDisposition::Hit,
96+
);
97+
assert_eq!(index.omission_reasons, expected);
98+
assert_eq!(project.omission_reasons, expected);
99+
assert_eq!(index.omissions.len(), DEFAULT_MAX_OMISSIONS);
100+
assert_eq!(index.omissions, project.omissions);
101+
assert_eq!(index.omitted_files, 265);
102+
assert_eq!(project.omitted_files, 265);
103+
assert!(index.omissions_truncated);
104+
assert!(project.omissions_truncated);
105+
assert!(
106+
index
107+
.omissions
108+
.iter()
109+
.all(|entry| entry.reason != "read-error:other")
110+
);
111+
assert_eq!(index.omissions[0].path, "src/file0000.rs");
112+
assert_eq!(index.omissions[255].path, "src/file0255.rs");
113+
114+
let status = StatusOutput::from_loaded_snapshot(project, &snapshot, &ResourceLimits::default());
115+
assert_eq!(status.cached_omissions, index.omissions);
116+
assert_eq!(status.project.omission_reasons, expected);
117+
assert_eq!(status.inventory.omitted_files, 265);
118+
let expected_json = serde_json::json!([
119+
{"reason": "file-count-limit", "count": 250},
120+
{"reason": "file-too-large", "count": 10},
121+
{"reason": "read-error:other", "count": 5},
122+
]);
123+
let mut index_json = serde_json::to_value(&index).expect("index JSON");
124+
let mut project_json = serde_json::to_value(&status.project).expect("project JSON");
125+
assert_eq!(index_json["omission_reasons"], expected_json);
126+
assert_eq!(project_json["omissionReasons"], expected_json);
127+
index_json
128+
.as_object_mut()
129+
.expect("index object")
130+
.remove("omission_reasons");
131+
project_json
132+
.as_object_mut()
133+
.expect("project object")
134+
.remove("omissionReasons");
135+
assert!(
136+
serde_json::from_value::<IndexOutput>(index_json)
137+
.expect("older index contract")
138+
.omission_reasons
139+
.is_empty()
140+
);
141+
assert!(
142+
serde_json::from_value::<crate::ProjectOutput>(project_json)
143+
.expect("older project contract")
144+
.omission_reasons
145+
.is_empty()
146+
);
147+
148+
for output in [
149+
CommandOutput::Index(OutputEnvelope::new(OutputStatus::Partial, index)),
150+
CommandOutput::Status(OutputEnvelope::new(OutputStatus::Partial, status)),
151+
] {
152+
let rendered = render_human(&output);
153+
assert!(rendered.contains("warning: omission entries truncated; listing 256 of 265\n"));
154+
let reasons = rendered
155+
.lines()
156+
.filter(|line| line.starts_with("omission reason="))
157+
.collect::<Vec<_>>();
158+
assert_eq!(
159+
reasons,
160+
[
161+
"omission reason=file-count-limit count=250",
162+
"omission reason=file-too-large count=10",
163+
"omission reason=read-error:other count=5",
164+
]
165+
);
166+
assert_eq!(
167+
rendered
168+
.lines()
169+
.filter(|line| line.starts_with("omitted src/"))
170+
.count(),
171+
DEFAULT_MAX_OMISSIONS
172+
);
173+
assert!(!rendered.contains("omitted src/file0260.rs"));
174+
}
175+
}

‎cli/src/execution/output.rs‎

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,13 @@ fn query_warning(project: Option<&ProjectOutput>) -> String {
7070
"warning: partial snapshot; {} source files omitted\n",
7171
project.omitted_files
7272
));
73+
if project.omissions_truncated {
74+
output.push_str(&format!(
75+
"warning: omission entries truncated; listing {} of {}\n",
76+
project.omissions.len(),
77+
project.omitted_files
78+
));
79+
}
7380
for omission in sorted_omissions(&project.omissions) {
7481
output.push_str(&format!(
7582
"warning: omitted {} reason={} detail={}\n",
@@ -107,15 +114,21 @@ fn render_index(envelope: &crate::OutputEnvelope<crate::IndexOutput>) -> String
107114
}
108115
output.push_str(&format!(
109116
"omitted files={}\n",
110-
envelope.results.omissions.len()
117+
envelope.results.omitted_files
111118
));
112-
let omissions = sorted_omissions(&envelope.results.omissions);
113-
let mut counts = std::collections::BTreeMap::<&str, usize>::new();
114-
for omission in &omissions {
115-
*counts.entry(&omission.reason).or_default() += 1;
119+
if envelope.results.omissions_truncated {
120+
output.push_str(&format!(
121+
"warning: omission entries truncated; listing {} of {}\n",
122+
envelope.results.omissions.len(),
123+
envelope.results.omitted_files
124+
));
116125
}
117-
for (reason, count) in counts {
118-
output.push_str(&format!("omission reason={} count={}\n", reason, count));
126+
let omissions = sorted_omissions(&envelope.results.omissions);
127+
for reason in &envelope.results.omission_reasons {
128+
output.push_str(&format!(
129+
"omission reason={} count={}\n",
130+
reason.reason, reason.count
131+
));
119132
}
120133
for omission in omissions {
121134
output.push_str(&format!(
@@ -153,13 +166,19 @@ fn render_status(status: &crate::StatusOutput) -> String {
153166
.timeout_millis
154167
.map_or_else(|| "none".into(), |value| value.to_string()),
155168
);
156-
let mut counts = std::collections::BTreeMap::<&str, usize>::new();
157-
let omissions = sorted_omissions(&status.project.omissions);
158-
for omission in &omissions {
159-
*counts.entry(&omission.reason).or_default() += 1;
169+
if status.project.omissions_truncated {
170+
output.push_str(&format!(
171+
"warning: omission entries truncated; listing {} of {}\n",
172+
status.project.omissions.len(),
173+
status.project.omitted_files
174+
));
160175
}
161-
for (reason, count) in counts {
162-
output.push_str(&format!("omission reason={} count={}\n", reason, count));
176+
let omissions = sorted_omissions(&status.project.omissions);
177+
for reason in &status.project.omission_reasons {
178+
output.push_str(&format!(
179+
"omission reason={} count={}\n",
180+
reason.reason, reason.count
181+
));
163182
}
164183
for omission in omissions {
165184
output.push_str(&format!(
@@ -547,6 +566,17 @@ mod tests {
547566
detail: "limit=12".into(),
548567
},
549568
],
569+
omission_reasons: vec![
570+
crate::result::CacheReasonCountOutput {
571+
reason: "file-too-large".into(),
572+
count: 1,
573+
},
574+
crate::result::CacheReasonCountOutput {
575+
reason: "read-error:other".into(),
576+
count: 1,
577+
},
578+
],
579+
omissions_truncated: false,
550580
cache_recovery: None,
551581
}
552582
}
@@ -595,6 +625,10 @@ mod tests {
595625
inventory_file_count: 3,
596626
inventory_total_bytes: 42,
597627
omissions: project(Freshness::Fresh, CacheCompletenessOutput::Partial).omissions,
628+
omitted_files: 2,
629+
omission_reasons: project(Freshness::Fresh, CacheCompletenessOutput::Partial)
630+
.omission_reasons,
631+
omissions_truncated: false,
598632
changed: 2,
599633
deleted: 1,
600634
ignored_omissions: 0,
@@ -617,6 +651,7 @@ mod tests {
617651
let mut project = project(Freshness::Fresh, CacheCompletenessOutput::Complete);
618652
project.omitted_files = 0;
619653
project.omissions = Vec::new();
654+
project.omission_reasons = Vec::new();
620655
project.cache_recovery = Some(detail.into());
621656

622657
let mut envelope = OutputEnvelope::new(
@@ -629,6 +664,9 @@ mod tests {
629664
inventory_file_count: 1,
630665
inventory_total_bytes: 42,
631666
omissions: Vec::new(),
667+
omitted_files: 0,
668+
omission_reasons: Vec::new(),
669+
omissions_truncated: false,
632670
changed: 1,
633671
deleted: 0,
634672
ignored_omissions: 0,
@@ -703,3 +741,7 @@ mod tests {
703741
assert_eq!(render_human(&CommandOutput::Impact(impact)), expected);
704742
}
705743
}
744+
745+
#[cfg(test)]
746+
#[path = "omission_output_tests.rs"]
747+
mod omission_output_tests;

0 commit comments

Comments
 (0)