From 1f42467ab02a52454bd471e9a4f14129e6975b9e Mon Sep 17 00:00:00 2001 From: soojy <70171585+soojy@users.noreply.github.com> Date: Sat, 12 Sep 2026 15:34:21 +0300 Subject: [PATCH 1/2] Fix systemd working directory and migrate affected service units --- CHANGELOG.md | 6 +++ README.md | 12 +++++ manifest.json | 2 +- scripts/omaproxy.py | 56 ++++++++++++++++++++++- tests/test_service_unit.py | 93 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 166 insertions(+), 3 deletions(-) create mode 100644 tests/test_service_unit.py diff --git a/CHANGELOG.md b/CHANGELOG.md index de3ca01..723d32c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## 0.1.4 + +- Fix generated systemd WorkingDirectory syntax so fresh installations can start. +- Repair the known quoted directory from older service files before Start, Restart, or enabling launch at login. Preserve custom service settings, credentials, and the backend; back up the original unit and retry safely if daemon-reload fails. +- Add a standalone `repair` command and seven regression tests, including validation with the real systemd unit parser. + ## 0.1.3 - Anchor both supported backend archive digests in the plugin snapshot. diff --git a/README.md b/README.md index 4e5c1b2..902dd1a 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,18 @@ systemctl --user daemon-reload The backend version and archive digests are pinned in the plugin and are not silently updated by plugin updates. See the [installer trust policy](docs/installer-security.md) for the reviewed digests and download/extraction limits. Stored credentials remain in `~/.config/omaproxy/` after removal. XDG overrides are supported; adjust the paths if you use them. +### Upgrading from 0.1.3 or earlier + +If the proxy could not start because systemd rejected its working directory, update the plugin and click its power switch to start it. Version 0.1.4 repairs the old generated line and reloads systemd automatically before Start, Restart, or enabling launch at login. Accounts, API keys, and the installed backend are preserved; no download or new sign-in is required. + +To repair the service without starting it: + +```bash +python3 ~/.config/omarchy/plugins/soojy.omaproxy/scripts/omaproxy.py repair +``` + +Only the exact quoted `WorkingDirectory` line generated by older OmaProxy versions is migrated. Custom directory overrides remain untouched. A backup is stored beside the unit as `omaproxy.service.before-working-directory-fix`. + ## Development ```bash diff --git a/manifest.json b/manifest.json index 7b04167..af5b54b 100644 --- a/manifest.json +++ b/manifest.json @@ -2,7 +2,7 @@ "schemaVersion": 1, "id": "soojy.omaproxy", "name": "OmaProxy", - "version": "0.1.3", + "version": "0.1.4", "author": "soojy", "license": "MIT", "description": "AI account quotas, remaining allowances, and reset times in Omarchy, with native account and proxy management.", diff --git a/scripts/omaproxy.py b/scripts/omaproxy.py index acf9ee5..8d80d1b 100644 --- a/scripts/omaproxy.py +++ b/scripts/omaproxy.py @@ -105,6 +105,8 @@ def api(route, method="GET", body=None, timeout=4): def systemctl(*args, check=True): + if args and args[0] in ("start", "restart", "enable"): + repair_service() return run(["systemctl", "--user", *args, UNIT], check=check) @@ -274,9 +276,56 @@ def unit_quote(value): return '"' + str(value).replace("\\", "\\\\").replace('"', '\\"').replace("%", "%%").replace("$", "$$") + '"' +def unit_working_directory(path): + # Unlike ExecStart arguments, WorkingDirectory is a single literal path: + # surrounding quotes become part of the path, and $ is not expanded. + value = str(path) + if (not Path(value).is_absolute() or value != value.strip() + or any(ord(c) < 32 for c in value) or value.endswith("\\")): + raise ValueError("Unsupported service working-directory path.") + return value.replace("%", "%%") + + +def repair_service(): + """Migrate only the known quoted WorkingDirectory emitted by <= 0.1.3.""" + unit_dir = CONFIG.parent / "systemd/user" + path = unit_dir / UNIT + if not path.exists(): + return False + with (unit_dir / ".omaproxy-unit.lock").open("w") as lock: + fcntl.flock(lock, fcntl.LOCK_EX) + original = path.read_text() + broken = "WorkingDirectory=" + unit_quote(CONFIG) + fixed = "WorkingDirectory=" + unit_working_directory(CONFIG) + section = "" + lines = [] + changed = False + for line in original.splitlines(keepends=True): + if line.strip().startswith("["): + section = line.strip() + if section == "[Service]" and line.rstrip("\r\n") == broken: + line = fixed + ("\n" if line.endswith("\n") else "") + changed = True + lines.append(line) + if not changed: + return False + backup = unit_dir / (UNIT + ".before-working-directory-fix") + if not backup.exists(): + private_write(backup, original) + private_write(path, "".join(lines)) + try: + run(["systemctl", "--user", "daemon-reload"]) + except (OSError, subprocess.SubprocessError): + # Leave the recognizable old line so the next attempt retries reload. + private_write(path, original) + raise + return True + + def setup(binary=None, port=8317): if not 1024 <= port <= 65535: raise ValueError("Port must be between 1024 and 65535.") + working_directory = unit_working_directory(CONFIG) cfg = settings() if binary: binary = str(Path(binary).expanduser().resolve(strict=True)) @@ -311,7 +360,7 @@ def setup(binary=None, port=8317): "[Unit]", "Description=OmaProxy local AI proxy", "After=network-online.target", "", "[Service]", "Type=simple", f"ExecStart={unit_quote(binary)} --config {unit_quote(CONFIG / 'config.yaml')}", - f"WorkingDirectory={unit_quote(CONFIG)}", + f"WorkingDirectory={working_directory}", "Restart=on-failure", "RestartSec=3", "UMask=0077", "NoNewPrivileges=true", "", "[Install]", "WantedBy=default.target", ""]) private_write(unit_dir / UNIT, unit) @@ -503,7 +552,7 @@ def main(): p.add_argument("--binary", help="Use a local CLIProxyAPI or Plus executable") p.add_argument("--port", type=int, default=8317) for name in ("status", "start", "stop", "restart", "dashboard", "logs", "config", "logs-view", - "auth-status", "auth-cancel", "auth-open", "auth-callback", "custom-add", "preferences"): + "auth-status", "auth-cancel", "auth-open", "auth-callback", "custom-add", "preferences", "repair"): sub.add_parser(name) p = sub.add_parser("quotas") p.add_argument("--force", action="store_true") @@ -529,6 +578,9 @@ def main(): result = status() elif not settings(): raise ValueError("Set up the proxy first.") + elif args.action == "repair": + changed = repair_service() + result = {"message": "Service repaired." if changed else "Service needs no repair."} elif args.action == "quotas": result = quota_snapshot(args.force) elif args.action.startswith("auth-"): diff --git a/tests/test_service_unit.py b/tests/test_service_unit.py new file mode 100644 index 0000000..2bf8ffa --- /dev/null +++ b/tests/test_service_unit.py @@ -0,0 +1,93 @@ +"""Service serialization and upgrade migration, including the real systemd parser.""" +from pathlib import Path +import shutil +import subprocess +import sys +import tempfile +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).parents[1] / 'scripts')) +import omaproxy as bridge + + +class ServiceUnitTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.config = Path(self.temp.name) / 'config with spaces $dollar %percent' / 'omaproxy' + self.config.mkdir(parents=True) + override = patch.object(bridge, 'CONFIG', self.config) + override.start() + self.addCleanup(override.stop) + self.unit = self.config.parent / 'systemd/user' / bridge.UNIT + self.unit.parent.mkdir(parents=True) + + def old_unit(self): + text = '[Service]\nExecStart=/usr/bin/true\nWorkingDirectory=' + bridge.unit_quote(self.config) + '\nRestartSec=9\n' + self.unit.write_text(text) + return text + + def test_directory_is_literal_and_only_specifiers_are_escaped(self): + self.assertEqual(bridge.unit_working_directory('/tmp/a b/$value/%name'), '/tmp/a b/$value/%%name') + for path in ['relative', '/tmp/a\nExecStart=/bad', '/tmp/a\r', '/tmp/a\\']: + with self.subTest(path=path), self.assertRaises(ValueError): + bridge.unit_working_directory(path) + + def test_repair_preserves_custom_lines_and_backup_and_is_idempotent(self): + old = self.old_unit() + with patch.object(bridge, 'run') as run: + self.assertTrue(bridge.repair_service()) + self.assertFalse(bridge.repair_service()) + run.assert_called_once_with(['systemctl', '--user', 'daemon-reload']) + self.assertEqual(self.unit.read_text(), old.replace(bridge.unit_quote(self.config), bridge.unit_working_directory(self.config))) + self.assertEqual(self.unit.with_name(bridge.UNIT + '.before-working-directory-fix').read_text(), old) + + def test_reload_failure_rolls_back_for_retry(self): + old = self.old_unit() + with patch.object(bridge, 'run', side_effect=subprocess.CalledProcessError(1, ['systemctl'])): + with self.assertRaises(subprocess.CalledProcessError): + bridge.repair_service() + self.assertEqual(self.unit.read_text(), old) + with patch.object(bridge, 'run'): + self.assertTrue(bridge.repair_service()) + + def test_custom_directory_and_missing_unit_are_not_changed(self): + with patch.object(bridge, 'run') as run: + self.assertFalse(bridge.repair_service()) + self.unit.write_text('[Service]\nWorkingDirectory=/custom/path\n') + self.assertFalse(bridge.repair_service()) + run.assert_not_called() + self.assertEqual(self.unit.read_text(), '[Service]\nWorkingDirectory=/custom/path\n') + + @unittest.skipUnless(shutil.which('systemd-analyze'), 'systemd-analyze unavailable') + def test_fresh_setup_unit_passes_real_systemd_parser(self): + with patch.object(bridge, 'run', return_value=subprocess.CompletedProcess([], 0, '', '')): + bridge.setup('/usr/bin/true') + expected = 'WorkingDirectory=' + bridge.unit_working_directory(self.config) + self.assertIn(expected + '\n', self.unit.read_text()) + result = subprocess.run(['systemd-analyze', '--user', 'verify', str(self.unit)], capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_start_and_enable_repair_before_systemctl_action(self): + for action in ['start', 'restart', 'enable']: + self.old_unit() + with self.subTest(action=action), patch.object(bridge, 'run') as run: + bridge.systemctl(action) + self.assertEqual([call.args[0] for call in run.call_args_list], [ + ['systemctl', '--user', 'daemon-reload'], ['systemctl', '--user', action, bridge.UNIT]]) + + @unittest.skipUnless(shutil.which('systemd-analyze'), 'systemd-analyze unavailable') + def test_real_systemd_parser_rejects_old_and_accepts_generated_unit(self): + self.old_unit() + old = subprocess.run(['systemd-analyze', '--user', 'verify', str(self.unit)], capture_output=True, text=True) + self.assertIn('not absolute', old.stderr) + with patch.object(bridge, 'run'): + bridge.repair_service() + fixed = subprocess.run(['systemd-analyze', '--user', 'verify', str(self.unit)], capture_output=True, text=True) + self.assertEqual(fixed.returncode, 0, fixed.stderr) + self.assertNotIn('not absolute', fixed.stderr) + + +if __name__ == '__main__': + unittest.main() From 405b666e140dca47030f07c9e6beb5a16494458e Mon Sep 17 00:00:00 2001 From: Marlos001 Date: Sat, 3 Oct 2026 14:36:26 -0300 Subject: [PATCH 2/2] Keep remote state on connection changes and allow clearing client keys --- BarWidget.qml | 22 ++++++++++++++++------ docs/configuration.md | 5 +++++ scripts/omaproxy.py | 7 ++++++- tests/test_integration.py | 33 +++++++++++++++++++++++++++++++++ tests/test_remote.py | 14 ++++++++++++++ 5 files changed, 74 insertions(+), 7 deletions(-) diff --git a/BarWidget.qml b/BarWidget.qml index 48ab6b4..5d5a284 100644 --- a/BarWidget.qml +++ b/BarWidget.qml @@ -32,6 +32,7 @@ Panel { property int connectionRevision: 0 property bool changingConnection: false property bool editRemote: false + property bool clearRemoteApiKey: false readonly property bool remoteConnection: snapshot.mode === "remote" property double now: Date.now() / 1000 readonly property bool busy: action.running @@ -71,7 +72,9 @@ Panel { function receive(result) { if (result.error) { noticeError = true; notice = result.error; return } if (result.connection_changed) { - snapshot = ({configured: false, running: false, accounts: [], models: [], providers: [], connection_id: result.connection_id}) + snapshot = ({configured: false, running: false, accounts: [], models: [], providers: [], connection_id: result.connection_id, + mode: result.mode, base_url: result.base_url || "", remote_base_url: result.remote_base_url || "", + has_api_key: !!result.has_api_key}) quotaData = ({accounts: []}) auth = ({}) preferences = ({}) @@ -81,6 +84,7 @@ Panel { addingAccount = false addingKey = false editRemote = false + clearRemoteApiKey = false } if (result.auth !== undefined) { var wasWaiting = signingIn @@ -139,7 +143,7 @@ Panel { onOpenedChanged: { if (opened) { refresh(); refreshQuotas(false); if (snapshot.configured && !remoteConnection && !authPoll.running) authPoll.running = true } - else { revealedEmails = ({}); remoteManagementKey.text = ""; remoteApiKey.text = "" } + else { revealedEmails = ({}); remoteManagementKey.text = ""; remoteApiKey.text = ""; clearRemoteApiKey = false } } onPageChanged: { scroll.contentY = 0; if (page === 2 && snapshot.running) perform(["preferences"]) } Component.onCompleted: refresh() @@ -602,19 +606,25 @@ Panel { Hint { text: "Enter the server base URL without /v1. Use HTTPS, or localhost HTTP for an SSH tunnel." } Field { id: remoteUrl; placeholderText: "Server URL, e.g. https://proxy.example.com"; text: root.snapshot.base_url || root.snapshot.remote_base_url || ""; enabled: !root.busy } Field { id: remoteManagementKey; placeholderText: "Management key"; password: true; enabled: !root.busy } - Field { id: remoteApiKey; placeholderText: "Client API key (optional, for models)"; password: true; enabled: !root.busy } - Hint { text: "Blank keys keep saved values for the same URL. The management key enables accounts and quotas; the client key enables model discovery." } + Field { id: remoteApiKey; placeholderText: "Client API key (optional, for models)"; password: true; enabled: !root.busy && !root.clearRemoteApiKey } + ActionButton { + text: "Remove saved client API key" + active: root.clearRemoteApiKey + enabled: !root.busy + onClicked: { root.clearRemoteApiKey = !root.clearRemoteApiKey; remoteApiKey.text = "" } + } + Hint { text: "Blank keys keep saved values for the same URL. The management key enables accounts and quotas; the client key enables model discovery. Select Remove saved client API key and save to use management only." } ActionButton { text: root.changingConnection ? "Checking…" : "Test and save connection" enabled: !root.busy && remoteUrl.text.trim() !== "" onClicked: { - root.perform(["connection-save"], {base_url: remoteUrl.text, management_key: remoteManagementKey.text, api_key: remoteApiKey.text}) + root.perform(["connection-save"], {base_url: remoteUrl.text, management_key: remoteManagementKey.text, api_key: remoteApiKey.text, clear_api_key: root.clearRemoteApiKey}) remoteManagementKey.text = "" remoteApiKey.text = "" } } } - ActionButton { visible: !root.snapshot.configured && !root.editRemote; text: "Set up local proxy"; enabled: !root.busy; onClicked: root.perform(["setup"]) } + ActionButton { visible: !root.snapshot.configured && !root.editRemote && !root.remoteConnection; text: "Set up local proxy"; enabled: !root.busy; onClicked: root.perform(["setup"]) } PanelSeparator { foreground: root.foreground } Label { text: "Display"; font.bold: true } ActionButton { diff --git a/docs/configuration.md b/docs/configuration.md index 7655750..9018f3b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -75,6 +75,11 @@ rejected, automatic management requests stop until the connection is tested and saved again. Rejected client keys stop model polling without hiding accounts or quotas. No keys or remote details belong in GitHub reports. +To remove a saved optional client API key, select **Remove saved client API key** +and choose **Test and save connection**. Account and quota access keeps using +the management key; model discovery stops. Blank key fields otherwise preserve +the saved values for the same server URL. + ## Removal To remove the integration while retaining credentials: diff --git a/scripts/omaproxy.py b/scripts/omaproxy.py index 2d46f0f..7bb7609 100644 --- a/scripts/omaproxy.py +++ b/scripts/omaproxy.py @@ -134,6 +134,8 @@ def connection_save(payload): value = str(payload.get(key, "")).strip() # Blank fields preserve saved keys only for the same server. cfg[key] = value or (previous.get(key, "") if previous.get("base_url") == cfg["base_url"] else "") + if key == "api_key" and payload.get("clear_api_key") is True: + cfg[key] = "" if any(ord(c) < 32 or ord(c) > 126 for c in cfg[key]): raise ValueError("Keys must contain printable ASCII characters.") if not cfg["management_key"]: @@ -146,6 +148,7 @@ def connection_save(payload): for name in ("auth-error.json", "model-error.json"): (state_dir(cfg) / name).unlink(missing_ok=True) return {"connection_changed": True, "connection_id": connection_id(cfg), + "mode": "remote", "base_url": cfg["base_url"], "has_api_key": bool(cfg["api_key"]), "message": "Remote connection saved. Accounts and limits come from this server."} @@ -153,7 +156,9 @@ def connection_local(): connection = read_json(CONFIG / "connection.json", {}) connection["mode"] = "local" private_write(CONFIG / "connection.json", json.dumps(connection) + "\n") - return {"connection_changed": True, "connection_id": "local", "message": "Local connection selected."} + return {"connection_changed": True, "connection_id": "local", "mode": "local", + "remote_base_url": connection.get("remote", {}).get("base_url", ""), + "message": "Local connection selected."} def account_rows(response): diff --git a/tests/test_integration.py b/tests/test_integration.py index ca7c23c..41371a7 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -132,6 +132,39 @@ def test_bridge_changes_routing_strategy(self): finally: omaproxy.api("routing/strategy", "PUT", {"value": "round-robin"}) + def test_remote_connection_uses_real_management_and_client_apis(self): + with tempfile.TemporaryDirectory() as directory, \ + patch.object(omaproxy, "CONFIG", Path(directory)), \ + patch.object(omaproxy, "run", side_effect=AssertionError("Remote mode must not invoke local processes")): + local = {"port": 18317, "api_key": "local-client", "management_key": "local-management"} + omaproxy.private_write(Path(directory) / "settings.json", json.dumps(local)) + result = omaproxy.connection_save({"base_url": self.base, + "management_key": "test-management-key", "api_key": "test-client-key"}) + self.assertTrue(result["connection_changed"]) + status = omaproxy.status() + self.assertTrue(status["running"]) + self.assertEqual(status["mode"], "remote") + self.assertIn("test-model", status["models"]) + self.assertEqual(status["accounts"], []) + self.assertEqual(omaproxy.quota_snapshot()["quotas"]["accounts"], []) + for secret in ("test-management-key", "test-client-key"): + self.assertNotIn(secret, json.dumps(status)) + omaproxy.connection_local() + self.assertEqual(omaproxy.settings(), local) + + def test_remote_management_only_and_bad_key_validation(self): + with tempfile.TemporaryDirectory() as directory, patch.object(omaproxy, "CONFIG", Path(directory)): + omaproxy.connection_save({"base_url": self.base, "management_key": "test-management-key"}) + status = omaproxy.status() + self.assertTrue(status["running"]) + self.assertFalse(status["has_api_key"]) + self.assertEqual(status["models"], []) + before = (Path(directory) / "connection.json").read_text() + for payload in ({"management_key": "wrong-management-key"}, {"api_key": "wrong-client-key"}): + with self.subTest(payload=payload), self.assertRaises(urllib.error.HTTPError): + omaproxy.connection_save(dict(payload, base_url=self.base)) + self.assertEqual((Path(directory) / "connection.json").read_text(), before) + def test_streaming_completion(self): result = self.call("/v1/chat/completions", {"model": "test-model", "stream": True, "messages": [{"role": "user", "content": "Hi"}]}, raw=True) diff --git a/tests/test_remote.py b/tests/test_remote.py index 32eec2d..9ade227 100644 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -46,6 +46,9 @@ def test_remote_setup_needs_no_binary_or_service_and_preserves_local(self): code, result = self.cli(["connection-save"], self.cfg) self.assertEqual(code, 0) self.assertTrue(result["connection_changed"]) + self.assertEqual(result["mode"], "remote") + self.assertEqual(result["base_url"], self.cfg["base_url"]) + self.assertTrue(result["has_api_key"]) self.assertEqual(self.config.joinpath("settings.json").read_text(), local) self.assertEqual((self.config / "connection.json").stat().st_mode & 0o777, 0o600) self.assertEqual(request.call_args_list[0].args, (self.cfg["base_url"] + "/v0/management/auth-files", "management-secret")) @@ -85,6 +88,17 @@ def test_saved_keys_reused_only_for_same_server(self): omaproxy.connection_save({"base_url": "https://different.example.test"}) request.assert_not_called() + def test_explicitly_clear_saved_client_key_keeps_management_access(self): + self.save() + original_id = omaproxy.connection_id(self.cfg) + with patch.object(omaproxy, "request", return_value={"files": []}) as request: + result = omaproxy.connection_save({"base_url": self.cfg["base_url"], "clear_api_key": True}) + request.assert_called_once_with(self.cfg["base_url"] + "/v0/management/auth-files", "management-secret") + self.assertEqual(omaproxy.settings()["api_key"], "") + self.assertEqual(omaproxy.settings()["management_key"], "management-secret") + self.assertFalse(result["has_api_key"]) + self.assertNotEqual(result["connection_id"], original_id) + def test_management_only_connection_and_secret_allowlist(self): self.save(dict(self.cfg, api_key="")) with patch.object(omaproxy, "request", return_value={"files": [{"name": "fake", "access_token": "private-token"}]}), \