refactor: add clone_queue module, use config from meta_core - #2
Conversation
Move CloneQueue/CloneTask from meta_git_cli into meta_git_lib as reusable queue logic. Update worktree helpers to import config from meta_core instead of meta_cli. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughAdds a new thread-safe clone task queue module for recursive, depth-limited repository discovery and queuing; migrates worktree helpers from Changes
Sequence Diagram(s)sequenceDiagram
participant Worker as Worker (thread)
participant Queue as CloneQueue
participant FS as Filesystem/.meta
participant Git as Git (clone)
Worker->>Queue: take_one()
alt task available
Queue-->>Worker: CloneTask
Worker->>Git: git clone (task.url, depth=git_depth)
Git-->>Worker: clone result
alt success
Worker->>Queue: mark_completed(task)
Queue->>FS: push_from_meta(task.target_path, task.depth_level + 1)
FS-->>Queue: discovered tasks
Queue-->>Queue: enqueue discovered tasks (dedupe)
else failure
Worker->>Queue: mark_failed(task)
end
else no task
Queue-->>Worker: None
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/clone_queue.rs`:
- Around line 55-78: The push method has a TOCTOU window because it locks
completed and pending separately; change it to acquire both mutexes together (in
a consistent order, e.g., lock completed then pending) before performing the
checks so the check for completed.contains(&path) and the check for
pending.iter().any(|t| t.target_path == path) are done atomically, then push the
CloneTask and increment total_discovered; keep using unwrap_or_else(|e|
e.into_inner()) for both locks to preserve current error handling and ensure you
don't introduce deadlocks by always locking the two Mutexes in the same order.
- Around line 198-203: Replace the eprintln! call that prints the meta warning
with log::warn! so logging is consistent with the rest of the module;
specifically change the block that checks task.is_meta && added == 0 to call
warn!("warning: '{}' is declared with `meta: true` but no .meta config was found
inside it", task.name). Ensure the log::warn macro is in scope (add use
log::warn; or call log::warn! fully-qualified) so the warning respects the
application's logging configuration.
Address CodeRabbit review feedback: - Fix TOCTOU race in CloneQueue::push() by holding both completed and pending locks atomically during the check-and-insert - Replace eprintln! with log::warn! for meta: true warning, consistent with rest of module's use of log crate - Replace deprecated serde_yaml with serde_yml Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CI was failing because the repo uses workspace-inherited fields (version.workspace = true) but was checked out in isolation without a workspace root. Add proper workspace setup and smart branch cloning that tries the PR branch name first before falling back to main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @.github/workflows/auto-format.yml:
- Around line 30-36: The workflow currently constructs BRANCH directly from
github.head_ref/github.ref_name which can allow script injection; change the
BRANCH assignment to read a pre-set environment input (e.g., an env var like
BRANCH or INPUT_BRANCH supplied to the job) and do not interpolate
github.head_ref directly in the shell. Update the clone loop that uses BRANCH
(the for repo in ... git clone ... -b "$BRANCH") to instead use that safe env
var, and also add a simple sanitization step (strip newlines/control characters
or allow only a safe character class) before passing BRANCH into git clone to
ensure malicious branch names cannot inject shell content.
In @.github/workflows/ci.yml:
- Around line 35-52: The three repeated steps ("Clone dependencies", "Create
workspace Cargo.toml", and "Create VERSION file") should be extracted into a
reusable local composite action (e.g., named setup-workspace) and replaced in
the test, clippy, and format jobs with a single uses line; implement the
composite action to run the git clone and the two shell blocks that write
Cargo.toml and VERSION, expose any inputs if needed (e.g., workspace members or
version string), update each job to call this new action, and remove the
duplicate step blocks from the workflow to ensure consistency and easier
maintenance.
- Around line 29-33: The BRANCH assignment and git clone loop currently
interpolate the untrusted expression github.head_ref directly into the shell,
enabling command injection; instead, expose the branch value via a job env
variable (e.g., SAFE_BRANCH) set from github.head_ref or github.ref_name, then
in the script use the env var (BRANCH or SAFE_BRANCH) only as a quoted argument
to git clone (and perform a simple allowlist/sanitization that strips or rejects
characters outside a safe set like alphanumerics, dot, dash, underscore, slash)
before calling --branch "$BRANCH"; apply the same change to the clippy and
format jobs so they also stop directly interpolating github.head_ref/ref_name
into the shell.
In `@Cargo.toml`:
- Line 21: The Cargo.toml dependency serde_yml = "0.0.12" is unmaintained and
must be replaced; edit Cargo.toml to remove serde_yml and add a maintained
alternative such as serde_yaml_ng = "<choose-appropriate-version>" (or
serde_norway / the maintained serde-yaml fork), then update all crate references
in the code (e.g., any use/import paths from serde_yml::... or extern crate
serde_yml to the new crate name like serde_yaml_ng::...) and adjust any API
differences if needed; finally run cargo update and cargo test to verify builds
and update Cargo.lock.
- Replace serde_yml (RUSTSEC-2025-0068: unsound, unmaintained) with serde_yaml_ng (maintained drop-in replacement) - Fix CI script injection: move github.head_ref to env: instead of inline shell interpolation - Add clone failure handling and branch logging in CI Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/auto-format.yml:
- Around line 45-57: The heredoc used for creating Cargo.toml in the GitHub
Action contains leading spaces on each content line, which will write invalid
indented TOML; update the step that runs cat > Cargo.toml << 'EOF' so the
heredoc lines are left-aligned (remove the leading indentation from the block
between EOF markers) or otherwise generate the file without indentation,
ensuring the literal lines like [workspace], members = [...], and
[workspace.package] are at column 0.
In @.github/workflows/ci.yml:
- Around line 42-55: The heredoc in the "Create workspace Cargo.toml" run step
is indented so the generated Cargo.toml contains leading spaces and is invalid
TOML; fix by ensuring the heredoc body is not indented (move the [workspace]...
EOF block to be left-aligned in the run block) or replace the heredoc with a
non-indented writer (e.g., printf or echo with explicit newlines) so the written
Cargo.toml has no leading whitespace; apply the same change to the identical
steps in the clippy and format jobs.
Summary
clone_queuemodule withCloneQueueandCloneTask(moved frommeta_git_cli)worktree/helpers.rsimports to usemeta_core::configinstead ofmeta_cli::configMerge order
Merge after: gitkb/meta_core#3
Test plan
cargo build --workspacepassescargo test --workspacepasses🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
CI / Workflows
Chores
Tests