Skip to content

Commit ef22e4d

Browse files
committed
fix: address v2.4 setup review findings
1 parent 3116fb5 commit ef22e4d

9 files changed

Lines changed: 152 additions & 78 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ dist/
2828
# Local-only planning docs (superpowers plans, drafts, scratch)
2929
docs/superpowers/
3030

31+
# Local release-note drafts
32+
release-notes/
33+
3134
# Local-only Python private corpus benchmark inputs/results
3235
benchmark/python/private-corpus.local.yaml
3336
benchmark/python/private-results/

crates/gather-step-cli/src/commands/generate.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,9 +198,10 @@ pub fn run_summary_pair(app: &AppContext) -> Result<()> {
198198
},
199199
)?;
200200
} else {
201-
output.line("warning: skipped .claude/rules/ generation because no workspace index exists");
201+
output
202+
.line("Warning: Skipped generating .claude/rules/ because no workspace index exists.");
202203
output.line(
203-
"hint: run `gather-step index`, then `gather-step generate claude-md --target rules`",
204+
"Hint: Run `gather-step index`, then `gather-step generate claude-md --target rules`.",
204205
);
205206
}
206207
if let Some(bar) = &generation_bar {

crates/gather-step-cli/src/commands/index.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ pub async fn run(app: &AppContext, args: IndexArgs) -> Result<()> {
356356
workspace = %app.workspace_path.display(),
357357
config = %config_path.display(),
358358
repos = config.repos.len(),
359-
"indexing from directory started",
359+
"Indexing from directory started.",
360360
);
361361

362362
let mut repo_results = Vec::with_capacity(config.repos.len());
@@ -504,7 +504,7 @@ pub async fn run(app: &AppContext, args: IndexArgs) -> Result<()> {
504504
files = authoritative_files,
505505
symbols = authoritative_symbols,
506506
edges = authoritative_edges,
507-
"indexing from directory finished",
507+
"Indexing from directory finished.",
508508
);
509509

510510
let depth_level = config_ref
@@ -560,7 +560,7 @@ pub async fn run(app: &AppContext, args: IndexArgs) -> Result<()> {
560560
info!(
561561
repo = %repo.name,
562562
path = %repo_root.display(),
563-
"indexing from directory started",
563+
"Indexing from directory started.",
564564
);
565565
let prepare_start = Instant::now();
566566
let payload = indexer_ref
@@ -842,14 +842,20 @@ pub async fn run(app: &AppContext, args: IndexArgs) -> Result<()> {
842842
storage_root = %storage_root.display(),
843843
duration_ms = total_wall_ms,
844844
index_size_bytes,
845-
"indexing from directory finished",
845+
"Indexing from directory finished.",
846846
);
847847

848848
output.emit(&payload)?;
849+
let repo_label = if payload.stats.indexed_repos == 1 {
850+
"repository"
851+
} else {
852+
"repositories"
853+
};
849854
output.line(format!(
850-
"\n {} {} repo(s) {}",
855+
"\n {} {} {} {}",
851856
style("✓ Indexed").green().bold(),
852857
style(payload.stats.indexed_repos).cyan(),
858+
repo_label,
853859
style(&payload.storage_root).dim()
854860
));
855861
output.line(format!(
@@ -865,7 +871,7 @@ pub async fn run(app: &AppContext, args: IndexArgs) -> Result<()> {
865871
style(format_bytes(payload.index_size_bytes)).dim(),
866872
));
867873
for warning in &payload.warnings {
868-
output.line(format!(" {} {warning}", style("warning:").yellow().bold()));
874+
output.line(format!(" {} {warning}", style("Warning:").yellow().bold()));
869875
}
870876

871877
// When `--artifact-path` is set, persist the IndexOutput payload so release

crates/gather-step-cli/src/commands/init.rs

Lines changed: 128 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,9 @@ async fn run_wizard(app: &AppContext, args: InitArgs) -> Result<()> {
139139
));
140140
}
141141
output.line(format!(
142-
"\n Found {} git repo(s)",
142+
"\n Found {} {}",
143143
style(repos.len()).cyan().bold(),
144+
style(git_repository_count_label(repos.len())).dim(),
144145
));
145146
let selected_repos = prompt_repo_selection(1, &repos, existing_config_repos.as_deref())?;
146147

@@ -171,7 +172,11 @@ async fn run_wizard(app: &AppContext, args: InitArgs) -> Result<()> {
171172
} else if args.no_watch {
172173
false
173174
} else {
174-
prompt_yes_no(5, "Enable auto-reindex on repository changes?", false)?
175+
prompt_yes_no(
176+
5,
177+
"Watch for repository changes and re-index automatically?",
178+
false,
179+
)?
175180
};
176181

177182
write_default_config_with_repos(app, &args, &selected_repos, existing_config.as_ref())?;
@@ -222,15 +227,15 @@ fn write_default_config_with_repos(
222227

223228
if repos.is_empty() {
224229
bail!(
225-
"No git repositories found under {}",
230+
"No Git repositories found under {}.",
226231
app.workspace_path.display()
227232
);
228233
}
229234

230235
let configured_repos = materialize_repo_config(repos, existing_config);
231236
let config = match existing_config {
232237
Some(existing) => GatherStepConfig {
233-
allow_listed_repos: existing.allow_listed_repos.clone(),
238+
allow_listed_repos: retain_allow_listed_repos_for_selected(existing, &configured_repos),
234239
repos: configured_repos,
235240
github: existing.github.clone(),
236241
jira: existing.jira.clone(),
@@ -273,21 +278,41 @@ fn materialize_repo_config(
273278
repos
274279
.iter()
275280
.map(|repo| {
276-
existing_by_path
277-
.get(repo.relative_path.as_str())
278-
.or_else(|| existing_by_name.get(repo.name.as_str()))
279-
.map_or_else(
280-
|| RepoConfig {
281-
name: repo.name.clone(),
282-
path: repo.relative_path.clone(),
283-
depth: None,
284-
},
285-
|existing| (*existing).clone(),
286-
)
281+
if let Some(existing) = existing_by_path.get(repo.relative_path.as_str()) {
282+
return (*existing).clone();
283+
}
284+
if let Some(existing) = existing_by_name.get(repo.name.as_str()) {
285+
return RepoConfig {
286+
name: existing.name.clone(),
287+
path: repo.relative_path.clone(),
288+
depth: existing.depth,
289+
};
290+
}
291+
RepoConfig {
292+
name: repo.name.clone(),
293+
path: repo.relative_path.clone(),
294+
depth: None,
295+
}
287296
})
288297
.collect()
289298
}
290299

300+
fn retain_allow_listed_repos_for_selected(
301+
existing_config: &GatherStepConfig,
302+
selected_repos: &[RepoConfig],
303+
) -> Vec<String> {
304+
let selected_names = selected_repos
305+
.iter()
306+
.map(|repo| repo.name.as_str())
307+
.collect::<BTreeSet<_>>();
308+
existing_config
309+
.allow_listed_repos
310+
.iter()
311+
.filter(|repo_name| selected_names.contains(repo_name.as_str()))
312+
.cloned()
313+
.collect()
314+
}
315+
291316
fn emit_config_summary(
292317
app: &AppContext,
293318
config_path: &Path,
@@ -317,7 +342,7 @@ fn emit_config_summary(
317342
output.line(format!(
318343
" {} {}",
319344
style(payload.repo_count).cyan().bold(),
320-
style("configured repository(ies)").dim()
345+
style(repository_count_label(payload.repo_count)).dim()
321346
));
322347
for repo in payload.repos {
323348
output.line(format!(
@@ -332,9 +357,9 @@ fn emit_config_summary(
332357

333358
fn emit_setup_complete(output: &crate::app::Output) {
334359
output.line(format!(
335-
"\n {} Gather Step is ready. Start planning with your agent, for example: {}. Docs: {}",
360+
"\n {} Gather Step is ready. Start planning with your agent. Example: {}. Docs: {}",
336361
style("✓ Setup complete.").green().bold(),
337-
style("\"Start planning for Task A, use gather-step\"").cyan(),
362+
style("\"Start planning your next task with gather-step\"").cyan(),
338363
style("https://gatherstep.dev/reference/mcp-tools/").underlined()
339364
));
340365
}
@@ -361,6 +386,22 @@ fn discovered_repos_from_config(config: &GatherStepConfig) -> Vec<DiscoveredRepo
361386
.collect()
362387
}
363388

389+
fn repository_count_label(count: usize) -> &'static str {
390+
if count == 1 {
391+
"configured repository"
392+
} else {
393+
"configured repositories"
394+
}
395+
}
396+
397+
fn git_repository_count_label(count: usize) -> &'static str {
398+
if count == 1 {
399+
"Git repository"
400+
} else {
401+
"Git repositories"
402+
}
403+
}
404+
364405
fn prompt_yes_no(step: usize, message: &str, default: bool) -> Result<bool> {
365406
let suffix = if default { "[Y/n]" } else { "[y/N]" };
366407
let mut stdout = io::stdout().lock();
@@ -412,7 +453,7 @@ fn prompt_repo_selection(
412453
existing_config_repos: Option<&[DiscoveredRepo]>,
413454
) -> Result<Vec<DiscoveredRepo>> {
414455
if repos.is_empty() {
415-
bail!("No git repositories found under the workspace");
456+
bail!("No Git repositories found under the workspace.");
416457
}
417458

418459
let default_names = existing_config_repos.map(|repos| {
@@ -502,7 +543,7 @@ fn prompt_repo_selection(
502543
}
503544

504545
if selected.is_empty() {
505-
bail!("select at least one repository before continuing");
546+
bail!("Select at least one repository before continuing.");
506547
}
507548

508549
Ok(selected
@@ -707,7 +748,12 @@ mod tests {
707748

708749
use pretty_assertions::assert_eq;
709750

710-
use super::discover_git_repos;
751+
use gather_step_core::{DepthLevel, GatherStepConfig, IndexingConfig, RepoConfig};
752+
753+
use super::{
754+
DiscoveredRepo, discover_git_repos, materialize_repo_config,
755+
retain_allow_listed_repos_for_selected,
756+
};
711757

712758
static TEMP_COUNTER: AtomicU64 = AtomicU64::new(0);
713759

@@ -788,4 +834,65 @@ mod tests {
788834
]
789835
);
790836
}
837+
838+
#[test]
839+
fn repo_config_merge_filters_allow_list_to_selected_repos() {
840+
let existing = GatherStepConfig {
841+
allow_listed_repos: vec!["api".to_owned(), "web".to_owned()],
842+
repos: vec![
843+
RepoConfig {
844+
name: "api".to_owned(),
845+
path: "apps/api".to_owned(),
846+
depth: Some(DepthLevel::Level2),
847+
},
848+
RepoConfig {
849+
name: "web".to_owned(),
850+
path: "apps/web".to_owned(),
851+
depth: Some(DepthLevel::Level1),
852+
},
853+
],
854+
github: None,
855+
jira: None,
856+
indexing: IndexingConfig::default(),
857+
};
858+
let selected = vec![DiscoveredRepo {
859+
name: "api".to_owned(),
860+
relative_path: "apps/api".to_owned(),
861+
}];
862+
let merged = materialize_repo_config(&selected, Some(&existing));
863+
864+
assert_eq!(
865+
retain_allow_listed_repos_for_selected(&existing, &merged),
866+
vec!["api".to_owned()]
867+
);
868+
}
869+
870+
#[test]
871+
fn repo_config_name_match_keeps_discovered_path() {
872+
let existing = GatherStepConfig {
873+
allow_listed_repos: Vec::new(),
874+
repos: vec![RepoConfig {
875+
name: "api".to_owned(),
876+
path: "old/api".to_owned(),
877+
depth: Some(DepthLevel::Level2),
878+
}],
879+
github: None,
880+
jira: None,
881+
indexing: IndexingConfig::default(),
882+
};
883+
let selected = vec![DiscoveredRepo {
884+
name: "api".to_owned(),
885+
relative_path: "new/api".to_owned(),
886+
}];
887+
let merged = materialize_repo_config(&selected, Some(&existing));
888+
889+
assert_eq!(
890+
merged,
891+
vec![RepoConfig {
892+
name: "api".to_owned(),
893+
path: "new/api".to_owned(),
894+
depth: Some(DepthLevel::Level2),
895+
}]
896+
);
897+
}
791898
}

crates/gather-step-cli/src/commands/tui.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ pub fn run(app: &AppContext, args: TuiArgs) -> Result<()> {
106106
pub fn run_with_options(app: &AppContext, args: TuiArgs) -> Result<()> {
107107
if !app.tui_is_available() {
108108
bail!(
109-
"TUI requires an interactive terminal\nhint: run `gather-step status`, `gather-step watch`, or pass `--json` for scriptable output"
109+
"TUI requires an interactive terminal.\nHint: Run `gather-step status`, `gather-step watch`, or pass `--json` for scriptable output."
110110
);
111111
}
112112

crates/gather-step-cli/tests/cli_init.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ fn init_generate_ai_files_without_index_writes_summaries_and_skips_rules() {
9090
String::from_utf8_lossy(&output.stderr)
9191
);
9292
let stdout = String::from_utf8_lossy(&output.stdout);
93-
assert!(stdout.contains("warning: skipped .claude/rules/ generation"));
93+
assert!(stdout.contains("Warning: Skipped generating .claude/rules/"));
9494
assert!(tmp.path().join("CLAUDE.gather.md").exists());
9595
assert!(tmp.path().join("AGENTS.gather.md").exists());
9696
assert!(!tmp.path().join(".claude/rules").exists());

crates/gather-step-parser/src/tree_sitter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3037,7 +3037,7 @@ fn resolve_python_sibling_package_import(
30373037
package = package_name,
30383038
first_package_dir = %first_package_dir.display(),
30393039
duplicate_package_dir = %package_dir.display(),
3040-
"multiple sibling Python packages matched import; leaving import unresolved"
3040+
"Multiple sibling Python packages matched import; leaving import unresolved."
30413041
);
30423042
return None;
30433043
}

release-notes/v2.4.0.md

Lines changed: 0 additions & 43 deletions
This file was deleted.

0 commit comments

Comments
 (0)