src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/System.php (1)

163-168: new Info(null) makes three API calls just to get the DNS name.

Info::__construct calls getStatus(), getPrefs(), and getTkaStatus() — only getDNSName() (which reads from $this->status) is actually needed here. The same pattern appears on line 434. Consider extracting hostname resolution from Info to avoid the overhead, or adding a lightweight static/factory method.

src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/ServeConfig.php (5)

87-107: Static analysis: $webConfig is unused in the foreach loop.

Only the key ($fqdn) is needed. Suppress with $_ or use the key-only pattern.

♻️ Minimal fix

-            foreach ($this->config->Web as $fqdn => $webConfig) {
+            foreach ($this->config->Web as $fqdn => $_webConfig) {

---

191-218: Static analysis: $enabled is unused in the foreach loop on line 200.

Same pattern as removeServeByPort — only the key is needed.

♻️ Minimal fix

-        foreach ($serveConfig->AllowFunnel as $hostAndPort => $enabled) {
+        foreach ($serveConfig->AllowFunnel as $hostAndPort => $_enabled) {

---

109-127: removeFunnel removes the TCP entry for the port, which is shared across all hostnames.

If multiple Web entries (e.g., host1:443 and host2:443) share the same TCP port, removing the funnel for one host will break TCP termination for the other. In practice, a single Tailscale node has one DNS name so this likely won't happen — but worth a note if multi-host scenarios are ever considered.

---

141-170: Port persistence via saveWebguiPort / getWebguiPort works correctly.

saveWebguiPort("") deletes the file, and getWebguiPort() defensively handles missing file, decode failures, and type mismatches. One minor note: file_put_contents can return false on failure — consider logging if the write fails, since a silent failure here would cause checkFunnelPort to skip future reconciliation.

---

172-189: getFunnelPort reads ident.cfg directly — this couples a model class to the filesystem.

This duplicates the ident.cfg parsing already done by callers (e.g., System::checkFunnelPort, Config.php funnel-port case). Consider accepting the port as a required parameter and dropping the fallback ident.cfg read, which would simplify testing and keep I/O in the caller.

src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (1)

28-34: Consider a Null Object pattern instead of nullable translator.

With ?Translator, roughly half the public methods (getStatusInfo, getConnectionInfo, getDashboardInfo, getKeyExpirationWarning, getTailscaleLockWarning, getTailscaleNetbiosWarning) become runtime bombs when $tr is null. Callers have no compile-time signal about which methods are safe.

Two lighter alternatives:

1. Null Object: Create a NullTranslator that returns the key as-is, so tr() never throws.
2. Split the class: Extract a TailscaleData (no translator needed) and keep Info for display-oriented methods that require a translator.

Either approach removes the hidden runtime exception.

src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/ServeConfig.php (5)

87-107: removeServeByPort accepts int while the rest of the API uses string for ports.

configureFunnel, removeFunnel, and getFunnelPort all declare $port as string. This method declares it as int. Since stdClass property names are always strings and the config is built with string keys (from JSON or configureFunnel), passing an int here works by implicit cast but is inconsistent and could confuse callers.

Suggested fix

-    public function removeServeByPort(int $port): void
+    public function removeServeByPort(string $port): void
     {

---

141-151: saveWebguiPort and getWebguiPort are filesystem I/O on a class that otherwise wraps an in-memory config.

This is a cohesion concern — these two methods read/write /boot/config/plugins/tailscale/funnel.json but have no relationship to the $this->config state. They'd be more at home in a dedicated persistence helper or in Config/Utils. Not blocking, but worth considering if this class grows further.

Also, file_put_contents can fail (permissions, disk full). Currently failures are silently ignored.

---

172-189: getFunnelPort reads the filesystem (ident.cfg) as a side-effect.

When $port is null, this method parses /boot/config/ident.cfg to determine the expected target port. This couples the class to a system-specific file path and makes unit testing harder. Consider always requiring the caller to supply the port (the caller in Config.php already has $identCfg['PORT'] available).

---

191-218: Port extraction via explode(":", ...) breaks for IPv6 literal hosts.

Line 208 splits on : and expects exactly 2 parts. While Tailscale DNS names are FQDNs (not IPv6 literals), a defensive approach would be to use strrpos / substr to extract the port after the last colon, which also handles edge cases.

Suggested fix

-                        $parts = explode(":", strval($hostAndPort));
-                        if (count($parts) == 2 && is_numeric($parts[1])) {
-                            return strval($parts[1]);
-                        }
+                        $lastColon = strrpos($hostAndPort, ":");
+                        if ($lastColon !== false) {
+                            $portPart = substr($hostAndPort, $lastColon + 1);
+                            if (is_numeric($portPart)) {
+                                return $portPart;
+                            }
+                        }

---

220-225: updateWebProxy silently no-ops if the path doesn't exist.

This is reasonable for defensive coding, but callers have no way to know whether the update actually took effect. Consider returning a bool or logging a warning when the path isn't found, so misuse doesn't go unnoticed.

src/usr/local/emhttp/plugins/tailscale/include/data/Config.php (1)

351-354: ident.cfg is parsed twice — once here (Line 346) and again inside getFunnelPort.

getFunnelPort($hostname) without a second argument falls back to parsing /boot/config/ident.cfg internally. Since you already have $identCfg['PORT'] from Line 346, pass it explicitly to avoid the redundant filesystem read.

Suggested fix

-            $currentFunnelPort = $serveConfig->getFunnelPort($hostname);
+            $currentFunnelPort = $serveConfig->getFunnelPort($hostname, $identCfg['PORT']);