fix(github): resolve PR review hanging in bundled app (#1793)
* fix(github): resolve PR review hanging in bundled app Use getEffectiveSourcePath() and getConfiguredPythonPath() in subprocess-runner.ts so the GitHub PR review runner correctly locates the backend and Python executable in packaged Electron builds — same pattern already used by title-generator and insights. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(github): remove dead code and update stale JSDoc Address PR review findings: - Remove unused fileURLToPath import, __filename and __dirname declarations - Update getBackendPath() JSDoc to reflect new path resolution strategy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(github): guard getPythonPath managed env with isEnvReady check Only use the managed Python path when pythonEnvManager.isEnvReady() is true, preventing the bare 'python' fallback from getConfiguredPythonPath() from being used when the managed env isn't set up. The backendPath .venv fallback remains for dev mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -9,12 +9,7 @@ import { spawn, exec, execFile } from 'child_process';
|
||||
import type { ChildProcess } from 'child_process';
|
||||
import { promisify } from 'util';
|
||||
import path from 'path';
|
||||
import { fileURLToPath } from 'url';
|
||||
import fs from 'fs';
|
||||
|
||||
// ESM-compatible __dirname
|
||||
const __filename = fileURLToPath(import.meta.url);
|
||||
const __dirname = path.dirname(__filename);
|
||||
import type { Project } from '../../../../shared/types';
|
||||
import type { AuthFailureInfo, BillingFailureInfo } from '../../../../shared/types/terminal';
|
||||
import { parsePythonCommand } from '../../../python-detector';
|
||||
@@ -22,6 +17,8 @@ import { detectAuthFailure, detectBillingFailure } from '../../../rate-limit-det
|
||||
import { getClaudeProfileManager } from '../../../claude-profile-manager';
|
||||
import { getOperationRegistry, type OperationType } from '../../../claude-profile/operation-registry';
|
||||
import { isWindows, isMacOS } from '../../../platform';
|
||||
import { getEffectiveSourcePath } from '../../../updater/path-resolver';
|
||||
import { pythonEnvManager, getConfiguredPythonPath } from '../../../python-env-manager';
|
||||
import { getTaskkillExePath, getWhereExePath } from '../../../utils/windows-paths';
|
||||
|
||||
const execAsync = promisify(exec);
|
||||
@@ -479,10 +476,20 @@ export function runPythonSubprocess<T = unknown>(
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the Python path for a project's backend
|
||||
* Cross-platform: uses Scripts/python.exe on Windows, bin/python on Unix
|
||||
* Get the Python path for running GitHub runners.
|
||||
*
|
||||
* Prefers the managed Python environment (bundled app venv) when ready,
|
||||
* falls back to project-local .venv for development repos.
|
||||
*/
|
||||
export function getPythonPath(backendPath: string): string {
|
||||
// Use managed env when it's fully set up (has dependencies installed)
|
||||
if (pythonEnvManager.isEnvReady()) {
|
||||
const managed = getConfiguredPythonPath();
|
||||
if (fs.existsSync(managed)) {
|
||||
return managed;
|
||||
}
|
||||
}
|
||||
// Fallback to venv in backend path (dev mode)
|
||||
return isWindows()
|
||||
? path.join(backendPath, '.venv', 'Scripts', 'python.exe')
|
||||
: path.join(backendPath, '.venv', 'bin', 'python');
|
||||
@@ -498,44 +505,30 @@ export function getRunnerPath(backendPath: string): string {
|
||||
/**
|
||||
* Get the auto-claude backend path for a project
|
||||
*
|
||||
* Auto-detects the backend location using multiple strategies:
|
||||
* 1. Development repo structure (apps/backend)
|
||||
* 2. Electron app bundle location
|
||||
* 3. Current working directory
|
||||
* Uses getEffectiveSourcePath() which handles:
|
||||
* 1. User settings (autoBuildPath)
|
||||
* 2. userData override (backend-source) for user-updated backend
|
||||
* 3. Bundled backend (process.resourcesPath/backend)
|
||||
* 4. Development paths
|
||||
* Falls back to project.path/apps/backend for development repos.
|
||||
*/
|
||||
export function getBackendPath(project: Project): string | null {
|
||||
// Import app module for production path detection
|
||||
let app: any;
|
||||
try {
|
||||
app = require('electron').app;
|
||||
} catch {
|
||||
// Electron not available in tests
|
||||
// Use shared path resolver which handles:
|
||||
// 1. User settings (autoBuildPath)
|
||||
// 2. userData override (backend-source) for user-updated backend
|
||||
// 3. Bundled backend (process.resourcesPath/backend)
|
||||
// 4. Development paths
|
||||
const effectivePath = getEffectiveSourcePath();
|
||||
if (fs.existsSync(effectivePath) && fs.existsSync(path.join(effectivePath, 'runners', 'github', 'runner.py'))) {
|
||||
return effectivePath;
|
||||
}
|
||||
|
||||
// Check if this is a development repo (has apps/backend structure)
|
||||
// Fallback: check project path for development repo structure
|
||||
const appsBackendPath = path.join(project.path, 'apps', 'backend');
|
||||
if (fs.existsSync(path.join(appsBackendPath, 'runners', 'github', 'runner.py'))) {
|
||||
return appsBackendPath;
|
||||
}
|
||||
|
||||
// Auto-detect from app location (same logic as agent-process.ts)
|
||||
const possiblePaths = [
|
||||
// Dev mode: from dist/main -> ../../backend (apps/frontend/out/main -> apps/backend)
|
||||
path.resolve(__dirname, '..', '..', '..', '..', '..', 'backend'),
|
||||
// Alternative: from app root -> apps/backend
|
||||
app ? path.resolve(app.getAppPath(), '..', 'backend') : null,
|
||||
// If running from repo root with apps structure
|
||||
path.resolve(process.cwd(), 'apps', 'backend'),
|
||||
].filter((p): p is string => p !== null);
|
||||
|
||||
for (const backendPath of possiblePaths) {
|
||||
// Check for runner.py as marker
|
||||
const runnerPath = path.join(backendPath, 'runners', 'github', 'runner.py');
|
||||
if (fs.existsSync(runnerPath)) {
|
||||
return backendPath;
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user