Web: fix copilot warnings, remove deprecated files. No functional changes#6948
Web: fix copilot warnings, remove deprecated files. No functional changes#6948
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 18 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="html/ops/nvidia.php">
<violation number="1" location="html/ops/nvidia.php:21">
P1: `TODO:` text was added as executable PHP instead of a comment, which causes a parse error. Prefix it with comment syntax.</violation>
</file>
<file name="html/user/pm.php">
<violation number="1" location="html/user/pm.php:202">
P2: Using falsey checks here rejects valid input values like `'0'`; keep the null check to preserve prior behavior.</violation>
<violation number="2" location="html/user/pm.php:238">
P2: Falsey validation changes behavior by treating `'0'` as missing input; use the original null checks to avoid rejecting valid values.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // script for studying NVIDIA GPUs on hosts | ||
|
|
||
| DEPRECATED: we don't use host.serialnum anymore | ||
| TODO: use host.misc instead of host.serialnum |
There was a problem hiding this comment.
P1: TODO: text was added as executable PHP instead of a comment, which causes a parse error. Prefix it with comment syntax.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At html/ops/nvidia.php, line 21:
<comment>`TODO:` text was added as executable PHP instead of a comment, which causes a parse error. Prefix it with comment syntax.</comment>
<file context>
@@ -18,7 +18,7 @@
// script for studying NVIDIA GPUs on hosts
-DEPRECATED: we don't use host.serialnum anymore
+TODO: use host.misc instead of host.serialnum
$cli_only = true;
</file context>
| TODO: use host.misc instead of host.serialnum | |
| // TODO: use host.misc instead of host.serialnum |
There was a problem hiding this comment.
Pull request overview
This PR aims to clean up the BOINC web code by removing deprecated/unused PHP endpoints and addressing static-analysis (Copilot) warnings, with the stated intent of having no functional behavior changes.
Changes:
- Removed multiple deprecated
html/user/*andhtml/ops/*PHP scripts. - Tightened some PHP comparisons / null checks (e.g.,
=== false,!== null) and added a null-guard in XML prefs repair. - Added/updated a few inline comments and simplified some conditionals.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| html/user/submit_status.php | Removed deprecated batch status UI endpoint. |
| html/user/show_coproc.php | Removed deprecated CUDA leaderboard viewer. |
| html/user/register.php | Removed deprecated registration page in favor of signup.php. |
| html/user/pending.php | Removed deprecated “pending credit” endpoint. |
| html/user/join.php | Removed deprecated join flow page (replaced by signup.php). |
| html/user/pm.php | Changed field validation checks for sending private messages. |
| html/user/edit_forum_preferences_action.php | Switched null checks to strict !== null in RPC-related branches. |
| html/user/am_set_info.php | Added unconditional “not supported” error return. |
| html/ops/watchdogs.php | Switched filemtime() failure check to strict === false. |
| html/ops/user_graph.php | Added operator guidance comments about jpgraph dependency. |
| html/ops/problem_host.php | Removed deprecated ops script. |
| html/ops/nvidia.php | Updated a deprecation note (but currently introduces invalid PHP syntax). |
| html/ops/grant_credit.php | Removed deprecated/non-working ops script. |
| html/ops/fix_project_prefs.php | Added null-guard before accessing DOM root children. |
| html/ops/cancel_workunits.php | Simplified defaulting logic for $limit. |
| html/ops/analyze_coproc_log.php | Removed deprecated CUDA log analyzer generator. |
| html/inc/util_ops.inc | Adjusted DB host fallback check. |
| html/inc/db.inc | Adjusted DB host fallback check in db_init_aux(). |
Comments suppressed due to low confidence (4)
html/user/pm.php:206
- Using
if (!$subject || !$content)treats the string "0" as empty/invalid (PHP falsey), which is a behavior change from the previous null-only check. If the goal is to reject only missing/empty fields, use explicit checks like=== nulland/ortrim($subject) === ''/trim($content) === ''so valid content isn't rejected accidentally.
if (!$subject || !$content) {
pm_team_form(
$logged_in_user, $teamid,
tra("You need to fill all fields to send a private message")
);
html/user/pm.php:242
if (!$to || !$subject || !$content)will also reject values like the string "0" due to PHP truthiness, which may be unintended and is a behavior change from the prior null-only check. Prefer explicit emptiness checks (e.g.,=== null/trim(...) === '') so only genuinely empty fields are blocked.
if (!$to || !$subject || !$content ) {
pm_form_page(
$replyto, $userid,
tra("You need to fill all fields to send a private message")
);
html/ops/user_graph.php:26
- The added note says to install jpgraph in
html/user/, but this file currently loads it viarequire_once("jpgraph/...")(relative tohtml/ops// include_path). Either update the comment to match the actual expected install location, or update the require paths to the documented location so operators don't install it in the wrong place.
// show credit graphs.
// you need to install jpgraph in html/user/
require_once("../inc/util_ops.inc");
function draw_graph($xarr, $arr) {
require_once ("jpgraph/jpgraph.php");
require_once ("jpgraph/jpgraph_line.php");
html/user/join.php:1
join.phpis being removed, but there are still in-repo links pointing to it (e.g.html/inc/bootstrap.incnavigation andhtml/user/create_account_form.php). Removing it will cause 404s and is a functional change. Either keepjoin.phpas a stub that redirects tosignup.php, or update the remaining links/routes to the replacement page in the same PR.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // script for studying NVIDIA GPUs on hosts | ||
|
|
||
| DEPRECATED: we don't use host.serialnum anymore | ||
| TODO: use host.misc instead of host.serialnum |
There was a problem hiding this comment.
Line TODO: use host.misc instead of host.serialnum is not a PHP comment and will cause a parse error if this script is executed. Prefix this with // (or convert it to a block comment) so the file remains syntactically valid.
| TODO: use host.misc instead of host.serialnum | |
| // TODO: use host.misc instead of host.serialnum |
| // now it doesn't exist | ||
| xml_error(-1, "not supported"); | ||
|
|
There was a problem hiding this comment.
xml_error(-1, "not supported"); is executed unconditionally, so this RPC handler will always exit with an error before emitting the normal XML header and before any prior behavior can run. If the intent is to deprecate/disable this endpoint, consider removing the file entirely (and updating any callers) or returning a properly-formed error response after xml_header() so clients can parse it; otherwise this should be guarded/removed.
| // now it doesn't exist | ||
| xml_error(-1, "not supported"); | ||
|
|
There was a problem hiding this comment.
PR title says "No functional changes", but this change makes am_set_info.php immediately return an error for every request (and exits), which is a functional/API behavior change for account manager RPC clients. Either adjust the PR description/title or avoid changing runtime behavior here.
Summary by cubic
Removed deprecated web pages and added stricter input checks (including a new blank-check helper) to resolve Copilot warnings. No user-visible changes.
html/ops/{grant_credit.php,problem_host.php,analyze_coproc_log.php,submit_status.php}andhtml/user/{register.php,join.php,pending.php,show_coproc.php}.is_blank()and tightened comparisons (!== null,=== false); applied to DB host resolution, PM fields, and request handling; simplified defaultlimit; added$rootguard infix_project_prefs.php.ops/nvidia.phpandops/user_graph.php;am_set_info.phpnow returns "not supported" viaxml_error(-1, ...).Written for commit 1fa76e8. Summary will update on new commits.