Fix SonarCloud HIGH issues: S1192 duplicated literals, S7688 shell tests

.gitignore

@@ -50,3 +50,6 @@ tmp/
 
 # Transient pre-modification backups
 backups/*.bak
+
+# Local SonarCloud API token (never commit)
+.sonar_cloud_token

CHANGELOG.md

@@ -25,6 +25,7 @@ This project follows the structure from Keep a Changelog and intends to use Sema
 
 ### Changed
 
+- **Refactored repeated string literals** (GUI selectors, HTML fragments, the `reports.` TOML prefix, the Git LFS label, and the workflows pathspec) into module-level constants to clear SonarCloud high-severity S1192/S7688 issues.
 - **GitHub Actions workflow now defaults to `--email-format html`:** The committed `email-report.yml` and its template were updated from `--email-format text` to `--email-format html`. Existing users who re-run setup (option 5 / wizard) will have their workflow re-rendered with this default. To keep plain-text output, set `email_format = "text"` in `.github-usage/config.toml` before re-running setup, or select **Text** in the wizard.
 - **Terminology:** the interactive CLI/TUI usage report is now called the **local full report** in user-facing copy (README, `start.sh`, CLI help, TUI). Internal `legacy_*` module names are unchanged for now (tracked in `TO_DO.md`).
 - **GitHub Actions `setup-python` v7:** Bump `actions/setup-python` from v6 to v7 in CI, security, email-report, and the email-report template (folds in Dependabot #7; no workflow input changes — this repo does not use the removed `pip-install` input).

src/github_usage/cli_runs_diff.py

@@ -22,6 +22,9 @@
 # stable sort order in porcelain output regardless of the user's locale.
 GIT_ENV: dict[str, str] = {**os.environ, "LC_ALL": "C"}
 
+# Directory holding the email-report workflow files; used as a git pathspec.
+_WORKFLOWS_DIR = ".github/workflows/"
+
 # Valid drift categories. Asserted by tests.
 DRIFT_CATEGORIES: frozenset[str] = frozenset(
     {
@@ -191,7 +194,7 @@ def _normalize_path(p: str | Path, repo_root: Path) -> str:
     return pp.as_posix()
 
 
-def _git_status_porcelain(repo_root: Path, pathspec: str = ".github/workflows/") -> dict[str, str]:
+def _git_status_porcelain(repo_root: Path, pathspec: str = _WORKFLOWS_DIR) -> dict[str, str]:
     """Run ``git status --porcelain=v1 -- <pathspec>`` and return ``{path: line}``.
 
     Skipped (returns ``{}``) if the subprocess fails for any reason;
@@ -302,7 +305,7 @@ def _list_local_paths(repo_root: Path) -> list[str]:
     paths: set[str] = set()
     try:
         proc = _run_git(
-            ["ls-tree", "-r", "HEAD", "--", ".github/workflows/"],
+            ["ls-tree", "-r", "HEAD", "--", _WORKFLOWS_DIR],
             cwd=repo_root,
         )
         if proc.returncode == 0:
@@ -351,7 +354,7 @@ def _list_remote_paths(repo_root: Path, remote: str, default_branch: str | None)
                 "-r",
                 f"{remote}/{default_branch}",
                 "--",
-                ".github/workflows/",
+                _WORKFLOWS_DIR,
             ],
             cwd=repo_root,
         )

src/github_usage/email_report_html.py

@@ -22,6 +22,11 @@
     visibility_group_header,
 )
 
+# Repeated HTML fragments, hoisted to satisfy S1192 and keep tag spelling in one place.
+_UL_CLOSE = "</ul>"
+_TABLE_OPEN = "<table>"
+_TABLE_CLOSE = "</table>"
+
 
 def _html_cost_row(label: str, cost: dict[str, float]) -> str:
     return (
@@ -42,7 +47,7 @@ def _format_html_billing_context_section(data: dict) -> list[str]:
         "<li>Actions minutes and storage are free for public repositories.</li>",
         "<li>Private and internal repositories consume your plan's monthly quota.</li>",
         f'<li>See <a href="{_BILLING_CONTEXT_URL}">GitHub Actions billing</a> for details.</li>',
-        "</ul>",
+        _UL_CLOSE,
     ]
 
 
@@ -53,7 +58,7 @@ def _format_html_actions_section(data: dict) -> list[str]:
     net = (data.get("monthly_costs") or {}).get("actions", {}).get("net", 0.0)
     return [
         "<h2>Actions</h2>",
-        "<table>",
+        _TABLE_OPEN,
         "<tr><th>Metric</th><th>Value</th></tr>",
         (
             f"<tr><td>Minutes</td><td>{actions.get('minutes', 0.0):,.1f} / "
@@ -66,7 +71,7 @@ def _format_html_actions_section(data: dict) -> list[str]:
             f"({actions.get('storage_percent', 0.0):.1f}%)</td></tr>"
         ),
         f"<tr><td>Net cost</td><td>{fmt_price(net)}</td></tr>",
-        "</table>",
+        _TABLE_CLOSE,
     ]
 
 
@@ -82,13 +87,13 @@ def _format_html_copilot_section(data: dict) -> list[str]:
     ]
     by_model = copilot.get("by_model") or {}
     if by_model:
-        parts.append("</ul>")
+        parts.append(_UL_CLOSE)
         parts.append("<h3>By model</h3>")
         parts.append("<ul>")
         for model, values in sorted(by_model.items()):
             requests = values.get("requests", values.get("total_requests", 0.0))
             parts.append(f"<li>{html.escape(model)}: {requests:,.1f} requests</li>")
-    parts.append("</ul>")
+    parts.append(_UL_CLOSE)
     return parts
 
 
@@ -116,10 +121,10 @@ def _format_html_monthly_costs_section(data: dict) -> list[str]:
         rows.append(_html_cost_row(label, monthly.get(key, {})))
     return [
         "<h2>Monthly Cost Estimate</h2>",
-        "<table>",
+        _TABLE_OPEN,
         "<tr><th>Category</th><th>Gross</th><th>Discount</th><th>Net</th></tr>",
         *rows,
-        "</table>",
+        _TABLE_CLOSE,
     ]
 
 
@@ -225,7 +230,7 @@ def _format_html_artifact_storage_section(data: dict) -> list[str]:
                 f"<li>{html_repo_cell(row)}: "
                 f"{_bytes_to_mb(row['artifact_bytes']):,.1f} MB artifacts</li>"
             )
-        parts.append("</ul>")
+        parts.append(_UL_CLOSE)
     else:
         for vis, group_rows in groups.items():
             parts.append(f"<h3>{html.escape(visibility_group_header(vis))}</h3>")
@@ -235,7 +240,7 @@ def _format_html_artifact_storage_section(data: dict) -> list[str]:
                     f"<li>{html_repo_cell(row)}: "
                     f"{_bytes_to_mb(row['artifact_bytes']):,.1f} MB artifacts</li>"
                 )
-            parts.append("</ul>")
+            parts.append(_UL_CLOSE)
     if artifact_storage.get("truncated"):
         parts.append(
             f"<p><em>Artifact scan truncated at "
@@ -258,7 +263,7 @@ def _format_html_release_assets_section(data: dict) -> list[str]:
                 f"<li>{html_repo_cell(row)}: "
                 f"{_bytes_to_mb(row['release_asset_bytes']):,.1f} MB release assets</li>"
             )
-        parts.append("</ul>")
+        parts.append(_UL_CLOSE)
     else:
         for vis, group_rows in groups.items():
             parts.append(f"<h3>{html.escape(visibility_group_header(vis))}</h3>")
@@ -268,7 +273,7 @@ def _format_html_release_assets_section(data: dict) -> list[str]:
                     f"<li>{html_repo_cell(row)}: "
                     f"{_bytes_to_mb(row['release_asset_bytes']):,.1f} MB release assets</li>"
                 )
-            parts.append("</ul>")
+            parts.append(_UL_CLOSE)
     if release_assets.get("truncated"):
         parts.append(
             f"<p><em>Release asset scan truncated at "
@@ -285,7 +290,7 @@ def _format_html_insights_section(data: dict) -> list[str]:
         "<h2>Key Insights</h2>",
         "<ul>",
         *[f"<li>{html.escape(insight)}</li>" for insight in insights],
-        "</ul>",
+        _UL_CLOSE,
     ]
 
 
@@ -299,7 +304,7 @@ def _format_html_errors_section(data: dict) -> list[str]:
             f"<li>{html.escape(section.replace('_', ' ').title())} "
             f"data unavailable - {html.escape(message)}</li>"
         )
-    parts.append("</ul>")
+    parts.append(_UL_CLOSE)
     return parts
 
 
@@ -351,7 +356,7 @@ def _run_out(value: int | None) -> str:
     parts = [
         f"<h2>Monthly Forecast{scope_note}</h2>",
         f"<p>Day {forecast['day_of_month']} of {forecast['days_in_month']}</p>",
-        "<table>",
+        _TABLE_OPEN,
         "<tr><th>Metric</th><th>Current</th><th>Projected</th><th>Limit</th><th>Run-out</th></tr>",
     ]
     for label, metric in rows:
@@ -364,7 +369,7 @@ def _run_out(value: int | None) -> str:
             f"<td>{_run_out(metric['run_out_day'])}</td>"
             "</tr>"
         )
-    parts.append("</table>")
+    parts.append(_TABLE_CLOSE)
     if has_split:
         note = _public_repos_html_note(forecast)
         if note:
@@ -431,7 +436,7 @@ def _html_api_notes_block(estimate: dict) -> list[str]:
     parts = ["<h2>REST API Quota Notes</h2>", "<ul>"]
     for note in notes:
         parts.append(f"<li>{html.escape(note)}</li>")
-    parts.append("</ul>")
+    parts.append(_UL_CLOSE)
     return parts
 
 
@@ -451,7 +456,7 @@ def _html_sources_block(sources: dict) -> list[str]:
             f"<li>{html.escape(label)}: "
             f'<a href="{html.escape(str(url))}">{html.escape(str(url))}</a></li>'
         )
-    parts.append("</ul>")
+    parts.append(_UL_CLOSE)
     return parts
 
 

src/github_usage/gui/views/email_report_view.py

@@ -13,6 +13,9 @@
 from ..layout import FormGrid, ViewActions, ViewOutput, ViewSection
 from ..log_utils import write_log
 
+# Repeated selector/label literals, hoisted to satisfy S1192.
+_EMAIL_PREVIEW = "#email-preview"
+
 
 class EmailReportView(VerticalScroll, AsyncViewMixin):
     """Dry-run preview and send for email reports."""
@@ -55,7 +58,7 @@ def _on_state_changed(self) -> None:
         self._reload_profiles()
 
     def _show_error(self, message: str) -> None:
-        preview = self.query_one("#email-preview", RichLog)
+        preview = self.query_one(_EMAIL_PREVIEW, RichLog)
         write_log(preview, format_simple(message), level="error")
 
     def _finish_reload(self) -> None:
@@ -102,7 +105,7 @@ def action_preview(self) -> None:
     @work(thread=True)
     def _run_preview(self) -> None:
         button = self.query_one("#preview-btn", Button)
-        preview = self.query_one("#email-preview", RichLog)
+        preview = self.query_one(_EMAIL_PREVIEW, RichLog)
         self._call_ui(
             self._begin_async,
             button,
@@ -133,7 +136,7 @@ def _run_preview(self) -> None:
             self._call_ui(self._end_async, button, "Preview (dry-run)")
 
     def _show_preview(self, code: int, body: str) -> None:
-        preview = self.query_one("#email-preview", RichLog)
+        preview = self.query_one(_EMAIL_PREVIEW, RichLog)
         preview.clear()
         if body.strip():
             preview.write(body.rstrip())
@@ -156,7 +159,7 @@ def _send(self) -> None:
     @work(thread=True)
     def _run_send(self) -> None:
         button = self.query_one("#send-btn", Button)
-        preview = self.query_one("#email-preview", RichLog)
+        preview = self.query_one(_EMAIL_PREVIEW, RichLog)
         self._call_ui(
             self._begin_async,
             button,
@@ -186,7 +189,7 @@ def _run_send(self) -> None:
             self._call_ui(self._end_async, button, "Send Email")
 
     def _show_send_result(self, code: int, message: str) -> None:
-        preview = self.query_one("#email-preview", RichLog)
+        preview = self.query_one(_EMAIL_PREVIEW, RichLog)
         if code == 0:
             write_log(preview, message, level="success")
         else:

src/github_usage/gui/views/report_view.py

@@ -28,6 +28,9 @@
 from ..layout import FormGrid, ViewActions, ViewOutput, ViewSection
 from ..log_utils import write_log
 
+# Repeated selector/label literals, hoisted to satisfy S1192.
+_REPORT_LOG = "#report-log"
+
 
 @dataclass(frozen=True)
 class _ReportRunParams:
@@ -105,20 +108,20 @@ def _run_report(self) -> None:
         if params is None:
             return
         button = self.query_one("#run-report", Button)
-        log = self.query_one("#report-log", RichLog)
+        log = self.query_one(_REPORT_LOG, RichLog)
         self._fetch_report(params, button, log)
 
     def action_run_report(self) -> None:
         if not self._is_running:
             params = self._read_run_params()
             if params is not None:
                 button = self.query_one("#run-report", Button)
-                log = self.query_one("#report-log", RichLog)
+                log = self.query_one(_REPORT_LOG, RichLog)
                 self._fetch_report(params, button, log)
 
     def _read_run_params(self) -> _ReportRunParams | None:
         """Validate form fields on the UI thread."""
-        log = self.query_one("#report-log", RichLog)
+        log = self.query_one(_REPORT_LOG, RichLog)
         timeout_str = self.query_one("#timeout", Input).value.strip() or "30"
         max_retries_str = self.query_one("#max-retries", Input).value.strip() or "3"
 
@@ -245,7 +248,7 @@ def _show_report_result(
         username_or_err: str | None,
         cache_hit,
     ) -> None:
-        log = self.query_one("#report-log", RichLog)
+        log = self.query_one(_REPORT_LOG, RichLog)
         table = self.query_one("#summary-table", DataTable)
         table.clear()
 

src/github_usage/gui/views/runs_view.py

@@ -13,6 +13,9 @@
 from ..layout import ViewActions, ViewOutput, ViewSection
 from ..log_utils import write_log
 
+# Repeated selector/label literals, hoisted to satisfy S1192.
+_RUNS_LOG = "#runs-log"
+
 
 class RunsView(VerticalScroll, AsyncViewMixin):
     """Read-only runs table and drift checker."""
@@ -59,17 +62,17 @@ def _load_runs(self) -> None:
                     row["schedule"],
                 )
         except Exception as exc:
-            log = self.query_one("#runs-log", RichLog)
+            log = self.query_one(_RUNS_LOG, RichLog)
             write_log(log, format_error(exc, context="Failed to load runs"), level="error")
 
     @on(Button.Pressed, "#refresh-runs")
     def _refresh(self) -> None:
         self._load_runs()
-        write_log(self.query_one("#runs-log", RichLog), "Runs refreshed", level="success")
+        write_log(self.query_one(_RUNS_LOG, RichLog), "Runs refreshed", level="success")
 
     def action_refresh_runs(self) -> None:
         self._load_runs()
-        write_log(self.query_one("#runs-log", RichLog), "Runs refreshed", level="success")
+        write_log(self.query_one(_RUNS_LOG, RichLog), "Runs refreshed", level="success")
 
     @on(Button.Pressed, "#check-drift")
     def _check_drift(self) -> None:
@@ -80,7 +83,7 @@ def _check_drift(self) -> None:
     @work(thread=True)
     def _run_drift(self) -> None:
         button = self.query_one("#check-drift", Button)
-        log = self.query_one("#runs-log", RichLog)
+        log = self.query_one(_RUNS_LOG, RichLog)
         self._call_ui(
             self._begin_async,
             button,
@@ -104,7 +107,7 @@ def _run_drift(self) -> None:
             self._call_ui(self._end_async, button, "Check drift")
 
     def _show_drift(self, result) -> None:
-        log = self.query_one("#runs-log", RichLog)
+        log = self.query_one(_RUNS_LOG, RichLog)
         for message in result.messages:
             log.write(message)
         table = self.query_one("#drift-table", DataTable)