fix: fix encoding issues in file operations and encoding checker
- Fix check_encoding.py to handle nested parentheses in open() calls - Add encoding="utf-8" to file operations in GitLab test files - Add encoding to bot_detection.py and file_lock.py - Fix trailing whitespace and end-of-file issues (auto-fixed)
This commit is contained in:
+2
-2
@@ -185,7 +185,7 @@ if git diff --cached --name-only | grep -q "^apps/backend/.*\.py$"; then
|
||||
# Tests to skip: graphiti (external deps), merge_file_tracker/service_orchestrator/worktree/workspace (Windows path/git issues)
|
||||
# Also skip tests that require optional dependencies (pydantic structured outputs)
|
||||
IGNORE_TESTS="--ignore=../../tests/test_graphiti.py --ignore=../../tests/test_merge_file_tracker.py --ignore=../../tests/test_service_orchestrator.py --ignore=../../tests/test_worktree.py --ignore=../../tests/test_workspace.py --ignore=../../tests/test_finding_validation.py --ignore=../../tests/test_sdk_structured_output.py --ignore=../../tests/test_structured_outputs.py"
|
||||
|
||||
|
||||
# Determine Python executable from venv
|
||||
VENV_PYTHON=""
|
||||
if [ -f ".venv/bin/python" ]; then
|
||||
@@ -193,7 +193,7 @@ if git diff --cached --name-only | grep -q "^apps/backend/.*\.py$"; then
|
||||
elif [ -f ".venv/Scripts/python.exe" ]; then
|
||||
VENV_PYTHON=".venv/Scripts/python.exe"
|
||||
fi
|
||||
|
||||
|
||||
if [ -n "$VENV_PYTHON" ]; then
|
||||
# Check if pytest is installed in venv
|
||||
if $VENV_PYTHON -c "import pytest" 2>/dev/null; then
|
||||
|
||||
@@ -71,7 +71,9 @@ class TestFileLock:
|
||||
def try_write(value):
|
||||
try:
|
||||
with FileLock(lock_file, timeout=1.0, exclusive=True):
|
||||
with open(lock_file.with_suffix(".txt"), "w") as f:
|
||||
with open(
|
||||
lock_file.with_suffix(".txt"), "w", encoding="utf-8"
|
||||
) as f:
|
||||
f.write(str(value))
|
||||
results.append(value)
|
||||
except Exception:
|
||||
@@ -122,14 +124,14 @@ class TestAtomicWrite:
|
||||
f.write("test content")
|
||||
|
||||
assert target_file.exists()
|
||||
assert target_file.read_text() == "test content"
|
||||
assert target_file.read_text(encoding="utf-8") == "test content"
|
||||
|
||||
def test_atomic_write_preserves_on_error(self, target_file):
|
||||
"""Test atomic write doesn't corrupt on error."""
|
||||
from runners.gitlab.utils.file_lock import atomic_write
|
||||
|
||||
# Create initial content
|
||||
target_file.write_text("original content")
|
||||
target_file.write_text("original content", encoding="utf-8")
|
||||
|
||||
try:
|
||||
with atomic_write(target_file) as f:
|
||||
@@ -139,7 +141,7 @@ class TestAtomicWrite:
|
||||
pass
|
||||
|
||||
# Original content should be preserved
|
||||
assert target_file.read_text() == "original content"
|
||||
assert target_file.read_text(encoding="utf-8") == "original content"
|
||||
|
||||
def test_atomic_write_context_manager(self, target_file):
|
||||
"""Test atomic write context manager."""
|
||||
@@ -149,7 +151,7 @@ class TestAtomicWrite:
|
||||
f.write("line 1\n")
|
||||
f.write("line 2\n")
|
||||
|
||||
content = target_file.read_text()
|
||||
content = target_file.read_text(encoding="utf-8")
|
||||
assert "line 1" in content
|
||||
assert "line 2" in content
|
||||
|
||||
@@ -171,7 +173,7 @@ class TestLockedJsonOperations:
|
||||
locked_json_write(data_file, data)
|
||||
|
||||
assert data_file.exists()
|
||||
with open(data_file) as f:
|
||||
with open(data_file, encoding="utf-8") as f:
|
||||
loaded = json.load(f)
|
||||
assert loaded == data
|
||||
|
||||
@@ -271,7 +273,7 @@ class TestLockedReadWrite:
|
||||
with locked_write(data_file) as f:
|
||||
f.write("test content")
|
||||
|
||||
assert data_file.read_text() == "test content"
|
||||
assert data_file.read_text(encoding="utf-8") == "test content"
|
||||
|
||||
def test_locked_read(self, data_file):
|
||||
"""Test reading with lock."""
|
||||
@@ -293,7 +295,7 @@ class TestLockedReadWrite:
|
||||
with locked_write(data_file, lock=None) as f:
|
||||
f.write("custom lock")
|
||||
|
||||
assert data_file.read_text() == "custom lock"
|
||||
assert data_file.read_text(encoding="utf-8") == "custom lock"
|
||||
|
||||
|
||||
class TestFileLockError:
|
||||
|
||||
@@ -87,7 +87,7 @@ class BotDetectionState:
|
||||
if not state_file.exists():
|
||||
return cls()
|
||||
|
||||
with open(state_file) as f:
|
||||
with open(state_file, encoding="utf-8") as f:
|
||||
return cls.from_dict(json.load(f))
|
||||
|
||||
|
||||
|
||||
@@ -348,7 +348,7 @@ async def locked_read(filepath: str | Path, timeout: float = 5.0) -> Any:
|
||||
|
||||
try:
|
||||
# Open file for reading
|
||||
with open(filepath) as f:
|
||||
with open(filepath, encoding="utf-8") as f:
|
||||
yield f
|
||||
finally:
|
||||
# Release lock
|
||||
@@ -441,7 +441,7 @@ async def locked_json_update(
|
||||
# Read current data
|
||||
def _read_json():
|
||||
if filepath.exists():
|
||||
with open(filepath) as f:
|
||||
with open(filepath, encoding="utf-8") as f:
|
||||
return json.load(f)
|
||||
return None
|
||||
|
||||
|
||||
@@ -379,4 +379,4 @@ describe('Task Lifecycle Integration', () => {
|
||||
});
|
||||
});
|
||||
|
||||
});
|
||||
});
|
||||
|
||||
@@ -28,7 +28,7 @@ function setupTestDirs(): void {
|
||||
TEST_DIR = mkdtempSync(path.join(tmpdir(), 'project-store-test-'));
|
||||
USER_DATA_PATH = path.join(TEST_DIR, 'userData');
|
||||
TEST_PROJECT_PATH = path.join(TEST_DIR, 'test-project');
|
||||
|
||||
|
||||
mkdirSync(USER_DATA_PATH, { recursive: true });
|
||||
mkdirSync(path.join(USER_DATA_PATH, 'store'), { recursive: true });
|
||||
mkdirSync(TEST_PROJECT_PATH, { recursive: true });
|
||||
|
||||
@@ -19,11 +19,11 @@ export class UsageMonitor extends EventEmitter {
|
||||
private currentUsage: ClaudeUsageSnapshot | null = null;
|
||||
private isChecking = false;
|
||||
private useApiMethod = true; // Try API first, fall back to CLI if it fails
|
||||
|
||||
|
||||
// Swap loop protection: track profiles that recently failed auth
|
||||
private authFailedProfiles: Map<string, number> = new Map(); // profileId -> timestamp
|
||||
private static AUTH_FAILURE_COOLDOWN_MS = 5 * 60 * 1000; // 5 minutes cooldown
|
||||
|
||||
|
||||
// Debug flag for verbose logging
|
||||
private readonly isDebug = process.env.DEBUG === 'true';
|
||||
|
||||
@@ -159,12 +159,12 @@ export class UsageMonitor extends EventEmitter {
|
||||
if ((error as any).statusCode === 401 || (error as any).statusCode === 403) {
|
||||
const profileManager = getClaudeProfileManager();
|
||||
const activeProfile = profileManager.getActiveProfile();
|
||||
|
||||
|
||||
if (activeProfile) {
|
||||
// Mark this profile as auth-failed to prevent swap loops
|
||||
this.authFailedProfiles.set(activeProfile.id, Date.now());
|
||||
console.warn('[UsageMonitor] Auth failure detected, marked profile as failed:', activeProfile.id);
|
||||
|
||||
|
||||
// Clean up expired entries from the failed profiles map
|
||||
const now = Date.now();
|
||||
this.authFailedProfiles.forEach((timestamp, profileId) => {
|
||||
@@ -172,7 +172,7 @@ export class UsageMonitor extends EventEmitter {
|
||||
this.authFailedProfiles.delete(profileId);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
try {
|
||||
const excludeProfiles = Array.from(this.authFailedProfiles.keys());
|
||||
console.warn('[UsageMonitor] Attempting proactive swap (excluding failed profiles):', excludeProfiles);
|
||||
@@ -287,7 +287,7 @@ export class UsageMonitor extends EventEmitter {
|
||||
if (error?.statusCode === 401 || error?.statusCode === 403) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
|
||||
console.error('[UsageMonitor] API fetch failed:', error);
|
||||
return null;
|
||||
}
|
||||
@@ -347,12 +347,12 @@ export class UsageMonitor extends EventEmitter {
|
||||
additionalExclusions: string[] = []
|
||||
): Promise<void> {
|
||||
const profileManager = getClaudeProfileManager();
|
||||
|
||||
|
||||
// Get all profiles to swap to, excluding current and any additional exclusions
|
||||
const allProfiles = profileManager.getProfilesSortedByAvailability();
|
||||
const excludeIds = new Set([currentProfileId, ...additionalExclusions]);
|
||||
const eligibleProfiles = allProfiles.filter(p => !excludeIds.has(p.id));
|
||||
|
||||
|
||||
if (eligibleProfiles.length === 0) {
|
||||
console.warn('[UsageMonitor] No alternative profile for proactive swap (excluded:', Array.from(excludeIds), ')');
|
||||
this.emit('proactive-swap-failed', {
|
||||
@@ -362,7 +362,7 @@ export class UsageMonitor extends EventEmitter {
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
// Use the best available from eligible profiles
|
||||
const bestProfile = eligibleProfiles[0];
|
||||
|
||||
|
||||
@@ -151,14 +151,14 @@ export function StagedInProjectMessage({ task, projectPath, hasWorktree = false,
|
||||
|
||||
const handleReviewAgain = async () => {
|
||||
if (!onReviewAgain) return;
|
||||
|
||||
|
||||
setIsResetting(true);
|
||||
setError(null);
|
||||
|
||||
try {
|
||||
// Clear the staged flag via IPC
|
||||
const result = await window.electronAPI.clearStagedState(task.id);
|
||||
|
||||
|
||||
if (!result.success) {
|
||||
setError(result.error || 'Failed to reset staged state');
|
||||
return;
|
||||
@@ -238,7 +238,7 @@ export function StagedInProjectMessage({ task, projectPath, hasWorktree = false,
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
|
||||
|
||||
{/* Secondary actions row */}
|
||||
<div className="flex gap-2">
|
||||
{/* Mark Done Only (when worktree exists) - allows keeping worktree */}
|
||||
@@ -263,7 +263,7 @@ export function StagedInProjectMessage({ task, projectPath, hasWorktree = false,
|
||||
)}
|
||||
</Button>
|
||||
)}
|
||||
|
||||
|
||||
{/* Review Again button - only show if worktree exists and callback provided */}
|
||||
{hasWorktree && onReviewAgain && (
|
||||
<Button
|
||||
@@ -287,11 +287,11 @@ export function StagedInProjectMessage({ task, projectPath, hasWorktree = false,
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
|
||||
|
||||
{error && (
|
||||
<p className="text-xs text-destructive">{error}</p>
|
||||
)}
|
||||
|
||||
|
||||
{hasWorktree && (
|
||||
<p className="text-xs text-muted-foreground">
|
||||
"Delete Worktree & Mark Done" cleans up the isolated workspace. "Mark Done Only" keeps it for reference.
|
||||
|
||||
@@ -50,8 +50,21 @@ class EncodingChecker:
|
||||
# Check 1: open() without encoding
|
||||
# Pattern: open(...) without encoding= parameter
|
||||
# Use negative lookbehind to exclude os.open(), urlopen(), etc.
|
||||
for match in re.finditer(r'(?<![a-zA-Z_\.])open\s*\([^)]+\)', content):
|
||||
call = match.group()
|
||||
# We need to find the full open() call including nested parentheses
|
||||
for match in re.finditer(r'(?<![a-zA-Z_\.])open\s*\(', content):
|
||||
start_pos = match.end()
|
||||
|
||||
# Find the matching closing parenthesis (handle nesting)
|
||||
paren_depth = 1
|
||||
end_pos = start_pos
|
||||
while end_pos < len(content) and paren_depth > 0:
|
||||
if content[end_pos] == '(':
|
||||
paren_depth += 1
|
||||
elif content[end_pos] == ')':
|
||||
paren_depth -= 1
|
||||
end_pos += 1
|
||||
|
||||
call = content[match.start():end_pos]
|
||||
|
||||
# Skip if it's binary mode (must contain 'b' in mode string)
|
||||
# Matches: "rb", "wb", "ab", "r+b", "w+b", etc.
|
||||
|
||||
Reference in New Issue
Block a user