Skip to content

Commit ac29f94

Browse files
committed
fix: address Copilot review feedback (PR #1232)
- Fix Python 3.9 typing compatibility (use typing module) - Remove unused imports: defaultdict in dependency_analysis.py - Remove unused variables: func_start, since_date, agent_id - Add explanatory comments for except pass blocks - Fix uv pip install command in code-analysis.yml - Update README.md to say Python 3.9+ instead of 3.10+ - Remove unused gitpython dependency from pyproject.toml - Rename manager-discovery skill to python-manager-discovery - Quote # symbol in cross-platform-paths description - Set user-invocable: false for reference skills - Remove / prefix from skill name references in agents/hooks
1 parent fcca3e8 commit ac29f94

15 files changed

Lines changed: 42 additions & 43 deletions

File tree

.github/agents/maintainer.agent.md

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,13 @@ tools:
6767

6868
Load these skills on-demand for detailed knowledge:
6969

70-
| Skill | When to Use |
71-
| ------------------------ | ------------------------------------------------- |
72-
| `/generate-snapshot` | Generate codebase health snapshot for planning |
73-
| `/run-pre-commit-checks` | Run mandatory checks before committing |
74-
| `/cross-platform-paths` | Reviewing/writing path-related code |
75-
| `/settings-precedence` | Reviewing/writing settings code |
76-
| `/manager-discovery` | Working on specific manager (poetry, conda, etc.) |
70+
| Skill | When to Use |
71+
| -------------------------- | ------------------------------------------------- |
72+
| `generate-snapshot` | Generate codebase health snapshot for planning |
73+
| `run-pre-commit-checks` | Run mandatory checks before committing |
74+
| `cross-platform-paths` | Reviewing/writing path-related code |
75+
| `settings-precedence` | Reviewing/writing settings code |
76+
| `python-manager-discovery` | Working on specific manager (poetry, conda, etc.) |
7777

7878
---
7979

@@ -103,7 +103,7 @@ Planning → Development → Review → Merge
103103

104104
1. **Gather context:**
105105
- Check open GitHub issues (`github/list_issues`, `github/search_issues`)
106-
- Generate snapshot: use `/generate-snapshot` skill for details
106+
- Generate snapshot: Use `generate-snapshot` skill for details
107107
- Check open PRs for related work
108108

109109
2. **Analyze and prioritize:**
@@ -140,9 +140,9 @@ git checkout -b feature/issue-N # or bug/issue-N, chore/issue-N
140140

141141
Follow guidelines from `.github/instructions/generic.instructions.md`:
142142

143-
- **Paths:** Use `/cross-platform-paths` skill for patterns
144-
- **Settings:** Use `/settings-precedence` skill for patterns
145-
- **Managers:** Use `/manager-discovery` skill for manager-specific knowledge
143+
- **Paths:** Use `cross-platform-paths` skill for patterns
144+
- **Settings:** Use `settings-precedence` skill for patterns
145+
- **Managers:** Use `python-manager-discovery` skill for manager-specific knowledge
146146
- **Localization:** `l10n.t()` for user-facing messages
147147
- **Logging:** `traceLog`/`traceVerbose`, never `console.log`
148148

@@ -172,7 +172,7 @@ Run **Reviewer** agent (`.github/agents/reviewer.agent.md`) with:
172172

173173
## 5. Pre-Commit Checks (REQUIRED)
174174

175-
Use `/run-pre-commit-checks` skill for details. Quick reference:
175+
Use `run-pre-commit-checks` skill for details. Quick reference:
176176

177177
```powershell
178178
npm run lint # ESLint

.github/agents/reviewer.agent.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ Automated reviews consistently miss:
4141

4242
For deep-dive patterns, these skills provide additional context:
4343

44-
| Skill | Use When |
45-
| ----------------------- | ------------------------------- |
46-
| `/cross-platform-paths` | Reviewing path-related code |
47-
| `/settings-precedence` | Reviewing settings code |
48-
| `/manager-discovery` | Reviewing manager-specific code |
44+
| Skill | Use When |
45+
| -------------------------- | ------------------------------- |
46+
| `cross-platform-paths` | Reviewing path-related code |
47+
| `settings-precedence` | Reviewing settings code |
48+
| `python-manager-discovery` | Reviewing manager-specific code |
4949

5050
The patterns below are the essential subset needed during reviews.
5151

.github/hooks/scripts/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ Runs when subagents complete:
3838

3939
## Requirements
4040

41-
These scripts use Python 3.10+ with no external dependencies (beyond what's already in the repo).
41+
These scripts use Python 3.9+ with no external dependencies (beyond what's already in the repo).
4242

4343
They expect:
4444

.github/hooks/scripts/post_tool_use.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import subprocess
1414
import sys
1515
from pathlib import Path
16+
from typing import Dict, List, Optional
1617

1718
# Tools that modify files and should trigger validation
1819
FILE_EDIT_TOOLS = {"editFiles", "createFile", "create_file", "replace_string_in_file"}
@@ -21,7 +22,7 @@
2122
TYPESCRIPT_EXTENSIONS = {".ts", ".tsx"}
2223

2324

24-
def run_eslint(files: list[str], cwd: Path) -> str | None:
25+
def run_eslint(files: List[str], cwd: Path) -> Optional[str]:
2526
"""Run ESLint on specified files and return errors."""
2627
ts_files = [f for f in files if Path(f).suffix in TYPESCRIPT_EXTENSIONS]
2728
if not ts_files:
@@ -61,12 +62,14 @@ def run_eslint(files: list[str], cwd: Path) -> str | None:
6162

6263
return f"ESLint: {', '.join(summary)}. " + " | ".join(sample_errors[:3])
6364
except (subprocess.TimeoutExpired, FileNotFoundError):
65+
# ESLint failures (timeout or missing binary) should not block the hook;
66+
# treat them as "no lint results" and continue without reporting lint output.
6467
pass
6568

6669
return None
6770

6871

69-
def extract_files_from_tool_input(tool_name: str, tool_input: dict) -> list[str]:
72+
def extract_files_from_tool_input(tool_name: str, tool_input: Dict) -> List[str]:
7073
"""Extract file paths from tool input based on tool type."""
7174
files = []
7275

.github/hooks/scripts/session_start.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@
1515
import subprocess
1616
import sys
1717
from pathlib import Path
18+
from typing import Dict, List, Optional
1819

1920

20-
def run_command(cmd: list[str], cwd: Path | None = None) -> str | None:
21+
def run_command(cmd: List[str], cwd: Optional[Path] = None) -> Optional[str]:
2122
"""Run a command and return stdout, or None on failure."""
2223
try:
2324
result = subprocess.run(
@@ -30,11 +31,12 @@ def run_command(cmd: list[str], cwd: Path | None = None) -> str | None:
3031
if result.returncode == 0:
3132
return result.stdout.strip()
3233
except (subprocess.TimeoutExpired, FileNotFoundError):
34+
# Git/gh CLI not available or timed out; return None to skip this context.
3335
pass
3436
return None
3537

3638

37-
def get_git_context(repo_root: Path) -> dict:
39+
def get_git_context(repo_root: Path) -> Dict:
3840
"""Get current git context."""
3941
context = {}
4042

@@ -65,7 +67,7 @@ def get_git_context(repo_root: Path) -> dict:
6567
return context
6668

6769

68-
def get_issue_context(repo_root: Path) -> dict:
70+
def get_issue_context(repo_root: Path) -> Dict:
6971
"""Get open issue context if gh CLI is available."""
7072
context = {}
7173

@@ -97,7 +99,7 @@ def get_issue_context(repo_root: Path) -> dict:
9799
return context
98100

99101

100-
def get_snapshot_summary(repo_root: Path) -> dict:
102+
def get_snapshot_summary(repo_root: Path) -> Dict:
101103
"""Get snapshot summary if available."""
102104
snapshot_path = repo_root / "analysis" / "analysis-snapshot.json"
103105
if not snapshot_path.exists():
@@ -115,6 +117,7 @@ def get_snapshot_summary(repo_root: Path) -> dict:
115117
"circular_dependencies": summary.get("circular_dependency_count", 0),
116118
}
117119
except (json.JSONDecodeError, OSError):
120+
# Snapshot file unreadable or malformed; skip snapshot context.
118121
return {}
119122

120123

@@ -155,8 +158,8 @@ def main() -> int:
155158

156159
# Add reminder about skills
157160
parts.append(
158-
"Available skills: /generate-snapshot, /run-pre-commit-checks, "
159-
"/cross-platform-paths, /settings-precedence, /manager-discovery"
161+
"Available skills: generate-snapshot, run-pre-commit-checks, "
162+
"cross-platform-paths, settings-precedence, python-manager-discovery"
160163
)
161164

162165
# Output response

.github/hooks/scripts/stop_hook.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@
1313
import subprocess
1414
import sys
1515
from pathlib import Path
16+
from typing import List, Optional, Tuple
1617

1718

18-
def run_command(cmd: list[str], cwd: Path | None = None) -> tuple[int, str]:
19+
def run_command(cmd: List[str], cwd: Optional[Path] = None) -> Tuple[int, str]:
1920
"""Run a command and return (exit_code, output)."""
2021
try:
2122
result = subprocess.run(
@@ -96,7 +97,7 @@ def main() -> int:
9697
"decision": "block",
9798
"reason": (
9899
"You have uncommitted TypeScript changes. "
99-
"Before finishing, run /run-pre-commit-checks skill "
100+
"Before finishing, use the run-pre-commit-checks skill "
100101
"or manually run: npm run lint && npm run compile-tests && npm run unittest. "
101102
"If checks pass and changes are ready, commit them. "
102103
"If this session is just research/exploration, you can proceed without committing."

.github/hooks/scripts/subagent_stop.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def main() -> int:
2121
input_data = {}
2222

2323
agent_type = input_data.get("agent_type", "")
24-
agent_id = input_data.get("agent_id", "")
24+
# agent_id available in input_data if needed for logging
2525
stop_hook_active = input_data.get("stop_hook_active", False)
2626

2727
# Prevent infinite loops

.github/skills/cross-platform-paths/SKILL.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
---
22
name: cross-platform-paths
3-
description: Critical patterns for cross-platform path handling in this VS Code extension. Windows vs POSIX path bugs are the #1 source of issues. Use this skill when reviewing or writing path-related code.
3+
description: 'Critical patterns for cross-platform path handling in this VS Code extension. Windows vs POSIX path bugs are the #1 source of issues. Use this skill when reviewing or writing path-related code.'
44
argument-hint: Review path handling in [file or component]
5+
user-invocable: false
56
---
67

78
# Cross-Platform Path Handling

.github/skills/manager-discovery/SKILL.md renamed to .github/skills/python-manager-discovery/SKILL.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
---
2-
name: manager-discovery
2+
name: python-manager-discovery
33
description: Environment manager-specific discovery patterns and known issues. Use when working on or reviewing environment discovery code for conda, poetry, pipenv, pyenv, or venv.
44
argument-hint: 'manager name (e.g., poetry, conda, pyenv)'
5-
user-invocable: true
5+
user-invocable: false
66
---
77

88
# Environment Manager Discovery Patterns

.github/skills/settings-precedence/SKILL.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
name: settings-precedence
33
description: VS Code settings precedence rules and common pitfalls. Essential for any code that reads or writes settings. Covers getConfiguration scope, inspect() vs get(), and multi-workspace handling.
44
argument-hint: Review settings handling in [file or component]
5+
user-invocable: false
56
---
67

78
# VS Code Settings Precedence

0 commit comments

Comments
 (0)