fix: thread recursive option from plugin protocol to worktree create - #25
Conversation
Re-adds notify-parent.yml to trigger parent meta repo's release-please when this child repo merges to main. Simplified: no checkout needed, all payload fields properly quoted with toJSON. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The host meta CLI strips --recursive from command args and passes it as a plugin protocol option. The worktree create handler was reading recursive from clap-parsed args (always false) instead of the protocol option. Inject --recursive back into clap args when the protocol option is set, so build_nested_dep_graph is invoked and transitive depends_on entries (e.g. core -> vendor/tree-sitter-markdown) are resolved correctly. Implements [[tasks/harmony-407]] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a GitHub Actions workflow that notifies a parent repo on pushes to main, and extends the worktree command to accept a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Dev (push)
participant GH as GitHub Actions
participant Action as repository-dispatch Action
participant Parent as Parent Repo (meta)
Dev->>GH: push to main
GH->>Action: run repository-dispatch with event `child-repo-updated` and payload (repo, sha, actor)
Action->>Parent: repository dispatch event
Parent-->>GH: (optional) handle event / trigger workflows
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR fixes a silent bug where the Key changes:
Issues found:
Confidence Score: 4/5Safe to merge after scoping the --recursive injection to the create subcommand to avoid regressions on other worktree commands. There is one clear P1 regression: any worktree subcommand other than create will fail with a clap parse error whenever the global --recursive flag is present. The fix is targeted and not risky; the overall approach of threading the protocol option back into clap args is sound. src/commands/worktree/mod.rs — the injection logic needs to be scoped to create and the ordering relative to the empty-args guard should be corrected.
|
| Filename | Overview |
|---|---|
| src/commands/worktree/mod.rs | Adds recursive parameter and injects --recursive into clap args, but the injection is not scoped to the create subcommand and bypasses the empty-args help guard. |
| src/lib.rs | Threads options.recursive through to execute_worktree_command; straightforward, correct change. |
| .github/workflows/notify-parent.yml | New workflow to dispatch a child-repo-updated event to the parent harmony-labs/meta repo on push to main; uses a pinned action SHA and a PAT secret, with correctly scoped permissions: {}. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["meta CLI receives\n--recursive flag"] --> B["Strip --recursive,\nset options.recursive = true"]
B --> C["execute_command()\nin lib.rs"]
C --> D{"starts_with\n'worktree' or\n'git worktree'?"}
D -- Yes --> E["execute_worktree_command(\n ..., recursive=true\n)"]
E --> F["Build clap_args\nfrom command + args"]
F --> G{"recursive &&\n--recursive not\nalready in args?"}
G -- Yes --> H["clap_args.push('--recursive')"]
G -- No --> I["clap_args unchanged"]
H --> J{"clap_args is_empty?"}
I --> J
J -- Yes --> K["show help"]
J -- No --> L["WorktreeParser::try_parse_from(clap_args)"]
L --> M{"subcommand?"}
M -- create --> N["handle_create(args)\nwith recursive=true ✓"]
M -- "list/status/remove/etc." --> O["❌ clap error: unexpected --recursive\n(P1 regression)"]
D -- No --> P["dispatch to other git command handlers"]
Comments Outside Diff (1)
-
src/commands/worktree/mod.rs, line 59-69 (link)Injection before empty-args guard bypasses help display
The
--recursiveinjection executes before theclap_args.is_empty()guard. If a user runsmeta --recursive git worktree(no subcommand) the injection turns the empty vec into["--recursive"], theis_empty()branch is never reached, and clap returns a cryptic "required argument missing" error instead of the intended help text.Moving the empty check before the injection would preserve the clean help path:
// No subcommand at all — show help (check before flag injection) if clap_args.is_empty() { print_worktree_help(); return CommandResult::Message(String::new()); } if recursive && !clap_args.iter().any(|a| a == "--recursive" || a == "-r") { clap_args.push("--recursive".to_string()); }
Prompt To Fix With AI
This is a comment left during a code review. Path: src/commands/worktree/mod.rs Line: 59-69 Comment: **Injection before empty-args guard bypasses help display** The `--recursive` injection executes before the `clap_args.is_empty()` guard. If a user runs `meta --recursive git worktree` (no subcommand) the injection turns the empty vec into `["--recursive"]`, the `is_empty()` branch is never reached, and clap returns a cryptic "required argument missing" error instead of the intended help text. Moving the empty check before the injection would preserve the clean help path: ```rust // No subcommand at all — show help (check before flag injection) if clap_args.is_empty() { print_worktree_help(); return CommandResult::Message(String::new()); } if recursive && !clap_args.iter().any(|a| a == "--recursive" || a == "-r") { clap_args.push("--recursive".to_string()); } ``` How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/commands/worktree/mod.rs
Line: 59-63
Comment:
**`--recursive` injected for all subcommands, not just `create`**
`--recursive` / `-r` is only declared in `CreateArgs`. Every other worktree subcommand (`list`, `status`, `remove`, `add`, `prune`, `diff`, `exec`) does not expose this flag. When `options.recursive` is `true` (i.e. the user ran any meta command with the global `--recursive` flag), the injection will cause clap to fail with an "unexpected argument" error for every worktree subcommand that isn't `create`.
For example, `meta --recursive git worktree list` would produce a clap parse error after this change, whereas it previously worked correctly.
The injection should be guarded to only apply when the dispatched subcommand is `create`:
```rust
// Determine the active subcommand name for flag injection
let active_sub = if subcommand.is_empty() {
clap_args.first().map(|s| s.as_str()).unwrap_or("")
} else {
subcommand
};
// Only inject --recursive for the `create` subcommand, which is the only
// one that declares this flag.
if recursive
&& active_sub == "create"
&& !clap_args.iter().any(|a| a == "--recursive" || a == "-r")
{
clap_args.push("--recursive".to_string());
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/commands/worktree/mod.rs
Line: 59-69
Comment:
**Injection before empty-args guard bypasses help display**
The `--recursive` injection executes before the `clap_args.is_empty()` guard. If a user runs `meta --recursive git worktree` (no subcommand) the injection turns the empty vec into `["--recursive"]`, the `is_empty()` branch is never reached, and clap returns a cryptic "required argument missing" error instead of the intended help text.
Moving the empty check before the injection would preserve the clean help path:
```rust
// No subcommand at all — show help (check before flag injection)
if clap_args.is_empty() {
print_worktree_help();
return CommandResult::Message(String::new());
}
if recursive && !clap_args.iter().any(|a| a == "--recursive" || a == "-r") {
clap_args.push("--recursive".to_string());
}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: thread recursive option from plugin..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/worktree/mod.rs`:
- Around line 59-63: The code currently reinjects "--recursive" whenever
recursive is true, regardless of which worktree subcommand is being invoked;
restrict this insertion to only the create subcommand by checking the target
subcommand before mutating clap_args. Concretely: when evaluating whether to
push "--recursive" (the existing recursive && !clap_args.iter().any(...)
branch), also verify that the invocation is for "create" (e.g., check the
subcommand name in whatever variable holds the requested subcommand or check
clap_args for "create") and only then push to clap_args; keep the existing
duplicate-check logic intact.
🪄 Autofix (Beta)
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
Run ID: 596e4821-000e-47b5-9a75-18979a621a6c
📒 Files selected for processing (3)
.github/workflows/notify-parent.ymlsrc/commands/worktree/mod.rssrc/lib.rs
Only `create` declares the --recursive flag. Injecting it for all worktree subcommands (list, status, remove, etc.) would cause clap parse errors when global --recursive is set. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
metaCLI strips--recursivefrom command args and passes it as a plugin protocol option (options.recursive)execute_worktree_commandnever threaded this option through to clap arg parsing, soCreateArgs.recursivewas alwaysfalsebuild_nested_dep_graphwas never invoked, and transitivedepends_onentries in nested.meta.yamlfiles were silently ignoredFix
recursive: boolparameter toexecute_worktree_commandoptions.recursivefrom the plugin dispatch inlib.rs--recursiveback into clap args when the protocol option is setTest plan
meta git worktree create test --repo open-source/gitkb/core --recursivenow includesvendor/tree-sitter-markdowncargo checksucceeds in the resulting worktree🤖 Generated with Claude Code
Implements [[tasks/harmony-407]]
Summary by CodeRabbit
New Features
Chores