Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 1, 2025

  • Issue 1: stop.py Implementation - Fix stop_background_services() to use PID file approach instead of killing the CLI process
  • Issue 2: Statistics Caching Bug - Remove @cache decorator from statistics() function in management.py
  • Issue 3: Unnecessary Global Statement - Remove global statistics statement from management.py
  • Issue 4: Telemetry Confusing Nesting - Rename inner "query" key to "text" in SearchEvent to avoid confusing properties["query"]["query"] nesting
  • Issue 5: CI/CD Failure - Fix syntax error in .github/actions/setup-python-env/action.yml line 107 where -E ci is incorrectly placed
  • Run linting and tests to verify changes
Original prompt

@copilot Please address the following suggestions in #172:
Code

788 tests maintained across 74 files - comprehensive coverage

Areas for Improvement Issue 1: stop.py Implementation

File: src/codeweaver/cli/commands/stop.py:44-47

async def stop_background_services() -> None: # Use signal-based shutdown (more secure than HTTP endpoint) os.kill(os.getpid(), signal.SIGTERM) # ⚠️ This kills the CLI process!

Problem: This sends SIGTERM to the current CLI process, not to background services.

Impact: cw stop won't actually stop background services.

Recommendation: Consider one of these approaches:
Code

PID file approach (most robust for daemon processes)

Management server shutdown endpoint (simpler, works for same-process architecture)

Document as placeholder if separate-process architecture is coming soon

Example PID file approach:
In start.py - write PID

pidfile = Path.home() / ".codeweaver" / "server.pid" pidfile.write_text(str(os.getpid()))
In stop.py - read and signal

pidfile = Path.home() / ".codeweaver" / "server.pid" if pidfile.exists(): pid = int(pidfile.read_text()) os.kill(pid, signal.SIGTERM) pidfile.unlink()

Issue 2: Statistics Caching Bug

File: src/codeweaver/server/management.py:43-46

@cache def statistics() -> SessionStatistics: return get_session_statistics()

Problem: The @cache decorator caches the first return value permanently. Statistics will never update after the first call.

Impact: /stats endpoint will return stale data.

Fix: Remove @cache or implement explicit cache invalidation with TTL:
Option 1: Remove caching <--- Use this option.

def statistics() -> SessionStatistics: return get_session_statistics()
Option 2: TTL cache (if performance is a concern)

from functools import lru_cache import time

_cache_time = 0 _cache_value = None

def statistics() -> SessionStatistics: global _cache_time, _cache_value now = time.time() if now - _cache_time > 5: # 5 second cache _cache_value = get_session_statistics() _cache_time = now return _cache_value

Issue 3: Unnecessary Global Statement

File: src/codeweaver/server/management.py:61

global statistics # ← Not needed, statistics is not being assigned

Fix: Remove the global statement.

Issue 4 The comment states "The query dict includes redacted query and results, not a hash" but the assertion checks for "query" within properties["query"]. This creates a confusing nested structure where properties["query"]["query"] contains the redacted query string. Consider renaming the inner key to something clearer like "redacted_query" or "text" to avoid this confusing nesting. (in telemetry package)

Finally, identify and resolve the CI/CD failures. All tests were passing locally when committed, so this is likely a CI issue vice a tests issue.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@bashandbone
Copy link
Contributor

@copilot -- your last run failed, I think because of the syntax issue in action.yml .... I have fixed that. Please try again.

@bashandbone
Copy link
Contributor

Closing due to repeated copilot failures

@bashandbone bashandbone closed this Dec 1, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Dec 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants