diff --git a/tailscalectl/TailscaleWidget.qml b/tailscalectl/TailscaleWidget.qml index a4fe36f..9b478a4 100644 --- a/tailscalectl/TailscaleWidget.qml +++ b/tailscalectl/TailscaleWidget.qml @@ -14,29 +14,16 @@ PluginComponent { property string tailscaleIP: "" property string currentExitNode: "" property var peers: [] - property string cachedClipboardTool: "" property string _copyText: "" property string _copyCurrentTool: "" - property int _copyAttempted: 0 - property bool _pendingToggle: false // transient one-shot for poll-then-act (#42) — reset immediately after use, no "desired state" cache + property bool _pendingToggle: false // transient one-shot for post-action verification (to be removed in first-principles refactor) layerNamespacePlugin: "tailscalectl" popoutWidth: 360 popoutHeight: 400 - Timer { - id: refreshTimer - interval: 5000 - running: true - repeat: true - onTriggered: { - if (!statusCheck.running) { - statusCheck.running = true - } - } - } - Component.onCompleted: { + // Initial status fetch on load. Subsequent fetches are on-demand (popout open, explicit refresh, or post-action verification). statusCheck.running = true } @@ -48,8 +35,8 @@ PluginComponent { onExited: (code, status) => { if (code !== 0) { var action = root.isConnected ? "disconnect" : "connect" - var detail = toggleProcess.stderr.text.trim().slice(0, 120) - ToastService.showError("tailscalectl", TailscaleLib.errorMessage(action) + (detail ? " — " + detail : "")) + var detail = toggleProcess.stderr.text.trim() + ToastService.showError("tailscalectl", TailscaleLib.formatError(action, detail)) } statusCheck.running = true } @@ -62,17 +49,14 @@ PluginComponent { onExited: (code, status) => { if (code === 0) { - root.cachedClipboardTool = root._copyCurrentTool - ToastService.showInfo("Copied " + root._copyText + " to clipboard") + ToastService.showInfo(TailscaleLib.getStrings().copied(root._copyText)) + } else if (root._copyCurrentTool === "dms") { + // One simple fallback attempt: try wl-copy. + root._copyCurrentTool = "wl-copy" + root._executeCopy() } else { - root._copyAttempted += 1 - if (root._copyAttempted < TailscaleLib.allClipboardTools().length) { - root._copyCurrentTool = TailscaleLib.nextClipboardTool(root._copyCurrentTool) - root._executeCopy() - } else { - var detail = copyProcess.stderr.text.trim().slice(0, 120) - ToastService.showError("tailscalectl", "No clipboard tool found" + (detail ? " — " + detail : "")) - } + var detail = copyProcess.stderr.text.trim() + ToastService.showError("tailscalectl", TailscaleLib.formatError("clipboard", detail)) } } } @@ -84,8 +68,8 @@ PluginComponent { onExited: (code, status) => { if (code !== 0) { - var detail = exitNodeProcess.stderr.text.trim().slice(0, 120) - ToastService.showError("tailscalectl", TailscaleLib.errorMessage("set") + (detail ? " — " + detail : "")) + var detail = exitNodeProcess.stderr.text.trim() + ToastService.showError("tailscalectl", TailscaleLib.formatError("set", detail)) } statusCheck.running = true } @@ -110,12 +94,10 @@ PluginComponent { root.tailscaleIP = "" root.currentExitNode = "" root.peers = [] - ToastService.showError("tailscalectl", TailscaleLib.errorMessage("status")) + ToastService.showError("tailscalectl", TailscaleLib.formatError("status")) } - // #42 poll-then-act: if a toggle was requested, use the *fresh* state we just received - // #47: structured logging evaluated and rejected — toasts + 120-char stderr already give - // actionable feedback; persistent logs would add state/rotation with no net UX benefit. + // Post-action verification: if a toggle was requested, use the fresh state we just received. if (root._pendingToggle) { root._pendingToggle = false toggleProcess.command = TailscaleLib.buildToggleCommand(root.isConnected) @@ -125,8 +107,7 @@ PluginComponent { } function toggleTailscale() { - // #42: poll-then-act — never decide up/down from potentially stale root.isConnected. - // Force a fresh statusCheck; the decision happens in its onExited using the just-received value. + // Post-action verification: force a fresh status check, then act on the real ground truth. root._pendingToggle = true statusCheck.running = true } @@ -138,19 +119,15 @@ PluginComponent { function copyToClipboard(text) { root._copyText = text - root._copyAttempted = 0 - if (root.cachedClipboardTool) { - root._copyCurrentTool = root.cachedClipboardTool - } else { - root._copyCurrentTool = TailscaleLib.allClipboardTools()[0] - } + // Simple two-tool fallback per first-principles plan: dms first, then wl-copy. + root._copyCurrentTool = "dms" root._executeCopy() } function _executeCopy() { var cmd = TailscaleLib.buildCopyCommand(root._copyText, root._copyCurrentTool) if (!cmd) { - ToastService.showError("tailscalectl", "Invalid clipboard command") + ToastService.showError("tailscalectl", TailscaleLib.formatError("clipboard")) return } copyProcess.command = cmd @@ -160,7 +137,7 @@ PluginComponent { popoutContent: Component { PopoutComponent { headerText: "Tailscale" - detailsText: root.isConnected ? "Connected" : "Disconnected" + detailsText: root.isConnected ? TailscaleLib.getStrings().connected : TailscaleLib.getStrings().disconnected showCloseButton: true Item { @@ -207,7 +184,7 @@ PluginComponent { Item { width: 1; height: 1; Layout.fillWidth: true } StyledText { - text: "Exit node: " + (root.currentExitNode || "None") + text: TailscaleLib.getStrings().exitNodePrefix + (root.currentExitNode || TailscaleLib.getStrings().none) font.pixelSize: Theme.fontSizeSmall color: Theme.surfaceVariantText anchors.verticalCenter: parent.verticalCenter diff --git a/tailscalectl/lib.js b/tailscalectl/lib.js index e367975..86efe02 100644 --- a/tailscalectl/lib.js +++ b/tailscalectl/lib.js @@ -33,9 +33,9 @@ function findActiveExitNode(peerMap) { var safeClipboardTools = ["dms", "wl-copy"] -// Direct argv form — no sh -c, no escaping, no future injection surface (#49) -// clipmanctl intentionally dropped (niche optional history manager, not needed on DMS + Wayland) -var clipboardTools = { + // Direct argv form — no sh -c, no escaping, no future injection surface. + // clipmanctl intentionally dropped (niche optional history manager, not needed on DMS + Wayland). + var clipboardTools = { "dms": { argv: ["dms", "cl", "copy"] }, "wl-copy": { argv: ["wl-copy"] } } @@ -76,8 +76,8 @@ function getStrings() { } } -// Light UI predicates extracted from QML (advances #43 — keeps view thin) -function shouldShowClearExitNode(currentExitNode) { + // Light UI predicates — keep the view thin. + function shouldShowClearExitNode(currentExitNode) { return currentExitNode !== "" } @@ -103,6 +103,11 @@ function buildToggleCommand(isConnected) { return isConnected ? ["tailscale", "down"] : ["tailscale", "up"] } +// Single source of truth for the status command used for on-demand and post-action verification. +function getStatusCommand() { + return ["tailscale", "status", "--json"] +} + function errorMessage(cmd) { var messages = { "up": "Failed to connect to Tailscale", @@ -110,12 +115,24 @@ function errorMessage(cmd) { "down": "Failed to disconnect from Tailscale", "disconnect": "Failed to disconnect from Tailscale", "set": "Failed to set exit node", - "status": "Failed to read Tailscale status" + "status": "Failed to read Tailscale status", + "clipboard": "Error copying to clipboard" + }; + return messages[cmd] || "Tailscale command failed"; +} + +// Central error formatting for the widget. Used by both success and error paths. +// detail is optional truncated stderr or extra context. +function formatError(action, detail) { + var base = errorMessage(action); + if (detail && detail.length > 0) { + var truncated = detail.length > 120 ? detail.slice(0, 120) : detail; + return base + " — " + truncated; } - return messages[cmd] || "Tailscale command failed" + return base; } // CommonJS export for Node.js tests (ignored by QML) if (typeof module !== "undefined" && module.exports) { - module.exports = { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, validateClipboardTool, buildCopyCommand, nextClipboardTool, allClipboardTools, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } + module.exports = { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, validateClipboardTool, buildCopyCommand, nextClipboardTool, allClipboardTools, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } } diff --git a/test/lib.test.js b/test/lib.test.js index cb64f07..22ad0e6 100644 --- a/test/lib.test.js +++ b/test/lib.test.js @@ -1,7 +1,7 @@ import { test } from "node:test" import assert from "node:assert" import lib from "../tailscalectl/lib.js" -const { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, validateClipboardTool, buildCopyCommand, nextClipboardTool, allClipboardTools, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } = lib +const { parsePeers, makeExitNodeCommand, findActiveExitNode, errorMessage, formatError, getStatusCommand, validateClipboardTool, buildCopyCommand, nextClipboardTool, allClipboardTools, buildToggleCommand, parseStatusResult, getStrings, shouldShowClearExitNode, isActiveExitNode } = lib /* * Test coverage note for #48: @@ -240,3 +240,30 @@ test("parseStatusResult sets isConnected false for non-Running BackendState", () const state = parseStatusResult(json) assert.strictEqual(state.isConnected, false) }) + +// --- formatError (central error + detail formatting per first-principles plan) --- + +test("formatError returns base message without detail", () => { + assert.strictEqual(formatError("status"), "Failed to read Tailscale status") + assert.strictEqual(formatError("set"), "Failed to set exit node") +}) + +test("formatError appends and truncates detail", () => { + const longDetail = "x".repeat(200) + const msg = formatError("up", longDetail) + assert.ok(msg.includes("Failed to connect to Tailscale")) + assert.ok(msg.endsWith("x".repeat(120))) + assert.ok(msg.length < 200) +}) + +test("formatError handles empty or falsy detail gracefully", () => { + assert.strictEqual(formatError("down", ""), "Failed to disconnect from Tailscale") + assert.strictEqual(formatError("connect", null), "Failed to connect to Tailscale") +}) + +// --- getStatusCommand (new pure helper for low-duty-cycle architecture) --- + +test("getStatusCommand returns the canonical tailscale status --json argv", () => { + const cmd = getStatusCommand() + assert.deepStrictEqual(cmd, ["tailscale", "status", "--json"]) +})