From fd860963d10a54dbc3b68ade0935e89edb0c15db Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 30 Apr 2026 18:06:05 +0000 Subject: [PATCH] fix: address code review feedback - Fix toggle/load_order handlers to use page-reload (not JSON) responses - Remove dead jsonResponse helper method from WorkshopModController - Fix robocopy exit code detection using ROBOCOPY_EXIT: sentinel (not text parsing) - Fix rsync dry-run change detection using RSYNC_EXIT: sentinel - Remove agentIdFromRemote() stub; pass agentId directly to triggerSteamCmdDownload() logging - Fix 'enabled' checkbox default in profile_form to use ($profile['enabled'] ?? 1) - Add missing error_toggle_failed / error_order_failed lang strings Agent-Logs-Url: https://github.com/GameServerPanel/GSP/sessions/dbeebd0e-e7a5-469d-8a8c-e63193d1ebb0 Co-authored-by: iaretechnician <2749183+iaretechnician@users.noreply.github.com> --- .../controllers/WorkshopModController.php | 33 ++++++---- modules/steam_workshop/lang/en_US.php | 2 + .../steam_workshop/lib/WorkshopInstaller.php | 64 ++++++++++--------- .../views/admin/profile_form.php | 2 +- .../views/user_workshop_mods.php | 4 +- 5 files changed, 59 insertions(+), 46 deletions(-) diff --git a/modules/steam_workshop/controllers/WorkshopModController.php b/modules/steam_workshop/controllers/WorkshopModController.php index 8139b6fc..68f7ae59 100644 --- a/modules/steam_workshop/controllers/WorkshopModController.php +++ b/modules/steam_workshop/controllers/WorkshopModController.php @@ -233,18 +233,25 @@ class WorkshopModController $enabled = !empty($_POST['enabled']); if ($homeId <= 0 || $workshopId === '') { - $this->jsonResponse(['ok' => false, 'error' => 'Missing parameters.']); + print_failure($this->lang['error_missing_params'] ?? 'Missing parameters.'); + $this->handleIndex($userId, $isAdmin); return; } $home = $this->getHome($homeId, $userId, $isAdmin); if ($home === null) { - $this->jsonResponse(['ok' => false, 'error' => 'Access denied.']); + print_failure($this->lang['error_home_not_found'] ?? 'Server not found.'); + $this->handleIndex($userId, $isAdmin); return; } $ok = $this->repo->toggleMod($homeId, $workshopId, $enabled); - $this->jsonResponse(['ok' => $ok]); + if (!$ok) { + print_failure($this->lang['error_toggle_failed'] ?? 'Failed to update mod status.'); + } + + $_GET['home_id'] = $homeId; + $this->handleModsPage($userId, $isAdmin); } private function handleLoadOrder(int $userId, bool $isAdmin): void @@ -254,18 +261,25 @@ class WorkshopModController $order = (int)($_POST['load_order'] ?? 0); if ($homeId <= 0 || $workshopId === '') { - $this->jsonResponse(['ok' => false, 'error' => 'Missing parameters.']); + print_failure($this->lang['error_missing_params'] ?? 'Missing parameters.'); + $this->handleIndex($userId, $isAdmin); return; } $home = $this->getHome($homeId, $userId, $isAdmin); if ($home === null) { - $this->jsonResponse(['ok' => false, 'error' => 'Access denied.']); + print_failure($this->lang['error_home_not_found'] ?? 'Server not found.'); + $this->handleIndex($userId, $isAdmin); return; } $ok = $this->repo->updateLoadOrder($homeId, $workshopId, $order); - $this->jsonResponse(['ok' => $ok]); + if (!$ok) { + print_failure($this->lang['error_order_failed'] ?? 'Failed to update load order.'); + } + + $_GET['home_id'] = $homeId; + $this->handleModsPage($userId, $isAdmin); } private function handleSync(int $userId, bool $isAdmin): void @@ -360,13 +374,6 @@ class WorkshopModController require __DIR__ . '/../views/' . $view . '.php'; } - /** @param array $data */ - private function jsonResponse(array $data): void - { - header('Content-Type: application/json'); - echo json_encode($data); - } - private function loadLang(): array { $file = __DIR__ . '/../lang/en_US.php'; diff --git a/modules/steam_workshop/lang/en_US.php b/modules/steam_workshop/lang/en_US.php index 47ea24b4..db527900 100644 --- a/modules/steam_workshop/lang/en_US.php +++ b/modules/steam_workshop/lang/en_US.php @@ -192,5 +192,7 @@ return [ 'error_missing_params' => 'Missing required parameters.', 'error_no_profile' => 'No Workshop profile configured for this game.', 'error_mod_not_found' => 'Mod or profile not found.', + 'error_toggle_failed' => 'Failed to update mod status.', + 'error_order_failed' => 'Failed to update load order.', ]; diff --git a/modules/steam_workshop/lib/WorkshopInstaller.php b/modules/steam_workshop/lib/WorkshopInstaller.php index c0c5784e..df065446 100644 --- a/modules/steam_workshop/lib/WorkshopInstaller.php +++ b/modules/steam_workshop/lib/WorkshopInstaller.php @@ -94,7 +94,7 @@ class WorkshopInstaller if ($cacheEntry === null || ($cacheEntry['status'] ?? '') !== 'cached') { $log[] = 'Cache MISS – triggering SteamCMD download on agent.'; $downloadResult = $this->triggerSteamCmdDownload( - $remote, $appId, $workshopId, $steamCmdPath, $cachePath, $log + $remote, $agentId, $appId, $workshopId, $steamCmdPath, $cachePath, $log ); if (!$downloadResult) { @@ -273,6 +273,7 @@ class WorkshopInstaller */ private function triggerSteamCmdDownload( object $remote, + int $agentId, string $appId, string $workshopId, string $steamCmdPath, @@ -292,13 +293,13 @@ class WorkshopInstaller ]); $log[] = "SteamCMD start: {$cmd}"; - $this->writeLog("STEAMCMD START agent={$this->agentIdFromRemote($remote)} app={$appId} mod={$workshopId}"); + $this->writeLog("STEAMCMD START agent={$agentId} app={$appId} mod={$workshopId}"); $output = $remote->exec($cmd); if ($output === null) { $log[] = 'SteamCMD: no response from agent (command may still be running).'; - $this->writeLog("STEAMCMD NO_RESPONSE app={$appId} mod={$workshopId}"); + $this->writeLog("STEAMCMD NO_RESPONSE agent={$agentId} app={$appId} mod={$workshopId}"); // Treat as unknown – check file existence } else { $log[] = 'SteamCMD output: ' . substr((string)$output, 0, 500); @@ -307,11 +308,11 @@ class WorkshopInstaller // Verify the download succeeded by checking for the cache path on the agent $exists = $remote->rfile_exists($cachePath); if ($exists === 1) { - $this->writeLog("STEAMCMD SUCCESS app={$appId} mod={$workshopId} path={$cachePath}"); + $this->writeLog("STEAMCMD SUCCESS agent={$agentId} app={$appId} mod={$workshopId} path={$cachePath}"); return true; } - $this->writeLog("STEAMCMD FAILURE app={$appId} mod={$workshopId} path={$cachePath}"); + $this->writeLog("STEAMCMD FAILURE agent={$agentId} app={$appId} mod={$workshopId} path={$cachePath}"); return false; } @@ -332,27 +333,33 @@ class WorkshopInstaller $log[] = "Pre-start compare: cache={$cachePath} dest={$installPath} method={$copyMethod}"; if ($copyMethod === 'rsync') { + // Dry-run: any output lines (beyond the exit sentinel) mean changes exist $cmd = sprintf( - 'rsync -rcn --delete %s %s 2>/dev/null; echo "EXIT:$?"', + 'rsync -rcn --delete %s %s 2>/dev/null; echo "RSYNC_EXIT:$?"', escapeshellarg(rtrim($cachePath, '/') . '/'), escapeshellarg(rtrim($installPath, '/') . '/') ); - $out = (string)$remote->exec($cmd); - // If rsync dry-run produces file list output, changes exist - $hasChanges = preg_match('/\S/', preg_replace('/EXIT:\d+\s*$/', '', $out) ?? '') === 1; - return $hasChanges; + $out = (string)$remote->exec($cmd); + // Strip the exit line, then check for any non-whitespace output + $body = preg_replace('/RSYNC_EXIT:\d+\s*$/', '', $out) ?? ''; + return preg_match('/\S/', $body) === 1; } if ($copyMethod === 'robocopy') { - // Robocopy /L = list only, /MIR = mirror, /NJH /NJS = no headers + // List-only mode: robocopy exit code 0 = no differences, 1+ = changes or errors. + // Embed the exit code in output so we can read it back via exec(). $cmd = sprintf( - 'robocopy /L /MIR /NJH /NJS %s %s', + 'robocopy /L /MIR /NJH /NJS %s %s; echo "ROBOCOPY_EXIT:$LASTEXITCODE"', escapeshellarg($cachePath), escapeshellarg($installPath) ); - $out = (string)$remote->exec($cmd); - // Exit code 0 = no changes, 1+ = changes - return trim($out) !== '' && !preg_match('/\bNo new\b/i', $out); + $out = (string)$remote->exec($cmd); + if (preg_match('/ROBOCOPY_EXIT:(\d+)/', $out, $m)) { + // 0 = no change; 1–7 = informational (changes found); 8+ = error + return (int)$m[1] !== 0; + } + // If we cannot determine, assume sync is needed + return true; } // custom_script: always sync @@ -392,7 +399,7 @@ class WorkshopInstaller ); } elseif ($copyMethod === 'robocopy') { $cmd = sprintf( - 'robocopy /MIR /NJH /NJS %s %s; echo "ROBOCOPY EXIT:$LASTEXITCODE"', + 'robocopy /MIR /NJH /NJS %s %s; echo "ROBOCOPY_EXIT:$LASTEXITCODE"', escapeshellarg($cachePath), escapeshellarg($installPath) ); @@ -419,17 +426,20 @@ class WorkshopInstaller $out = (string)$remote->exec($cmd); $log[] = 'Sync output: ' . substr($out, 0, 500); - // Check exit code hint embedded in output - if (preg_match('/EXIT:(\d+)/', $out, $m)) { - $code = (int)$m[1]; - // robocopy exit codes 0..7 are success/info, 8+ are errors - if ($copyMethod === 'robocopy') { - $ok = $code < 8; + // Determine success from embedded exit code sentinel + if ($copyMethod === 'robocopy') { + if (preg_match('/ROBOCOPY_EXIT:(\d+)/', $out, $m)) { + // 0–7 = success/informational; 8+ = error + $ok = (int)$m[1] < 8; } else { - $ok = $code === 0; + $ok = true; // assume success if no code extracted } } else { - $ok = true; // assume success if no code + if (preg_match('/EXIT:(\d+)/', $out, $m)) { + $ok = (int)$m[1] === 0; + } else { + $ok = true; + } } if ($ok) { @@ -471,12 +481,6 @@ class WorkshopInstaller return 'linux'; } - private function agentIdFromRemote(object $remote): string - { - // OGPRemoteLibrary stores host/port; use reflection-free fallback - return 'unknown'; - } - private function writeLog(string $message): void { $file = $this->logDir . '/workshop_install.log'; diff --git a/modules/steam_workshop/views/admin/profile_form.php b/modules/steam_workshop/views/admin/profile_form.php index 768f0d6a..f05244d0 100644 --- a/modules/steam_workshop/views/admin/profile_form.php +++ b/modules/steam_workshop/views/admin/profile_form.php @@ -135,7 +135,7 @@ $tplVarNote = $lang['profile_template_vars'] ?? 'Available: {home_id} {agent_id diff --git a/modules/steam_workshop/views/user_workshop_mods.php b/modules/steam_workshop/views/user_workshop_mods.php index 31f44976..04afcd3e 100644 --- a/modules/steam_workshop/views/user_workshop_mods.php +++ b/modules/steam_workshop/views/user_workshop_mods.php @@ -203,14 +203,14 @@ $baseAction = '?m=steam_workshop&p=main';