Open
Conversation
…d-project handlers
Adds --workspace-create, --workspace-list, --workspace-add-project CLI arguments and their dispatch handlers to the main CLI entry point, wiring up the multi_repo_commands module into the Auto-Code framework. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Create tests/test_dependency_graph.py with 82 tests covering MultiRepoDependencyGraph: node/edge management, topological sort, cycle detection, BFS affected-repo traversal, impact scores, and round-trip serialization. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR introduces a multi-codebase orchestration feature with a dependency graph analyzer for cross-repository dependency modeling, CLI handlers for workspace lifecycle management (create, list, add-project), and CLI integration with corresponding test coverage and documentation. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI Handler
participant WSManager as WorkspaceManager
participant FS as File System
rect rgba(100, 150, 200, 0.5)
Note over User,FS: Workspace Creation Flow
User->>CLI: --workspace-create --workspace-name my_ws
CLI->>WSManager: workspace_exists(my_ws)
WSManager->>FS: Check workspace.json exists
FS-->>WSManager: Not found
WSManager-->>CLI: False
CLI->>WSManager: create_workspace(my_ws, base_dir, description)
WSManager->>FS: Create workspace directory
WSManager->>FS: Write workspace.json
FS-->>WSManager: Success
CLI-->>User: Print success + suggested next commands
end
rect rgba(150, 100, 200, 0.5)
Note over User,FS: Project Addition Flow
User->>CLI: --workspace-add-project --workspace-name my_ws --workspace-project-name proj1 ...
CLI->>WSManager: load_workspace(my_ws)
WSManager->>FS: Read workspace.json
FS-->>WSManager: Workspace data
CLI->>WSManager: get_project_state(proj1)
WSManager-->>CLI: None (not exists)
CLI->>WSManager: add_project(proj1, path, relationship, dependencies)
WSManager->>WSManager: Update project list + state
WSManager->>FS: Write updated workspace.json
CLI-->>User: Print added project details
end
sequenceDiagram
participant Analysis as Dependency Analysis
participant Graph as DepGraph
participant RepoNodes as Repo Nodes
participant Manifest as Manifest Parser
rect rgba(200, 100, 100, 0.5)
Note over Analysis,Manifest: Dependency Analysis & Build Order
Analysis->>Graph: add_repo(repo_name, repo_path)
Graph->>RepoNodes: Create RepoNode + Register
Analysis->>Graph: analyze()
Graph->>Manifest: Parse manifest files (each repo)
Manifest-->>Graph: External modules + dependencies
Graph->>Graph: Infer cross-repo dependencies
Graph->>Graph: Add IMPORTS edges (automated)
Graph-->>Analysis: Analysis complete
Analysis->>Graph: get_build_order()
Graph->>Graph: Topological sort (Kahn algorithm)
Graph->>Graph: Cycle detection
Graph-->>Analysis: Ordered list [repo1, repo2, ...]
Analysis-->>Analysis: Return build sequence
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…lti-codebase-orchestration # Conflicts: # apps/backend/cli/main.py
…tion - Resolve merge conflict in cli/main.py keeping both multi-repo workspace and environment setup CLI arguments - Fix ruff errors: unsorted imports, unnecessary dict comprehension (use dict.fromkeys), f-strings without placeholders - Add explanatory comments to all bare except-pass blocks to satisfy GitHub Advanced Security checks - Extract _normalize_package_name and _read_package_json helpers in dependency_graph.py to eliminate duplicated package.json reading and name normalization logic - Extract _resolve_base_dir helper in multi_repo_commands.py to deduplicate default workspace directory resolution - Refactor test_multi_repo_commands.py: extract _patch_workspace_cli context manager, _make_mock_manager, _make_mock_project, and _setup_add_project_mocks helpers, consolidate 3 error tests into one parametrized test (net reduction of ~86 lines) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix ruff: import sorting, dict.fromkeys, remove empty f-strings - Add explanatory comments to empty except blocks - Extract helpers to reduce duplication: _normalize_package_name(), _read_package_json(), _resolve_base_dir(), _patch_workspace_cli() - Parametrize repeated error-handling tests - Add docs/features/MULTI-CODEBASE-ORCHESTRATION.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
@.auto-claude/specs/163-multi-codebase-orchestration/implementation_plan.json:
- Around line 58-79: Update the implementation plan JSON to match the actual CLI
changes: remove the stale reference to `--dep-graph` from the task "description"
and update the verifier under "verification.command" to expect three `workspace`
occurrences (change the grep count expected from 2 to 3) so it matches the flags
added in `apps/backend/cli/main.py` (`--workspace-create`, `--workspace-list`,
`--workspace-add-project`); ensure the "description" text and the "expected"
value in the `implementation_plan.json` subtask reflect these exact CLI flags
and counts.
In `@apps/backend/analysis/dependency_graph.py`:
- Around line 244-245: analyze() currently discards manually-added IMPORTS edges
because add_edge() uses EdgeType.IMPORTS and the rebuild logic at analyze()
(lines around 290-303) filters out IMPORTS; update the graph model and rebuild
logic to distinguish inferred vs manually-added edges (e.g., add a boolean or
enum property like Edge.inferred or EdgeSource on edges) rather than overloading
EdgeType, adjust add_edge() to accept and set this new flag, and change
analyze()'s preserve step to keep edges where Edge.inferred == False (or the
manual source) so manually recorded IMPORTS are not removed when the graph is
reconstructed.
- Around line 381-391: The BFS currently allows the seed repo (name) to be
re-added via cycles because visited starts empty; to fix, mark the seed as
visited before traversal (e.g., add name to visited) so dependents equal to name
are skipped and never appended to affected, or alternatively guard inside the
loop to skip when dependent == name; update the logic around visited, queue,
affected, self.nodes, current and dependents so the seed repo is excluded from
cyclic traversals.
- Around line 676-685: The manual split-based parsing in the
requirements-reading loop should be replaced with
packaging.requirements.Requirement to correctly handle PEP 440 operators and
direct references: import Requirement from packaging.requirements, call
Requirement(stripped).name inside the loop (wrap in try/except to skip malformed
lines), then add the normalized name to deps (use the existing
_normalize_package_name or lowercase+replace("-", "_") as before). Update the
loop in dependency_graph.py where requirements.read_text(...) is parsed and
remove the chained .split() logic so names come from Requirement(...).name.
In `@apps/backend/cli/main.py`:
- Around line 552-600: The workspace flags (--workspace-create,
--workspace-list, --workspace-add-project) are currently added directly to the
ArgumentParser and can be passed together, causing _run_cli() to only execute
the first matching branch; wrap these three options in a mutually exclusive
group by calling parser.add_mutually_exclusive_group() and adding the three
flags to that group instead of calling parser.add_argument() on parser for them,
leaving related auxiliary flags (--workspace-description,
--workspace-project-name, --workspace-relationship, --workspace-deps) outside
the group so they remain available when any single workspace command is chosen.
In `@apps/backend/cli/multi_repo_commands.py`:
- Around line 102-113: The workspace scan over base_dir.iterdir() in the
function using _resolve_base_dir should be wrapped in a try/except so filesystem
errors (e.g., NotADirectoryError, PermissionError, OSError) are handled and the
function returns False on error; catch those exceptions around the loop that
builds workspaces (the for entry in sorted(base_dir.iterdir()) block), call
print_status with an error message including the exception, and return False
instead of letting the exception propagate.
In `@docs/features/MULTI-CODEBASE-ORCHESTRATION.md`:
- Around line 7-31: The docs page MULTI-CODEBASE-ORCHESTRATION.md is out of sync
with the actual CLI and config model; update the documented CLI examples and
config schema to match the real flags used in the codebase (replace the
non‑existent --workspace init/add/list/remove/deps/status and project-role usage
with the actual flags --workspace-create, --workspace-list,
--workspace-add-project, etc., and ensure the workspace.json schema matches what
apps/backend/cli and analysis code expect), remove references to unsupported
features (Cargo/Go support and python cli/main.py invocation if not present),
and reformat the page using the reusable docs/templates and follow
docs/STYLE_GUIDE.md conventions; also apply the same corrections to the other
affected blocks (lines noted in the review) so the examples and schema
accurately reflect the implementation.
In `@tests/test_dependency_graph.py`:
- Around line 5-14: Add regression tests that exercise the full analyze()
workflow instead of manually building graphs with add_repo()/add_edge(): create
temporary package manifests in pytest tmp_path (or tmpdir), invoke analyze() to
build the graph, and assert that dependency extraction yields inferred edges,
that manually added edges (via add_edge() or register) are preserved, and that
edge deduplication/cycle detection behaves as expected; use fixtures/mocks to
isolate filesystem/IO and ensure tests cover inferred-edge creation, manual-edge
preservation, and any transform functions used by analyze() (refer to analyze(),
add_repo(), add_edge(), and any manifest-parsing helpers) so regressions in
extraction/inference are caught.
In `@tests/test_multi_repo_commands.py`:
- Around line 6-10: Add a thin integration test that exercises the CLI entry
points by calling parse_args and _run_cli (or whatever parse/dispatch functions
were added to main.py) to validate the new flags --workspace-create,
--workspace-list, --workspace-add-project and the split handling of
--workspace-deps; specifically, call the parsing function with representative
argv arrays and assert the parsed namespace/args shape, then call the dispatcher
with those args while mocking the handler functions
(handle_workspace_create_command, handle_workspace_list_command,
handle_workspace_add_project_command) and any I/O/side-effects so tests remain
isolated and follow pytest conventions. Ensure you include tests for both
presence and combinations of flags, verify the correct handler is invoked with
expected arguments, and use pytest fixtures/mocks to avoid touching real state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0595c0c2-9fba-4fc4-9ac1-33f344913683
📒 Files selected for processing (7)
.auto-claude/specs/163-multi-codebase-orchestration/implementation_plan.jsonapps/backend/analysis/dependency_graph.pyapps/backend/cli/main.pyapps/backend/cli/multi_repo_commands.pydocs/features/MULTI-CODEBASE-ORCHESTRATION.mdtests/test_dependency_graph.pytests/test_multi_repo_commands.py
Comment on lines
+58
to
+79
| "description": "Add --workspace-create, --workspace-list, --workspace-add-project, --dep-graph args to main.py", | ||
| "depends_on": [ | ||
| "phase-1-multi-repo-commands" | ||
| ], | ||
| "parallel_safe": false, | ||
| "subtasks": [ | ||
| { | ||
| "id": "subtask-2-1", | ||
| "description": "Add multi-repo CLI arguments and dispatch to main.py", | ||
| "service": "backend", | ||
| "files_to_modify": [ | ||
| "apps/backend/cli/main.py" | ||
| ], | ||
| "files_to_create": [], | ||
| "patterns_from": [ | ||
| "apps/backend/cli/main.py" | ||
| ], | ||
| "verification": { | ||
| "type": "command", | ||
| "command": "python apps/backend/run.py --help 2>&1 | grep -c workspace", | ||
| "expected": "2" | ||
| }, |
Contributor
There was a problem hiding this comment.
This completed phase is already out of sync with the actual CLI.
The description still mentions --dep-graph, and the verifier assumes only two workspace hits in --help. apps/backend/cli/main.py in this PR adds three workspace flags and no --dep-graph, so this plan will misreport completion if it gets re-run or audited.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.auto-claude/specs/163-multi-codebase-orchestration/implementation_plan.json
around lines 58 - 79, Update the implementation plan JSON to match the actual
CLI changes: remove the stale reference to `--dep-graph` from the task
"description" and update the verifier under "verification.command" to expect
three `workspace` occurrences (change the grep count expected from 2 to 3) so it
matches the flags added in `apps/backend/cli/main.py` (`--workspace-create`,
`--workspace-list`, `--workspace-add-project`); ensure the "description" text
and the "expected" value in the `implementation_plan.json` subtask reflect these
exact CLI flags and counts.
Comment on lines
+244
to
+245
| edge_type: EdgeType = EdgeType.IMPORTS, | ||
| weight: int = 1, |
Contributor
There was a problem hiding this comment.
analyze() drops manually added IMPORTS edges.
add_edge() defaults to EdgeType.IMPORTS, but Lines 291-303 preserve only non-IMPORTS edges before rebuilding the graph. Any caller that manually records an import relationship loses it on the next analyze(), which can change build order and affected-repo calculations. Please track inferred-vs-manual separately instead of overloading edge_type.
Also applies to: 290-303
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/analysis/dependency_graph.py` around lines 244 - 245, analyze()
currently discards manually-added IMPORTS edges because add_edge() uses
EdgeType.IMPORTS and the rebuild logic at analyze() (lines around 290-303)
filters out IMPORTS; update the graph model and rebuild logic to distinguish
inferred vs manually-added edges (e.g., add a boolean or enum property like
Edge.inferred or EdgeSource on edges) rather than overloading EdgeType, adjust
add_edge() to accept and set this new flag, and change analyze()'s preserve step
to keep edges where Edge.inferred == False (or the manual source) so manually
recorded IMPORTS are not removed when the graph is reconstructed.
Comment on lines
+381
to
+391
| visited: set[str] = set() | ||
| queue: deque[str] = deque([name]) | ||
| affected: list[str] = [] | ||
|
|
||
| while queue: | ||
| current = queue.popleft() | ||
| for dependent in self.nodes[current].dependents: | ||
| if dependent not in visited: | ||
| visited.add(dependent) | ||
| affected.append(dependent) | ||
| queue.append(dependent) |
Contributor
There was a problem hiding this comment.
Exclude the seed repo from cyclic traversals.
visited starts empty, so a self-loop or any cycle back to name appends the changed repo to affected even though this method promises to exclude it.
Suggested fix
- visited: set[str] = set()
+ visited: set[str] = {name}
queue: deque[str] = deque([name])
affected: list[str] = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/analysis/dependency_graph.py` around lines 381 - 391, The BFS
currently allows the seed repo (name) to be re-added via cycles because visited
starts empty; to fix, mark the seed as visited before traversal (e.g., add name
to visited) so dependents equal to name are skipped and never appended to
affected, or alternatively guard inside the loop to skip when dependent == name;
update the logic around visited, queue, affected, self.nodes, current and
dependents so the seed repo is excluded from cyclic traversals.
Comment on lines
+676
to
+685
| try: | ||
| for line in requirements.read_text(encoding="utf-8").splitlines(): | ||
| stripped = line.strip() | ||
| if stripped and not stripped.startswith("#"): | ||
| # Strip version specifiers: foo>=1.0 → foo | ||
| name = stripped.split("=")[0].split(">")[0].split("<")[0] | ||
| name = name.split("!")[0].split("[")[0].strip() | ||
| if name: | ||
| deps.add(name.lower().replace("-", "_")) | ||
| except OSError: |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file and the specific lines mentioned
head -n 700 apps/backend/analysis/dependency_graph.py | tail -n 50Repository: OBenner/Auto-Coding
Length of output: 1989
🏁 Script executed:
# Let's see the imports and context around these lines
head -n 710 apps/backend/analysis/dependency_graph.py | head -n 20Repository: OBenner/Auto-Coding
Length of output: 862
🏁 Script executed:
# Check if packaging is already imported or available
rg -n "from packaging|import packaging|from requirements|import requirements" apps/backend/analysis/dependency_graph.pyRepository: OBenner/Auto-Coding
Length of output: 45
🏁 Script executed:
# Check the entire function context
rg -B 20 "for line in requirements.read_text" apps/backend/analysis/dependency_graph.pyRepository: OBenner/Auto-Coding
Length of output: 674
🏁 Script executed:
# Let's see how deps are used after being collected
rg -A 30 "for line in requirements.read_text" apps/backend/analysis/dependency_graph.pyRepository: OBenner/Auto-Coding
Length of output: 1548
🏁 Script executed:
# Check if packaging library is available in requirements or dependencies
rg -l "packaging" apps/backend/ | head -20Repository: OBenner/Auto-Coding
Length of output: 134
🏁 Script executed:
# Let's test the split logic manually to confirm the parsing issue
python3 << 'EOF'
# Test the current parsing logic
def parse_requirement_current(line):
stripped = line.strip()
if stripped and not stripped.startswith("#"):
name = stripped.split("=")[0].split(">")[0].split("<")[0]
name = name.split("!")[0].split("[")[0].strip()
if name:
return name.lower().replace("-", "_")
return None
# Test cases
test_cases = [
"foo>=1.0", # Standard >=
"foo~=1.2", # Tilde release
"foo != 1.0", # Not equal
"foo[extra]>=1.0", # Extras
"foo @ git+https://github.com/user/repo.git", # Direct reference
"foo @ file:///path/to/local", # File reference
"requests>=2.25.0", # Normal package
]
print("Current parsing results:")
for test in test_cases:
result = parse_requirement_current(test)
print(f" {test!r:50} → {result!r}")
EOFRepository: OBenner/Auto-Coding
Length of output: 583
🏁 Script executed:
# Check if there's a packaging library being used elsewhere in the backend
rg "packaging\.requirements\|packaging\.specifiers\|Requirement\|Specifier" apps/backend/ -t py | head -20Repository: OBenner/Auto-Coding
Length of output: 45
🏁 Script executed:
# Let's check the Python version requirement
cat apps/backend/pyproject.toml | grep -A 5 "python"Repository: OBenner/Auto-Coding
Length of output: 122
🏁 Script executed:
# Check if there's a requirements.txt or setup.py that shows packaging as a dependency
find apps/backend -name "requirements*.txt" -o -name "pyproject.toml" | xargs grep -l "packaging" 2>/dev/nullRepository: OBenner/Auto-Coding
Length of output: 47
🏁 Script executed:
# Verify how _extract_declared_deps is used
rg "_extract_declared_deps\|_declared_deps" apps/backend/analysis/dependency_graph.py -A 5 -B 2Repository: OBenner/Auto-Coding
Length of output: 45
🏁 Script executed:
# Check what packages are available/installed in the backend
ls -la apps/backend/ | grep -E "requirements|setup|pyproject"Repository: OBenner/Auto-Coding
Length of output: 130
🏁 Script executed:
# Look for root level dependency declarations
ls -la | grep -E "requirements|setup|pyproject|poetry"Repository: OBenner/Auto-Coding
Length of output: 45
🏁 Script executed:
# Check if packaging is a standard library (it's not, but let's verify availability)
python3 -c "from packaging.requirements import Requirement; print('packaging available')" 2>&1Repository: OBenner/Auto-Coding
Length of output: 82
🏁 Script executed:
# Now let's see how the deps collected are used in _infer_cross_repo_edges
rg -A 15 "def _infer_cross_repo_edges" apps/backend/analysis/dependency_graph.pyRepository: OBenner/Auto-Coding
Length of output: 876
🏁 Script executed:
# Check if there are any tests for this parsing logic
find apps/backend -name "*test*dependency*" -o -name "*dependency*test*" | xargs ls -la 2>/dev/nullRepository: OBenner/Auto-Coding
Length of output: 4760
🏁 Script executed:
# Let's verify the exact flow: extract_declared_deps -> usage in inference
rg "deps\." apps/backend/analysis/dependency_graph.py | grep -v "^Binary"Repository: OBenner/Auto-Coding
Length of output: 190
🏁 Script executed:
# Let's check the _normalize_package_name function
rg -B 5 -A 10 "def _normalize_package_name" apps/backend/analysis/dependency_graph.pyRepository: OBenner/Auto-Coding
Length of output: 650
🏁 Script executed:
# Let's verify the impact: if foo~ doesn't match normalized names
python3 << 'EOF'
# Simulate what happens with normalized vs current parsing
def _normalize_package_name(name):
# Let's find the actual implementation
return name.lower().replace("-", "_").replace(".", "_")
# Current parsing problems
current_results = {
"foo~=1.2": "foo~",
"foo @ git+https://github.com/user/repo.git": "foo @ git+https://github.com/user/repo.git",
}
# What should be matched?
target_packages = {
"foo", # The actual package name we want to match
}
print("With current parsing:")
for input_req, parsed_name in current_results.items():
normalized = _normalize_package_name(parsed_name)
match = normalized in {_normalize_package_name(t) for t in target_packages}
print(f" {input_req!r}")
print(f" → parsed: {parsed_name!r} → normalized: {normalized!r} → matches: {match}")
print("\nWith proper parsing (using packaging.Requirement):")
try:
from packaging.requirements import Requirement
for input_req in current_results.keys():
try:
req = Requirement(input_req)
name = req.name
normalized = _normalize_package_name(name)
match = normalized in {_normalize_package_name(t) for t in target_packages}
print(f" {input_req!r}")
print(f" → parsed: {name!r} → normalized: {normalized!r} → matches: {match}")
except Exception as e:
print(f" {input_req!r} → ERROR: {e}")
except ImportError:
print("packaging not available")
EOFRepository: OBenner/Auto-Coding
Length of output: 562
🏁 Script executed:
# Let's check the actual requirements.txt in the project to see if it uses ~= or @ syntax
cat apps/backend/requirements.txtRepository: OBenner/Auto-Coding
Length of output: 2619
🏁 Script executed:
# Search for any test files that test dependency parsing
find apps/backend -path "*/test*" -type f -name "*.py" | xargs grep -l "dependency_graph\|_extract_declared_deps" 2>/dev/nullRepository: OBenner/Auto-Coding
Length of output: 47
Replace manual split() parsing with packaging.requirements.Requirement.
The current chained .split() approach fails on PEP 440 operators: foo~=1.2 parses to foo~ (never matches), and direct references like foo @ git+https://... remain unparsed. Since packaging is already available, use Requirement(line).name instead:
Example fix
from packaging.requirements import Requirement
for line in requirements.read_text(encoding="utf-8").splitlines():
stripped = line.strip()
if stripped and not stripped.startswith("#"):
try:
name = Requirement(stripped).name
if name:
deps.add(_normalize_package_name(name))
except Exception:
pass # Skip malformed linesThis prevents silent misses of cross-repo matches in modern Python projects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/analysis/dependency_graph.py` around lines 676 - 685, The manual
split-based parsing in the requirements-reading loop should be replaced with
packaging.requirements.Requirement to correctly handle PEP 440 operators and
direct references: import Requirement from packaging.requirements, call
Requirement(stripped).name inside the loop (wrap in try/except to skip malformed
lines), then add the normalized name to deps (use the existing
_normalize_package_name or lowercase+replace("-", "_") as before). Update the
loop in dependency_graph.py where requirements.read_text(...) is parsed and
remove the chained .split() logic so names come from Requirement(...).name.
Comment on lines
+552
to
+600
| # Multi-repo workspace commands | ||
| parser.add_argument( | ||
| "--workspace-create", | ||
| type=str, | ||
| default=None, | ||
| metavar="NAME", | ||
| help="Create a new multi-repo workspace with the given name", | ||
| ) | ||
| parser.add_argument( | ||
| "--workspace-list", | ||
| action="store_true", | ||
| help="List all multi-repo workspaces", | ||
| ) | ||
| parser.add_argument( | ||
| "--workspace-add-project", | ||
| type=str, | ||
| nargs=2, | ||
| default=None, | ||
| metavar=("WORKSPACE", "PATH"), | ||
| help="Add a project to a workspace: --workspace-add-project <workspace> <path>", | ||
| ) | ||
| parser.add_argument( | ||
| "--workspace-description", | ||
| type=str, | ||
| default=None, | ||
| metavar="DESC", | ||
| help="With --workspace-create or --workspace-add-project: optional description", | ||
| ) | ||
| parser.add_argument( | ||
| "--workspace-project-name", | ||
| type=str, | ||
| default=None, | ||
| metavar="NAME", | ||
| help="With --workspace-add-project: override project name (default: directory name)", | ||
| ) | ||
| parser.add_argument( | ||
| "--workspace-relationship", | ||
| type=str, | ||
| default="independent", | ||
| choices=["independent", "depends_on", "library", "monorepo_package"], | ||
| help="With --workspace-add-project: project relationship type (default: independent)", | ||
| ) | ||
| parser.add_argument( | ||
| "--workspace-deps", | ||
| type=str, | ||
| default=None, | ||
| metavar="PROJECTS", | ||
| help="With --workspace-add-project: comma-separated project names this project depends on", | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l apps/backend/cli/main.pyRepository: OBenner/Auto-Coding
Length of output: 92
🏁 Script executed:
sed -n '552,610p' apps/backend/cli/main.pyRepository: OBenner/Auto-Coding
Length of output: 1940
🏁 Script executed:
sed -n '689,730p' apps/backend/cli/main.pyRepository: OBenner/Auto-Coding
Length of output: 1341
🏁 Script executed:
grep -n "_run_cli" apps/backend/cli/main.py | head -5Repository: OBenner/Auto-Coding
Length of output: 113
🏁 Script executed:
sed -n '660,750p' apps/backend/cli/main.pyRepository: OBenner/Auto-Coding
Length of output: 3016
🏁 Script executed:
rg "add_mutually_exclusive_group" apps/backend/cli/main.py | head -10Repository: OBenner/Auto-Coding
Length of output: 178
🏁 Script executed:
sed -n '1,650p' apps/backend/cli/main.py | grep -n "add_mutually_exclusive_group" | head -5Repository: OBenner/Auto-Coding
Length of output: 186
🏁 Script executed:
sed -n '175,200p' apps/backend/cli/main.pyRepository: OBenner/Auto-Coding
Length of output: 847
🏁 Script executed:
sed -n '551,575p' apps/backend/cli/main.pyRepository: OBenner/Auto-Coding
Length of output: 758
🏁 Script executed:
sed -n '540,580p' apps/backend/cli/main.py | head -50Repository: OBenner/Auto-Coding
Length of output: 1278
Use mutually exclusive group for workspace commands to match existing patterns.
These flags are command-style entry points (like --merge, --review, --discard at lines 188-205), but workspace commands are defined with plain parser.add_argument() instead of add_mutually_exclusive_group(). This allows conflicting combinations like --workspace-create foo --workspace-list, where _run_cli() silently executes only the first matching branch via early return. Users can unintentionally trigger a write operation when expecting a read.
Wrap the three workspace commands (--workspace-create, --workspace-list, --workspace-add-project) in a mutually exclusive group to align with the codebase's existing patterns (lines 175-186, 188-205) and prevent accidental command conflicts at parse time.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/cli/main.py` around lines 552 - 600, The workspace flags
(--workspace-create, --workspace-list, --workspace-add-project) are currently
added directly to the ArgumentParser and can be passed together, causing
_run_cli() to only execute the first matching branch; wrap these three options
in a mutually exclusive group by calling parser.add_mutually_exclusive_group()
and adding the three flags to that group instead of calling
parser.add_argument() on parser for them, leaving related auxiliary flags
(--workspace-description, --workspace-project-name, --workspace-relationship,
--workspace-deps) outside the group so they remain available when any single
workspace command is chosen.
Comment on lines
+102
to
+113
| base_dir = _resolve_base_dir(base_dir) | ||
|
|
||
| if not base_dir.exists(): | ||
| print_status("No workspaces found", "info") | ||
| print() | ||
| print("Create a workspace: python run.py --workspace-create <name>") | ||
| return True | ||
|
|
||
| workspaces = [] | ||
| for entry in sorted(base_dir.iterdir()): | ||
| if entry.is_dir() and (entry / "workspace.json").exists(): | ||
| workspaces.append(entry) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the full function context around lines 102-113
cat -n apps/backend/cli/multi_repo_commands.py | head -150 | tail -80Repository: OBenner/Auto-Coding
Length of output: 3371
🏁 Script executed:
# Get a broader view of the function definition
sed -n '85,125p' apps/backend/cli/multi_repo_commands.pyRepository: OBenner/Auto-Coding
Length of output: 1263
🏁 Script executed:
# Check if there's a try-except block wrapping this section
sed -n '90,115p' apps/backend/cli/multi_repo_commands.pyRepository: OBenner/Auto-Coding
Length of output: 837
🏁 Script executed:
# Verify Path.iterdir() behavior when called on a file (checking Python 3.12+ documentation/behavior)
python3 << 'EOF'
from pathlib import Path
import tempfile
import os
# Test 1: iterdir() on a file path
with tempfile.TemporaryDirectory() as tmpdir:
test_file = Path(tmpdir) / "test.txt"
test_file.write_text("test")
try:
list(test_file.iterdir())
print("Test 1 (file): No exception raised")
except NotADirectoryError as e:
print(f"Test 1 (file): NotADirectoryError raised - {type(e).__name__}")
except Exception as e:
print(f"Test 1 (file): {type(e).__name__} raised")
# Test 2: iterdir() on unreadable directory (permissions)
with tempfile.TemporaryDirectory() as tmpdir:
test_dir = Path(tmpdir) / "restricted"
test_dir.mkdir()
os.chmod(test_dir, 0o000)
try:
list(test_dir.iterdir())
print("Test 2 (no permissions): No exception raised")
except PermissionError as e:
print(f"Test 2 (no permissions): PermissionError raised")
except Exception as e:
print(f"Test 2 (no permissions): {type(e).__name__} raised")
finally:
os.chmod(test_dir, 0o755) # restore for cleanup
print("\nConclusion: iterdir() can raise exceptions for edge cases not caught by .exists() check")
EOFRepository: OBenner/Auto-Coding
Length of output: 261
Guard the workspace scan with error handling.
Lines 111–113 iterate over base_dir.iterdir() without catching exceptions. The check on line 104 only verifies that base_dir exists, but iterdir() will raise NotADirectoryError if base_dir is a file or PermissionError if it's unreadable. This violates the documented error contract (line 100: "False on error"). Wrap the loop in a try-except block to handle filesystem errors gracefully.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/backend/cli/multi_repo_commands.py` around lines 102 - 113, The
workspace scan over base_dir.iterdir() in the function using _resolve_base_dir
should be wrapped in a try/except so filesystem errors (e.g.,
NotADirectoryError, PermissionError, OSError) are handled and the function
returns False on error; catch those exceptions around the loop that builds
workspaces (the for entry in sorted(base_dir.iterdir()) block), call
print_status with an error message including the exception, and return False
instead of letting the exception propagate.
Comment on lines
+7
to
+31
| ### Workspace Management | ||
|
|
||
| ```bash | ||
| cd apps/backend | ||
|
|
||
| # Initialize a multi-repo workspace | ||
| python cli/main.py --workspace init --workspace-name "my-platform" | ||
|
|
||
| # Add a project to the workspace | ||
| python cli/main.py --workspace add --workspace-dir /path/to/workspace \ | ||
| --project-path /path/to/repo --project-role backend | ||
|
|
||
| # List projects in the workspace | ||
| python cli/main.py --workspace list --workspace-dir /path/to/workspace | ||
|
|
||
| # Remove a project | ||
| python cli/main.py --workspace remove --workspace-dir /path/to/workspace \ | ||
| --project-name my-repo | ||
|
|
||
| # Analyze cross-repo dependency graph | ||
| python cli/main.py --workspace deps --workspace-dir /path/to/workspace | ||
|
|
||
| # Show workspace status | ||
| python cli/main.py --workspace status --workspace-dir /path/to/workspace | ||
| ``` |
Contributor
There was a problem hiding this comment.
This page still describes a different CLI and config model.
The code in apps/backend/cli/main.py, apps/backend/cli/multi_repo_commands.py, and apps/backend/analysis/dependency_graph.py ships --workspace-create, --workspace-list, and --workspace-add-project; this page documents --workspace init/add/list/remove/deps/status, project-role, python cli/main.py, Cargo/Go support, and a workspace.json schema with role. Users following it will hit argparse/import errors or rely on features that do not exist in this PR.
As per coding guidelines, docs/**/*.md: Follow documentation writing conventions and use reusable templates from docs/templates/ when documenting new features, modules, or APIs. Adhere to style guide in docs/STYLE_GUIDE.md.
Also applies to: 33-59, 63-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/features/MULTI-CODEBASE-ORCHESTRATION.md` around lines 7 - 31, The docs
page MULTI-CODEBASE-ORCHESTRATION.md is out of sync with the actual CLI and
config model; update the documented CLI examples and config schema to match the
real flags used in the codebase (replace the non‑existent --workspace
init/add/list/remove/deps/status and project-role usage with the actual flags
--workspace-create, --workspace-list, --workspace-add-project, etc., and ensure
the workspace.json schema matches what apps/backend/cli and analysis code
expect), remove references to unsupported features (Cargo/Go support and python
cli/main.py invocation if not present), and reformat the page using the reusable
docs/templates and follow docs/STYLE_GUIDE.md conventions; also apply the same
corrections to the other affected blocks (lines noted in the review) so the
examples and schema accurately reflect the implementation.
Comment on lines
+5
to
+14
| Tests cover: | ||
| - Adding and removing repos (nodes) | ||
| - Adding edges between repos | ||
| - Querying dependencies and dependents | ||
| - Topological sort (build order) via Kahn's algorithm | ||
| - Cycle detection | ||
| - Affected repo traversal (BFS) | ||
| - Impact score calculation | ||
| - Serialization (to_dict / from_dict) | ||
| - Error handling (unknown repos, duplicate registration, cycles) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
The core analyze() workflow still has no regression coverage.
Every case here builds the graph manually via add_repo() / add_edge(). None create temp manifests and call analyze(), so dependency extraction, inferred-edge creation, and manual-edge preservation can regress without this suite noticing.
As per coding guidelines, tests/**: Ensure tests are comprehensive and follow pytest conventions. Check for proper mocking and test isolation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_dependency_graph.py` around lines 5 - 14, Add regression tests
that exercise the full analyze() workflow instead of manually building graphs
with add_repo()/add_edge(): create temporary package manifests in pytest
tmp_path (or tmpdir), invoke analyze() to build the graph, and assert that
dependency extraction yields inferred edges, that manually added edges (via
add_edge() or register) are preserved, and that edge deduplication/cycle
detection behaves as expected; use fixtures/mocks to isolate filesystem/IO and
ensure tests cover inferred-edge creation, manual-edge preservation, and any
transform functions used by analyze() (refer to analyze(), add_repo(),
add_edge(), and any manifest-parsing helpers) so regressions in
extraction/inference are caught.
Comment on lines
+6
to
+10
| Tests the cli/multi_repo_commands.py module including: | ||
| - handle_workspace_create_command: Create a new workspace | ||
| - handle_workspace_list_command: List existing workspaces | ||
| - handle_workspace_add_project_command: Add a project to a workspace | ||
| """ |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
main.py dispatch coverage is still missing.
These tests invoke the handlers directly, but the PR also changes apps/backend/cli/main.py to parse --workspace-create, --workspace-list, --workspace-add-project, and split --workspace-deps. A thin parse_args() / _run_cli() test would catch flag-name and argument-shaping regressions here.
As per coding guidelines, tests/**: Ensure tests are comprehensive and follow pytest conventions. Check for proper mocking and test isolation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_multi_repo_commands.py` around lines 6 - 10, Add a thin
integration test that exercises the CLI entry points by calling parse_args and
_run_cli (or whatever parse/dispatch functions were added to main.py) to
validate the new flags --workspace-create, --workspace-list,
--workspace-add-project and the split handling of --workspace-deps;
specifically, call the parsing function with representative argv arrays and
assert the parsed namespace/args shape, then call the dispatcher with those args
while mocking the handler functions (handle_workspace_create_command,
handle_workspace_list_command, handle_workspace_add_project_command) and any
I/O/side-effects so tests remain isolated and follow pytest conventions. Ensure
you include tests for both presence and combinations of flags, verify the
correct handler is invoked with expected arguments, and use pytest
fixtures/mocks to avoid touching real state.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enhance feature-69 with multi-repo management
Summary by CodeRabbit
New Features
Documentation