Skip to content

Commit 91aaf88

Browse files
Copilotncrmro
andcommitted
Address code review feedback
- Fix ValueError handling in port command's relative path calculation - Improve test clarity for duplicate job names in different scopes - Rename test to better reflect what it's actually testing Co-authored-by: ncrmro <8276365+ncrmro@users.noreply.github.com>
1 parent 3acff99 commit 91aaf88

2 files changed

Lines changed: 30 additions & 10 deletions

File tree

src/deepwork/cli/port.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,16 @@ def _port_job(project_path: Path) -> None:
163163
console.print()
164164
console.print("[bold]Next steps:[/bold]")
165165
console.print(" 1. Run [cyan]deepwork sync[/cyan] to regenerate skills")
166-
console.print(
167-
f" 2. The job is now available in {dest_name} ({dest_job_path.relative_to(project_path) if dest_scope == JobScope.LOCAL else dest_job_path})"
168-
)
166+
167+
# Display the destination path safely
168+
if dest_scope == JobScope.LOCAL:
169+
try:
170+
relative_path = dest_job_path.relative_to(project_path)
171+
console.print(f" 2. The job is now available in {dest_name} ({relative_path})")
172+
except ValueError:
173+
# Fallback if path is not relative
174+
console.print(f" 2. The job is now available in {dest_name} ({dest_job_path})")
175+
else:
176+
console.print(f" 2. The job is now available in {dest_name} ({dest_job_path})")
177+
169178
console.print()

tests/integration/test_global_jobs.py

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,15 @@ def test_sync_discovers_global_jobs(
6666
assert "global_test_job" in result.output
6767
assert "Loaded global_test_job v1.0.0" in result.output
6868

69-
def test_sync_prefers_local_over_global_for_duplicates(
69+
def test_sync_loads_both_local_and_global_jobs_with_same_name(
7070
self, mock_claude_project: Path, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
7171
) -> None:
72-
"""Test that local jobs take precedence over global jobs with the same name."""
72+
"""Test that sync loads both local and global jobs even when they have the same name.
73+
74+
Note: Both jobs are loaded because they are in physically different directories.
75+
This is different from duplicate detection - DeepWork allows the same job name
76+
in different scopes, treating them as separate jobs.
77+
"""
7378
# Set up temporary global directory
7479
global_config = tmp_path / "config"
7580
monkeypatch.setenv("XDG_CONFIG_HOME", str(global_config))
@@ -119,12 +124,18 @@ def test_sync_prefers_local_over_global_for_duplicates(
119124
)
120125

121126
assert result.exit_code == 0
122-
# Should load both jobs (they are in different directories, so no conflict)
123-
# But the local one is listed first
127+
# Both jobs should be loaded (they are in different physical directories)
124128
output_lines = result.output.split("\n")
125-
duplicate_lines = [line for line in output_lines if "duplicate_job" in line]
126-
# Should see exactly 2 references (one for each scope's discovery + one for loading each)
127-
assert len(duplicate_lines) >= 2
129+
130+
# Check that we found jobs in both scopes
131+
assert any("local scope" in line for line in output_lines)
132+
assert any("global scope" in line for line in output_lines)
133+
134+
# Should see duplicate_job loaded twice (once from each location)
135+
duplicate_loaded_lines = [
136+
line for line in output_lines if "Loaded duplicate_job" in line
137+
]
138+
assert len(duplicate_loaded_lines) == 2, "Should load duplicate_job from both locations"
128139

129140
def test_global_jobs_dir_respects_xdg_config_home(
130141
self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path

0 commit comments

Comments
 (0)