From 4e2cbb188c3741c7e98f8a7fb509223491d5d6df Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 7 Jul 2025 13:59:12 +0200 Subject: [PATCH 1/7] api/generators: remove term 'vars' interact purely with 'generators' --- .../ui-2d/src/routes/machines/install/vars-step.tsx | 2 +- pkgs/clan-cli/clan_cli/tests/test_vars.py | 10 +++++----- pkgs/clan-cli/clan_cli/vars/generate.py | 4 ++-- pkgs/clan-cli/clan_lib/tests/test_create.py | 6 +++--- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkgs/clan-app/ui-2d/src/routes/machines/install/vars-step.tsx b/pkgs/clan-app/ui-2d/src/routes/machines/install/vars-step.tsx index d257ebcb5..ef8e3f868 100644 --- a/pkgs/clan-app/ui-2d/src/routes/machines/install/vars-step.tsx +++ b/pkgs/clan-app/ui-2d/src/routes/machines/install/vars-step.tsx @@ -173,7 +173,7 @@ export const VarsStep = (props: VarsStepProps) => { toast.error("Error fetching data"); return; } - const result = await callApi("generate_vars_for_machine", { + const result = await callApi("run_generators", { machine_name: props.machine_id, base_dir: props.dir, generators: generatorsQuery.data.map((generator) => generator.name), diff --git a/pkgs/clan-cli/clan_cli/tests/test_vars.py b/pkgs/clan-cli/clan_cli/tests/test_vars.py index 899050f7d..432068ebe 100644 --- a/pkgs/clan-cli/clan_cli/tests/test_vars.py +++ b/pkgs/clan-cli/clan_cli/tests/test_vars.py @@ -10,9 +10,9 @@ from clan_cli.tests.helpers import cli from clan_cli.vars.check import check_vars from clan_cli.vars.generate import ( Generator, - create_machine_vars, + run_generators, create_machine_vars_interactive, - get_machine_generators, + get_generators, ) from clan_cli.vars.get import get_machine_var from clan_cli.vars.graph import all_missing_closure, requested_closure @@ -668,7 +668,7 @@ def test_api_set_prompts( monkeypatch.chdir(flake.path) - create_machine_vars( + run_generators( machine_name="my_machine", base_dir=flake.path, generators=["my_generator"], @@ -682,7 +682,7 @@ def test_api_set_prompts( store = in_repo.FactStore(machine) assert store.exists(Generator("my_generator"), "prompt1") assert store.get(Generator("my_generator"), "prompt1").decode() == "input1" - create_machine_vars( + run_generators( machine_name="my_machine", base_dir=flake.path, generators=["my_generator"], @@ -694,7 +694,7 @@ def test_api_set_prompts( ) assert store.get(Generator("my_generator"), "prompt1").decode() == "input2" - generators = get_machine_generators( + generators = get_generators( machine_name="my_machine", base_dir=flake.path, full_closure=True, diff --git a/pkgs/clan-cli/clan_cli/vars/generate.py b/pkgs/clan-cli/clan_cli/vars/generate.py index 67c0fa071..3c3764a75 100644 --- a/pkgs/clan-cli/clan_cli/vars/generate.py +++ b/pkgs/clan-cli/clan_cli/vars/generate.py @@ -423,7 +423,7 @@ def get_closure( @API.register -def get_machine_generators( +def get_generators( machine_name: str, base_dir: Path, full_closure: bool = False, @@ -461,7 +461,7 @@ def _generate_vars_for_machine( @API.register -def create_machine_vars( +def run_generators( machine_name: str, generators: list[str], all_prompt_values: dict[str, dict[str, str]], diff --git a/pkgs/clan-cli/clan_lib/tests/test_create.py b/pkgs/clan-cli/clan_lib/tests/test_create.py index cae4d5590..58809bd7e 100644 --- a/pkgs/clan-cli/clan_lib/tests/test_create.py +++ b/pkgs/clan-cli/clan_lib/tests/test_create.py @@ -14,7 +14,7 @@ from clan_cli.machines.create import create_machine from clan_cli.secrets.key import generate_key from clan_cli.secrets.sops import maybe_get_admin_public_keys from clan_cli.secrets.users import add_user -from clan_cli.vars.generate import create_machine_vars, get_machine_generators +from clan_cli.vars.generate import run_generators, get_generators from clan_lib.api.disk import hw_main_disk_options, set_machine_disk_schema from clan_lib.api.modules import list_modules @@ -222,7 +222,7 @@ def test_clan_create_api( # Invalidate cache because of new inventory clan_dir_flake.invalidate_cache() - generators = get_machine_generators(machine.name, machine.flake.path) + generators = get_generators(machine.name, machine.flake.path) all_prompt_values = {} for generator in generators: prompt_values = {} @@ -235,7 +235,7 @@ def test_clan_create_api( raise ClanError(msg) all_prompt_values[generator.name] = prompt_values - create_machine_vars( + run_generators( machine_name=machine.name, base_dir=machine.flake.path, generators=[gen.name for gen in generators], From 80d0dc98056b86ae92e3e5ee6c3eb95d263e7e58 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 7 Jul 2025 14:04:39 +0200 Subject: [PATCH 2/7] api/hw: rename generate_machine_hardware_info into 'run' --- .../ui-2d/src/routes/machines/install/hardware-step.tsx | 2 +- pkgs/clan-cli/clan_cli/machines/hardware.py | 4 ++-- pkgs/clan-cli/clan_lib/machines/hardware.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkgs/clan-app/ui-2d/src/routes/machines/install/hardware-step.tsx b/pkgs/clan-app/ui-2d/src/routes/machines/install/hardware-step.tsx index 2696a12bc..0b9bb4ab3 100644 --- a/pkgs/clan-app/ui-2d/src/routes/machines/install/hardware-step.tsx +++ b/pkgs/clan-app/ui-2d/src/routes/machines/install/hardware-step.tsx @@ -127,7 +127,7 @@ export const HWStep = (props: StepProps) => { return; } - const r = await callApi("generate_machine_hardware_info", { + const r = await callApi("run_machine_hardware_info", { opts: { machine: { name: props.machine_id, diff --git a/pkgs/clan-cli/clan_cli/machines/hardware.py b/pkgs/clan-cli/clan_cli/machines/hardware.py index 4db7c0e92..5389c0efd 100644 --- a/pkgs/clan-cli/clan_cli/machines/hardware.py +++ b/pkgs/clan-cli/clan_cli/machines/hardware.py @@ -5,7 +5,7 @@ from pathlib import Path from clan_lib.machines.hardware import ( HardwareConfig, HardwareGenerateOptions, - generate_machine_hardware_info, + run_machine_hardware_info, ) from clan_lib.machines.machines import Machine from clan_lib.machines.suggestions import validate_machine_names @@ -38,7 +38,7 @@ def update_hardware_config_command(args: argparse.Namespace) -> None: host_key_check=args.host_key_check, private_key=args.identity_file ) - generate_machine_hardware_info(opts, target_host) + run_machine_hardware_info(opts, target_host) def register_update_hardware_config(parser: argparse.ArgumentParser) -> None: diff --git a/pkgs/clan-cli/clan_lib/machines/hardware.py b/pkgs/clan-cli/clan_lib/machines/hardware.py index 402213faa..1119a6c08 100644 --- a/pkgs/clan-cli/clan_lib/machines/hardware.py +++ b/pkgs/clan-cli/clan_lib/machines/hardware.py @@ -67,7 +67,7 @@ class HardwareGenerateOptions: @API.register -def generate_machine_hardware_info( +def run_machine_hardware_info( opts: HardwareGenerateOptions, target_host: Remote ) -> HardwareConfig: """ From cf92303f317decfccc163c909022e541e94a844d Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 7 Jul 2025 14:05:57 +0200 Subject: [PATCH 3/7] api/hw: rename 'describe_machine_hardware' into 'get_machine_hardware_summary' --- .../ui-2d/src/routes/machines/install/hardware-step.tsx | 2 +- pkgs/clan-cli/clan_lib/machines/hardware.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/clan-app/ui-2d/src/routes/machines/install/hardware-step.tsx b/pkgs/clan-app/ui-2d/src/routes/machines/install/hardware-step.tsx index 0b9bb4ab3..dfd7b1b95 100644 --- a/pkgs/clan-app/ui-2d/src/routes/machines/install/hardware-step.tsx +++ b/pkgs/clan-app/ui-2d/src/routes/machines/install/hardware-step.tsx @@ -71,7 +71,7 @@ export const HWStep = (props: StepProps) => { const hwReportQuery = useQuery(() => ({ queryKey: [props.dir, props.machine_id, "hw_report"], queryFn: async () => { - const result = await callApi("describe_machine_hardware", { + const result = await callApi("get_machine_hardware_summary", { machine: { flake: { identifier: props.dir, diff --git a/pkgs/clan-cli/clan_lib/machines/hardware.py b/pkgs/clan-cli/clan_lib/machines/hardware.py index 1119a6c08..08c50e0ac 100644 --- a/pkgs/clan-cli/clan_lib/machines/hardware.py +++ b/pkgs/clan-cli/clan_lib/machines/hardware.py @@ -157,7 +157,7 @@ class MachineHardwareBrief(TypedDict): @API.register -def describe_machine_hardware(machine: Machine) -> MachineHardwareBrief: +def get_machine_hardware_summary(machine: Machine) -> MachineHardwareBrief: """ Return a high-level summary of hardware config and platform type. """ From c382e8f1f36dcfadf0854a9bc417cb60e4bc5edf Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 7 Jul 2025 14:07:53 +0200 Subject: [PATCH 4/7] api/tasks: rename 'cancel_task' into 'delete_task' --- pkgs/clan-app/ui-2d/src/api/index.tsx | 2 +- pkgs/clan-app/ui/src/api/index.tsx | 2 +- pkgs/clan-cli/clan_lib/api/tasks.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkgs/clan-app/ui-2d/src/api/index.tsx b/pkgs/clan-app/ui-2d/src/api/index.tsx index cc02db123..519062817 100644 --- a/pkgs/clan-app/ui-2d/src/api/index.tsx +++ b/pkgs/clan-app/ui-2d/src/api/index.tsx @@ -90,7 +90,7 @@ const handleCancel = async ( orig_task: Promise>, ) => { console.log("Canceling operation: ", ops_key); - const { promise, op_key } = _callApi("cancel_task", { task_id: ops_key }); + const { promise, op_key } = _callApi("delete_task", { task_id: ops_key }); promise.catch((error) => { toast.custom( (t) => ( diff --git a/pkgs/clan-app/ui/src/api/index.tsx b/pkgs/clan-app/ui/src/api/index.tsx index cc02db123..519062817 100644 --- a/pkgs/clan-app/ui/src/api/index.tsx +++ b/pkgs/clan-app/ui/src/api/index.tsx @@ -90,7 +90,7 @@ const handleCancel = async ( orig_task: Promise>, ) => { console.log("Canceling operation: ", ops_key); - const { promise, op_key } = _callApi("cancel_task", { task_id: ops_key }); + const { promise, op_key } = _callApi("delete_task", { task_id: ops_key }); promise.catch((error) => { toast.custom( (t) => ( diff --git a/pkgs/clan-cli/clan_lib/api/tasks.py b/pkgs/clan-cli/clan_lib/api/tasks.py index 49fbbce0a..7ce48f049 100644 --- a/pkgs/clan-cli/clan_lib/api/tasks.py +++ b/pkgs/clan-cli/clan_lib/api/tasks.py @@ -17,7 +17,7 @@ BAKEND_THREADS: dict[str, WebThread] | None = None @API.register_abstract -def cancel_task(task_id: str) -> None: +def delete_task(task_id: str) -> None: """Cancel a task by its op_key.""" assert BAKEND_THREADS is not None, "Backend threads not initialized" future = BAKEND_THREADS.get(task_id) From 947e0a5488e0fabee56f3cbb44ce95c2fe4aa2b7 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 7 Jul 2025 14:35:56 +0200 Subject: [PATCH 5/7] openapi: add strict verb checking --- pkgs/clan-cli/openapi.py | 72 +++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 27 deletions(-) diff --git a/pkgs/clan-cli/openapi.py b/pkgs/clan-cli/openapi.py index 069fc244b..d33996164 100644 --- a/pkgs/clan-cli/openapi.py +++ b/pkgs/clan-cli/openapi.py @@ -5,23 +5,27 @@ from pathlib import Path # !!! IMPORTANT !!! # AVOID VERBS NOT IN THIS LIST -# We might restrict this even further to build a consistent and easy to use API +# IF YOU WANT TO ADD TO THIS LIST CREATE AN ISSUE/DISCUSS FIRST +# +# Verbs are restricted to make API usage intuitive and consistent. +# +# Discouraged verbs: +# do Too vague +# process Sounds generic; lacks clarity. +# generate Ambiguous: does it mutate state or not? Prefer 'run' +# handle Abstract and fuzzy +# show overlaps with get or list +# describe overlap with get or list +# can, is often used for helpers, use check instead for structure responses COMMON_VERBS = { - "get", - "list", - "show", - "set", - "create", - "update", - "delete", - "generate", - "maybe", - "open", - "flash", - "install", - "deploy", - "check", - "cancel", + "get", # fetch single item + "list", # fetch collection + "create", # instantiate resource + "set", # update or configure + "delete", # remove resource + "open", # initiate session, shell, file, etc. + "check", # validate, probe, or assert + "run", # start imperative task or action; machine-deploy etc. } @@ -39,7 +43,8 @@ def singular(word: str) -> str: return word -def normalize_tag(parts: list[str]) -> list[str]: +def normalize_op_name(op_name: str) -> list[str]: + parts = op_name.lower().split("_") # parts contains [ VERB NOUN NOUN ... ] # Where each NOUN is a SUB-RESOURCE verb = parts[0] @@ -53,20 +58,21 @@ def normalize_tag(parts: list[str]) -> list[str]: return [verb, *nouns] -def operation_to_tag(op_name: str) -> str: - def check_operation_name(verb: str, _resource_nouns: list[str]) -> None: - if not is_verb(verb): - print( - f"""⚠️ WARNING: Verb '{op_name}' of API operation {op_name} is not allowed. +def check_operation_name(op_name: str, normalized: list[str]) -> list[str]: + verb = normalized[0] + _nouns = normalized[1:] + warnings = [] + if not is_verb(verb): + warnings.append( + f"""Verb '{verb}' of API operation {op_name} is not allowed. Use one of: {", ".join(COMMON_VERBS)} """ - ) + ) + return warnings - parts = op_name.lower().split("_") - normalized = normalize_tag(parts) - - check_operation_name(normalized[0], normalized[1:]) +def operation_to_tag(op_name: str) -> str: + normalized = normalize_op_name(op_name) return " / ".join(normalized[1:]) @@ -134,6 +140,18 @@ def main() -> None: "components": {"schemas": {}}, } + # === Convert each function === + warnings = [] + for func_name, _ in functions.items(): + normalized = normalize_op_name(func_name) + check_res = check_operation_name(func_name, normalized) + if check_res: + warnings.append(check_res) + if warnings: + for message in warnings: + print(f"⚠️ WARNING: {message}") + os.abort() + # === Convert each function === for func_name, func_schema in functions.items(): args_schema = fix_nullables(deepcopy(func_schema["properties"]["arguments"])) From 2a3d1efc6fb83291379dfffa573c2a67552c27ab Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 7 Jul 2025 14:51:15 +0200 Subject: [PATCH 6/7] api: expose docstring as function description --- pkgs/clan-cli/clan_lib/api/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pkgs/clan-cli/clan_lib/api/__init__.py b/pkgs/clan-cli/clan_lib/api/__init__.py index 76c813936..64a9df0ae 100644 --- a/pkgs/clan-cli/clan_lib/api/__init__.py +++ b/pkgs/clan-cli/clan_lib/api/__init__.py @@ -254,6 +254,7 @@ API.register(open_file) "type": "object", "required": ["arguments", "return"], "additionalProperties": False, + "description": func.__doc__, "properties": { "return": return_type, "arguments": { From fd07d02d2db7c5edfa21abf31d3c9c898730f7ac Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 7 Jul 2025 14:52:49 +0200 Subject: [PATCH 7/7] openapi: warn on missing description --- pkgs/clan-cli/openapi.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/pkgs/clan-cli/openapi.py b/pkgs/clan-cli/openapi.py index d33996164..af70d47cd 100644 --- a/pkgs/clan-cli/openapi.py +++ b/pkgs/clan-cli/openapi.py @@ -140,16 +140,26 @@ def main() -> None: "components": {"schemas": {}}, } - # === Convert each function === - warnings = [] - for func_name, _ in functions.items(): + # === Check all functions === + warnings: list[str] = [] + errors: list[str] = [] + for func_name, func_schema in functions.items(): normalized = normalize_op_name(func_name) check_res = check_operation_name(func_name, normalized) if check_res: - warnings.append(check_res) + errors.extend(check_res) + + if not func_schema.get("description"): + warnings.append( + f"{func_name} doesn't have a description. Python docstring is required for an API function." + ) + if warnings: for message in warnings: - print(f"⚠️ WARNING: {message}") + print(f"⚠️ Warn: {message}") + if errors: + for m in errors: + print(f"❌ Error: {m}") os.abort() # === Convert each function === @@ -168,6 +178,7 @@ def main() -> None: "post": { "summary": func_name, "operationId": func_name, + "description": func_schema.get("description"), "tags": [tag], "requestBody": { "required": True,