← Commit history

0.2.38-0.2.39: review-pass correctness fixes (cleanup_libs, set_params, add_body, extract_intlib, schematic_data params, licence gates, setup_trial wording) + a realistic selftest budget

John Lauer ·704ff1c079 ·1mo ago ·parent e508a9d
10 files changed +117−26
bridge-rs/Cargo.toml+1−1
@@ -1,6 +1,6 @@ [package] name = "altium-bridge"-version = "0.2.37"+version = "0.2.39" edition = "2021" description = "Adom altium-bridge — the bridge process, in Rust. Calls ab's verbs for the desktop; speaks file IPC to the resident DelphiScript agent inside Altium." license = "MIT"
bridge-rs/src/library.rs+3
@@ -356,6 +356,9 @@ pub fn add_symbol_art(args: &Value) -> Value {  /// Read-only: a SchLib component's drawing (primitive counts, body rectangle, polyline bbox, models). pub fn symbol_prims(args: &Value) -> Value {+    if let Some(g) = support::need_licensed() {+        return g;+    }     let lib = match s(args, "libraryPath") {         Some(l) if !l.is_empty() => l,         _ => return json!({"success": false, "error": "libraryPath (.SchLib) required"}),
bridge-rs/src/license.rs+2−1
@@ -193,7 +193,8 @@ pub fn setup_trial(_args: &Value) -> Value {     let lic = instance::licence_state();     if lic.get("running").and_then(|b| b.as_bool()) != Some(true) {         return json!({"success": true, "stage": "launch", "installed": true,-            "summary": "Altium is installed but not running. Launching it next.",+            "summary": "Altium is installed but not running. This verb reports the next step, it does not launch: call altium_launch (or altium_ensure_licensed, which launches and claims a seat), then altium_setup_trial again.",+            "needsHuman": Value::Null,             "nextAction": "altium_launch (or altium_ensure_licensed)"});     }     let ss = signin_status(&json!({}));
bridge-rs/src/main.rs+1−1
@@ -29,7 +29,7 @@ use tiny_http::{Header, Method, Response, Server}; /// Attached to every failed reply (see the /command handler). const REPORT_ISSUES: &str = "If this looks like an altium-bridge BUG (not a refusal you expected), report it: check for a duplicate with `adom-wiki issue list adom/altium-bridge`, then `adom-wiki issue create adom/altium-bridge --title \"<verb>: <one-line symptom>\" --category bug-report --body \"<what you ran, what you expected, what happened, the VERBATIM response JSON (and errorShot if present), bridgeVersion, the Altium version from altium_status, the OS>\"`. A relay, routing, toast or lifecycle problem belongs on adom/adom-bridge instead. Reports from real machines are how this bridge gets correct: three different Altium builds behaved three different ways in one weekend."; -pub const BRIDGE_VERSION: &str = "0.2.37";+pub const BRIDGE_VERSION: &str = "0.2.39";  fn json_response(status: u16, body: &serde_json::Value) -> Response<std::io::Cursor<Vec<u8>>> {     let data = serde_json::to_vec(body).unwrap_or_else(|_| b"{}".to_vec());
bridge-rs/src/outputs.rs+3
@@ -227,6 +227,9 @@ pub fn export_step(args: &Value) -> Value { }  pub fn import_kicad(args: &Value) -> Value {+    if let Some(g) = support::need_licensed() {+        return g;+    }     let file = match s(args, "filePath") {         Some(f) if !f.is_empty() => f,         _ => return json!({"success": false, "error": "filePath (.kicad_sym / .kicad_sch / .kicad_pcb) is required"}),
bridge-rs/src/verbs.rs+11−5
@@ -119,8 +119,10 @@ fn describe() -> Value {               "A toast only reaches the machine it is shown on. Check toast.displayed and ALSO tell the user in chat."],             json!({"stage": "launch", "humanIsPresent": true})),         verb_doc("selftest_script_error", "Regression test for the script-error safety net (from 0.2.23): makes the agent raise a REAL DelphiScript runtime error on purpose and checks that the bridge photographs Altium's Error box, stops the halted script itself and leaves no dialog behind. Flashes one error box for under a second. Run it after any change to agent.rs / dialogs.rs and once on every new Altium build.",-            json!({}), json!({"netWorks": "bool", "outcome": "halted_and_recovered | contained_by_agent | unhandled", "errorShot": "base64 png (halted outcome only)", "autoRecovered": "bool", "dialogsLeft": "int"}), 60,-            "netWorks:true is the pass. The agent is stopped afterwards by design; the next verb restarts it.", &["agent_recover", "dialog_log", "agent_status"], &["It really does halt the engine for a moment: do not run it while a user is mid-edit in Altium."], json!({})),+            json!({}), json!({"netWorks": "bool", "outcome": "halted_and_recovered | contained_by_agent | unhandled", "errorShot": "base64 png (halted outcome only)", "autoRecovered": "bool", "dialogsLeft": "int"}), 180,+            "netWorks:true is the pass. The agent is stopped afterwards by design; the next verb restarts it.", &["agent_recover", "dialog_log", "agent_status"],+            &["It really does halt the engine for a moment: do not run it while a user is mid-edit in Altium.",+              "By design this is the SLOWEST verb in the catalog: it halts the engine, waits for the halt to be seen, recovers it and verifies by restarting the agent. Altium Designer Develop parks SILENTLY (no Error box), so the halt is only seen when the wait expires — measured 18 s to over 60 s on the same machine. Its budget is 180 s for that reason; a timeout here means the safety net is broken, not that it was slow."], json!({})),         verb_doc("symbol_prims", "Read a SchLib component's drawing: primitive counts by kind, body rectangle, polyline bounding box, display mode and the footprint models. Read-only, use it to verify add_symbol_art / set_models.",             json!({"libraryPath": "path (.SchLib)", "symbolName": "string", "models": "bool, default false (the model walk halts the engine on some builds)"}), json!({"pins": "int", "rectangles": "int", "lines": "int", "polylines": "int", "bodyRectMil": "[..]", "polyBboxMil": "[..]", "models": "[\"PCBLIB:NAME\"]"}), 60,             "polylines should equal what add_symbol_art reported and polyBboxMil should sit inside bodyRectMil.", &["add_symbol_art", "set_models", "read_symbol"], &[], json!({})),@@ -381,9 +383,9 @@ fn describe() -> Value {             "Use it to verify an imported footprint on a altium_test_board: pad layerId 74 = multi-layer (plated through), which is what altium_plate_vias produces.",             &["component_pins", "plate_vias", "test_board", "pcb_report"], &["Needs a .PcbDoc current: it reads a placed component, not a PcbLib member (altium_pcb_report reads libraries).", "3D bodies, fills and regions only show up in otherPrimitives."], json!({"designator": "U1"})),         verb_doc("schematic_data", "Components on the CURRENT schematic sheet with their pins (number, name, electrical type), position and orientation. designator filters to one part.",-            json!({"designator": "string - optional filter", "limit": "int, default 500", "pins": "bool, default true"}),+            json!({"designator": "string - optional filter", "limit": "int, default 500", "pins": "bool, default true", "params": "bool, default false - include each component's parameters"}),             json!({"data": "{document, count, components: [{designator, partId, libReference, comment, xMil, yMil, orientation, pins: [{number, name, electrical}], params: []}]}", "roundtripMs": "int"}), 120,-            "Pin-level view of a sheet. For parameters use altium_read_schematic (params come back empty here).",+            "Pin-level view of a sheet. params[] is empty unless you pass params:true; altium_read_schematic is the fuller parameter view.",             &["read_schematic", "cross_check", "show_schematic"], &["Needs a .SchDoc current; one sheet only.", "electrical is Altium's pin-type ordinal, not a name."], json!({"designator": "U1"})),         // ---- surfaces ----         verb_doc("show_project", "Open a project (.PrjPcb) through the agent and return a proof frame of Altium's main window. With no filePath it opens Altium's bundled example project (Bluetooth Sentinel).",@@ -1058,7 +1060,11 @@ pub fn dispatch(command: &str, args: &Value, _forwarded: &[(String, String)]) ->         },         "schematic_data" => {             let pins = if args.get("pins").and_then(|b| b.as_bool()) == Some(false) { "0" } else { "1" };-            board_read("schematic_data", vec![("designator", s(args, "designator").unwrap_or_default()), ("limit", s(args, "limit").unwrap_or("500".into())), ("pins", pins.into())], 120)+            // The agent has always honoured params=1; the dispatch never sent it, so every reply+            // carried an empty params[] and the doc had to apologise for it. Off by default:+            // a sheet's parameters are bulky and most callers want pins.+            let params = if args.get("params").and_then(|b| b.as_bool()) == Some(true) { "1" } else { "0" };+            board_read("schematic_data", vec![("designator", s(args, "designator").unwrap_or_default()), ("limit", s(args, "limit").unwrap_or("500".into())), ("pins", pins.into()), ("params", params.into())], 120)         }         // ---- surfaces ----         "show_project" => surfaces::show_project(args),
bridge.json+1−1
@@ -2,7 +2,7 @@   "manifest_version": 1,   "name": "altium",   "displayName": "Altium Designer",-  "version": "0.2.37",+  "version": "0.2.39",   "description": "Drive Altium Designer from Adom: a resident DelphiScript agent (eval + the 1,100-command process launcher), the X2 -R command line, UIA over the VCL shell, dialogs by PID, screenshots of every Altium window, and Altium 365.",   "homepage": "https://wiki.adom.inc/adom/altium-bridge",   "docs": "https://wiki.adom.inc/adom/altium-bridge",
dev-skills/altium-bridge-test/run_verb_tests.py+4−1
@@ -202,7 +202,10 @@ def matrix(w):     # ---- phase 5: the script lanes and the safety net -----------------------------------------     M["run_script"] = dict(phase=5, **ref("refused while the agent is resident (agent_resident)", proc="Nope"))     M["dismiss_dialogs"] = dict(phase=5, **anyo("nothing to dismiss"))-    M["selftest_script_error"] = dict(phase=5, timeout=120, **ok(), check=lambda d: d.get("netWorks") is True)+    # The slowest verb by design, and the slowest on ADDevelop, which parks SILENTLY so the halt is+    # only seen when the wait expires (18 s to over 60 s on one machine). Keep this ABOVE the verb's+    # own declared budget (180 s), or the battery blames the bridge for its own cut-off.+    M["selftest_script_error"] = dict(phase=5, timeout=220, **ok(), check=lambda d: d.get("netWorks") is True)     M["agent_recover"] = dict(phase=5, **ok())     M["agent_stop"] = dict(phase=6, **ok())     M["close"] = dict(phase=6, timeout=300, **ok(discardChanges=True, waitSec=90), note="releases the licence seat; every modified document is the battery's own scratch")
page.json+1−1
@@ -140,4 +140,4 @@     "skills/altium-wiki-import/SKILL.md",     "uninstall.sh"   ]-}\ No newline at end of file+}
src/agent/AdomAgent.pas.tmpl+90−15
@@ -1026,10 +1026,28 @@ Begin     Result := '{' + JRaw('ok', 'true') + ',' + JStr('libpkg', Path) + ',' + JStr('outputDir', OutDir) + '}'; End; +{ The folder part of a path, ending in '\'. Built from Copy/Length only: SysUtils names that+  this engine does not expose raise an Error box and park the script on a breakpoint, so the agent+  never calls one that is not already proven elsewhere in this file. }+Function AdomDirOf(P : String) : String;+Var+    I : Integer;+Begin+    Result := '';+    For I := Length(P) DownTo 1 Do+    Begin+        If (Copy(P, I, 1) = '\') Or (Copy(P, I, 1) = '/') Then+        Begin+            Result := Copy(P, 1, I);+            Break;+        End;+    End;+End;+ { Extract editable SchLib/PcbLib sources from a compiled .IntLib: path=<IntLib> } Function CmdExtractIntLib(Lines : TStringList) : String; Var-    Path, P, Acc : String;+    Path, P, Acc, Dir, UP : String;     WS : IWorkspace;     Prj : IProject;     Doc : IDocument;@@ -1042,6 +1060,10 @@ Begin         Exit;     End;     IntegratedLibraryManager.ExtractSources(Path);+    { ExtractSources writes the editable libraries BESIDE the .IntLib. This used to report every+      .SchLib/.PcbLib open anywhere in the workspace, so a caller with other libraries open was+      handed paths that had nothing to do with this IntLib. Scope it to the IntLib's own folder. }+    Dir := UpperCase(AdomDirOf(Path));     Acc := '';     WS := GetWorkspace;     If WS <> Nil Then@@ -1052,14 +1074,17 @@ Begin             Begin                 Doc := Prj.DM_LogicalDocuments(J);                 P := Doc.DM_FullPath;-                If (Pos('.SchLib', P) > 0) Or (Pos('.PcbLib', P) > 0) Or (Pos('.SCHLIB', P) > 0) Or (Pos('.PCBLIB', P) > 0) Then+                UP := UpperCase(P);+                If ((Pos('.SCHLIB', UP) > 0) Or (Pos('.PCBLIB', UP) > 0)) And+                   ((Dir = '') Or (Pos(Dir, UP) = 1)) Then                 Begin                     If Acc <> '' Then Acc := Acc + ',';                     Acc := Acc + '"' + JsonEsc(P) + '"';                 End;             End;         End;-    Result := '{' + JRaw('ok', 'true') + ',' + JStr('intlib', Path) + ',' + JRaw('sources', '[' + Acc + ']') + '}';+    Result := '{' + JRaw('ok', 'true') + ',' + JStr('intlib', Path) + ',' ++        JStr('searchedUnder', AdomDirOf(Path)) + ',' + JRaw('sources', '[' + Acc + ']') + '}'; End;  { Set/replace parameters on a component of the CURRENT SchLib and optionally link a footprint.@@ -1102,6 +1127,7 @@ Var     Iter : ISch_Iterator;     Comp : ISch_Component;     Impl : ISch_Implementation;+    C2   : ISch_Component;     Name, Fp, Cmt, Spec, K, V : String;     Items : TStringList;     I, P, N : Integer;@@ -1115,19 +1141,35 @@ Begin         Result := '{' + JRaw('ok', 'false') + ',' + JStr('error', 'current document is not a schematic library') + '}';         Exit;     End;+    { With no name this used to take whichever component the iterator yielded FIRST, so a+      set_params against a multi-symbol library silently wrote the footprint and comment onto+      an arbitrary part. An omitted name may only mean "the one component in here". }     Comp := Nil;+    N := 0;     Iter := SLib.SchLibIterator_Create;     Iter.AddFilter_ObjectSet(MkSet(eSchComponent));-    Comp := Iter.FirstSchObject;-    While Comp <> Nil Do+    C2 := Iter.FirstSchObject;+    While C2 <> Nil Do     Begin-        If (Name = '') Or (UpperCase(Comp.LibReference) = UpperCase(Name)) Then Break;-        Comp := Iter.NextSchObject;+        N := N + 1;+        If Name = '' Then+        Begin+            If Comp = Nil Then Comp := C2;+        End+        Else If UpperCase(C2.LibReference) = UpperCase(Name) Then Comp := C2;+        C2 := Iter.NextSchObject;     End;     SLib.SchIterator_Destroy(Iter);+    If (Name = '') And (N > 1) Then+    Begin+        Result := '{' + JRaw('ok', 'false') + ',' + JStr('errorCode', 'name_required') + ',' ++            JStr('error', 'this library holds ' + IntToStr(N) + ' components: name the one to set') + '}';+        Exit;+    End;     If Comp = Nil Then     Begin-        Result := '{' + JRaw('ok', 'false') + ',' + JStr('error', 'component not found: ' + Name) + '}';+        Result := '{' + JRaw('ok', 'false') + ',' + JStr('errorCode', 'name_not_found') + ',' ++            JStr('error', 'component not found: ' + Name) + '}';         Exit;     End;     N := 0;@@ -1289,6 +1331,14 @@ Begin         Result := '{' + JRaw('ok', 'false') + ',' + JStr('error', 'current document is not a PCB library') + '}';         Exit;     End;+    { An omitted name may only mean "the one footprint in here": adding a 3D body to whichever+      footprint happened to be first is a silent edit of the wrong part. }+    If (Name = '') And (PLib.ComponentCount > 1) Then+    Begin+        Result := '{' + JRaw('ok', 'false') + ',' + JStr('errorCode', 'name_required') + ',' ++            JStr('error', 'this library holds ' + IntToStr(PLib.ComponentCount) + ' footprints: name the one to add the body to') + '}';+        Exit;+    End;     Found := False;     For I := 0 To PLib.ComponentCount - 1 Do     Begin@@ -3662,17 +3712,37 @@ Var     PLib : IPCB_Library;     C    : ISch_Component;     F    : IPCB_LibComponent;-    I, NRemoved : Integer;+    PIt  : ISch_Iterator;+    Pin  : ISch_Pin;+    NPin, NRemoved, NKept : Integer; Begin     NRemoved := 0;+    NKept    := 0;     SLib := SchServer.GetCurrentSchDocument;     If (SLib <> Nil) And (SLib.ObjectID = eSchLib) Then     Begin         C := FindSchComp(SLib, 'Component_1');         If C <> Nil Then         Begin-            SLib.RemoveSchComponent(C);-            NRemoved := NRemoved + 1;+            { Only the FACTORY stray, never a symbol someone drew into it. The PcbLib side has+              always checked this (GroupCount = 0); the SchLib side deleted whatever was called+              Component_1, pins and all. }+            NPin := 0;+            PIt := C.SchIterator_Create;+            PIt.AddFilter_ObjectSet(MkSet(ePin));+            Pin := PIt.FirstSchObject;+            While Pin <> Nil Do+            Begin+                NPin := NPin + 1;+                Pin := PIt.NextSchObject;+            End;+            C.SchIterator_Destroy(PIt);+            If NPin = 0 Then+            Begin+                SLib.RemoveSchComponent(C);+                NRemoved := NRemoved + 1;+            End+            Else NKept := NKept + 1;         End;         SLib.GraphicallyInvalidate;     End;@@ -3680,14 +3750,19 @@ Begin     If PLib <> Nil Then     Begin         F := FindFp(PLib, 'PCBComponent_1');-        If (F <> Nil) And (GroupCount(F, ePadObject) = 0) Then+        If F <> Nil Then         Begin-            PLib.RemoveComponent(F);-            NRemoved := NRemoved + 1;+            If GroupCount(F, ePadObject) = 0 Then+            Begin+                PLib.RemoveComponent(F);+                NRemoved := NRemoved + 1;+            End+            Else NKept := NKept + 1;         End;         PLib.Board.ViewManager_FullUpdate;     End;-    Result := '{' + JRaw('ok','true') + ',' + JRaw('removed', IntToStr(NRemoved)) + '}';+    Result := '{' + JRaw('ok','true') + ',' + JRaw('removed', IntToStr(NRemoved)) ++        ',' + JRaw('keptNotEmpty', IntToStr(NKept)) + '}'; End;  { Show or hide a named parameter on a SchLib component. name, param, visible=0|1 }