From 8d09d44b4ce7fed1ce15600d8260a11b36191e43 Mon Sep 17 00:00:00 2001 From: Simon Dick Date: Sun, 9 Aug 2026 11:35:58 +0100 Subject: [PATCH 1/3] Add the cooperative geometry port (WHERE), the escape hatch for layout.gadget children (#49) layout.gadget children are permanently invisible to structural walking on classic AmigaOS 3.x, so no plain manifest entry could ever address them. This adds manifest format v2 (WHEREPORT/WHEREGADGET), a new WHERE verb, and routes CLICK/TYPE @name through an app's own cooperative ARexx port for geometry while still acting with real input.device input -- verified end to end against a new CAAPP.WHERE port on fixtures/classact-app, whose three gadgets previously had no manifest entry at all. Along the way, found and fixed a real gap: a hand-built RexxMsg (CreateRexxMsg/FillRexxMsg/PutMsg, the same recipe MUIREXX already uses) arrives with ln_Type left at NT_MESSAGE, causing rexxsyslib.library's own IsRexxMsg() to reject it even though the message is otherwise correct -- invisible in the existing MUIREXX check only because MUI-Demo never calls IsRexxMsg() on what it receives. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018CbPo4nMpnM5JQX3LKE5JP --- CLAUDE.md | 46 +++- Makefile | 2 +- docs/implementation-plan.md | 32 ++- fixtures/classact-app/CAApp.manifest | 26 +- fixtures/classact-app/README.md | 14 +- fixtures/classact-app/src/main.c | 344 ++++++++++++++++++++++++--- host/amipilot/client.py | 36 ++- host/tests/test_client.py | 57 +++++ manifest/README.md | 15 +- manifest/SPEC.md | 180 ++++++++++++-- server/README.md | 93 ++++++++ server/WIRE.md | 2 +- server/include/action_engine.h | 11 + server/include/arexx_cmd.h | 28 +++ server/include/manifest.h | 38 ++- server/include/where.h | 63 +++++ server/src/action.c | 21 ++ server/src/amipilotserver/main.c | 206 ++++++++++++++-- server/src/arexx_cmd.c | 31 +++ server/src/manifest.c | 74 +++++- server/src/where.c | 195 +++++++++++++++ tests/copperline/README.md | 36 +++ tests/copperline/run.sh | 106 +++++++++ tests/copperline/where-test.py | 110 +++++++++ userdocs/ARexx-Reference.md | 64 +++++ userdocs/Changelog.md | 19 ++ userdocs/Locator-Tiers-and-Limits.md | 21 +- userdocs/Wire-Protocol.md | 2 +- 28 files changed, 1749 insertions(+), 123 deletions(-) create mode 100644 server/include/where.h create mode 100644 server/src/where.c create mode 100644 tests/copperline/where-test.py diff --git a/CLAUDE.md b/CLAUDE.md index 29fc5e3..9f1ca23 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,51 @@ read that before making architectural decisions; this file only covers what's needed to build and navigate the code day to day. **Current state:** v1.0 released (phase 1.0 complete — the first full -release). Phase 0.5 (reliability and reach into the wider ecosystem) +release). Since then, the cooperative geometry port is real too (issue +#49, not yet in a tagged release): the escape hatch for gadgets nested +inside a `window.class` window's `layout.gadget`, permanently invisible +to structural walking on classic AmigaOS 3.x. A manifest gains a +version-2 record pair, `WHEREPORT ` and `WHEREGADGET + `, naming a small optional ARexx port the +*application itself* exposes, answering `WHERE ` with its own +live `GetAttr(GA_Left/GA_Top/GA_Width/GA_Height)` geometry; +`CLICK`/`TYPE @name` then act on that geometry with a genuine +`input.device` click (`AmipClickWindowRelative()`, +`server/src/action.c`) exactly as they would for a plain `GADGET` name +-- discovery is cooperative, but actuation stays real input, unlike +`MUIREXX`, where the target's own port does the acting too. The +standalone `WHERE` verb (`server/src/where.c`) is a diagnostic probe: +`GETTEXT`/`DRAG` have no path through a `WHEREPORT` at all, an honest +stated limit (RC 10, "geometry only") rather than a silent fallback. +See `manifest/SPEC.md`'s "The cooperative geometry port" section for +the full wire contract, including its "Clash guard" note on why +`WHEREPORT` resolution is exact-match only (no `MUIREXX`-style `.1` +fallback) and why applications should use a dedicated port name. +Verified end to end against `fixtures/classact-app`'s own new +`CAAPP.WHERE` port (`tests/copperline/run.sh`'s `run_where_check`, +`tests/copperline/where-test.py`) -- all three of that fixture's +gadgets, previously named by nothing at all (its manifest deliberately +named zero gadgets as the honest example of this exact limit), are now +addressed purely via `WHEREGADGET`; `CAApp.golden` is unchanged, +confirming the walker's own view of the window genuinely didn't change. +A real bug found building this, live (2026-08-09): a `RexxMsg` +constructed by hand via `CreateRexxMsg()`/`FillRexxMsg()`/`PutMsg()` -- +the same recipe `MUIREXX`'s own `AmipMuiRexxSend()` (`server/src/ +muirexx.c`) already used successfully against real MUI-Demo -- arrives +at the receiver with its node type left at `NT_MESSAGE`, not +`NT_REPLYMSG`; `rexxsyslib.library`'s own `IsRexxMsg()` reports such a +message as not a genuine `RexxMsg` at all despite every other field +being correct, confirmed by direct inspection of the raw message from +the fixture's own receiving side. This was invisible in the existing +MUIREXX check only because MUI-Demo's own ARexx handling never calls +`IsRexxMsg()` on what it receives -- fixed by having `CAAPP.WHERE` +(a port dedicated solely to this one protocol) trust any message that +arrives on it rather than gating on `IsRexxMsg()`, see the doc comment +on `HandleWhereMessage()` in `fixtures/classact-app/src/main.c`. +`MUIREXX` has this same latent gap, not fixed there since no real MUI +target has ever tripped it and it's outside this issue's scope. + +Phase 0.5 (reliability and reach into the wider ecosystem) before it: `WAITFOR` (including its `TEXT=` condition) and `CLICK`'s `EXPECT=` (wait/expectation primitives, docs/implementation-plan.md's "Async by design" section) diff --git a/Makefile b/Makefile index 0950442..f1724f3 100644 --- a/Makefile +++ b/Makefile @@ -110,7 +110,7 @@ AREXX_SRC := $(ACTION_SRCDIR)/arexx.c $(ACTION_SRCDIR)/arexx_cmd.c \ $(ACTION_SRCDIR)/manifest.c $(ACTION_SRCDIR)/serial.c \ $(ACTION_SRCDIR)/tcp.c $(ACTION_SRCDIR)/fs.c \ $(ACTION_SRCDIR)/muirexx.c $(ACTION_SRCDIR)/wblaunch.c \ - $(ACTION_SRCDIR)/screenshot.c + $(ACTION_SRCDIR)/screenshot.c $(ACTION_SRCDIR)/where.c AMIPILOTD_SRCDIR := server/src/amipilotserver AMIPILOTD_BIN := $(BUILD)/AmiPilotServer diff --git a/docs/implementation-plan.md b/docs/implementation-plan.md index c65bfd3..7f7ba6b 100644 --- a/docs/implementation-plan.md +++ b/docs/implementation-plan.md @@ -107,13 +107,13 @@ Three components: marked in the API as the fragile tier — for genuinely custom-rendered corners nothing structural can see, and nothing else. -**Future tier between 1 and 2 — the cooperative geometry port (design -note, not yet scheduled):** the layout.gadget child-enumeration limit -(see "Honest limits") blocks *external* discovery, but the application's -own code holds a live object pointer to every gadget it created — it -needs them for its own event dispatch. An app (or a GUI generator -emitting the app's boilerplate) can therefore expose a tiny optional -ARexx port answering `WHERE `: it calls +**A cooperative variant of tier 1, between it and tier 2 — the +cooperative geometry port (`WHERE`, issue #49, implemented):** the +layout.gadget child-enumeration limit (see "Honest limits") blocks +*external* discovery, but the application's own code holds a live +object pointer to every gadget it created — it needs them for its own +event dispatch. An app can therefore expose a tiny optional ARexx port +answering `WHERE `: it calls `GetAttr(GA_Left/GA_Top/GA_Width/GA_Height)` on its own object pointers and returns the live window-relative geometry. AmiPilot then acts on that answer with a genuine input.device click at those coordinates — @@ -122,10 +122,20 @@ real event path**, unlike the MUI tier where the port does the acting too. No coordinates ever appear in a script (they're resolved live at action time by the app itself), so relayout and font changes can't break anything — the same immunity the manifest tier has, extended to -the one place structural walking can't reach. The manifest format would -gain a record declaring the port's name so consumers can discover it. -This is the designated escape hatch for ReAction/window.class UIs whose -gadgets are otherwise permanently invisible to tiers 1–2. +the one place structural walking can't reach. The manifest format +gained a version-2 record pair for this — `WHEREPORT ` +(declaring the port) and `WHEREGADGET ` +(a gadget resolved through it instead of `GA_ID`) — see +`manifest/SPEC.md`'s "The cooperative geometry port" section for the +full wire contract and `server/README.md`'s own WHERE entry for what's +verified live, including a real bug found building it (a hand-built +`RexxMsg`'s node type needing to match what a genuine ARexx +interpreter's own messages carry before `IsRexxMsg()` will recognise +it). This is the escape hatch for ReAction/window.class UIs whose +gadgets are otherwise permanently invisible to tiers 1–2 — for +applications that implement it; a third party still can't retrofit it +onto a binary that doesn't, the same honest boundary quirk profiles +already have. Per-application **quirk profiles** (a small config layer) capture app-specific mappings and known oddities, so community knowledge about diff --git a/fixtures/classact-app/CAApp.manifest b/fixtures/classact-app/CAApp.manifest index 3753117..5805ecb 100644 --- a/fixtures/classact-app/CAApp.manifest +++ b/fixtures/classact-app/CAApp.manifest @@ -1,14 +1,22 @@ ; AmiPilot manifest for the ClassAct/ReAction conformance fixture -- see ; manifest/SPEC.md for the format. ; -; Deliberately names NO gadgets: this fixture's button/string/checkbox -; are children of a window.class window's layout.gadget, which classic -; AmigaOS 3.x provides no public API to enumerate (see the project's -; documented "Confirmed limit") -- they are unreachable by GA_ID, and -; the manifest format has no way to express "unreachable but trust me" -; on purpose. This file exists as the honest example of that boundary: -; a manifest names exactly what a consumer can actually reach, nothing -; more. -MANIFEST 1 +; This fixture's button/string/checkbox are children of a window.class +; window's layout.gadget, which classic AmigaOS 3.x provides no public +; API to enumerate (see the project's documented "Confirmed limit") -- +; they are unreachable by GA_ID, and a plain GADGET record has no way +; to express "unreachable but trust me" on purpose. This is the format +; version 2 example of the honest escape hatch for exactly that +; boundary (issue #49): the fixture itself exposes a small, optional +; ARexx port ("CAAPP.WHERE", see fixtures/classact-app/src/main.c) +; answering WHERE queries with each gadget's own live geometry, and +; every gadget below is a WHEREGADGET resolved through it -- not a +; workaround for the walker's own limit, a real, live-queried answer +; to it. +MANIFEST 2 APP CAApp +WHEREPORT CAAPP.WHERE WINDOW main "AmiPilot ClassAct Fixture" +WHEREGADGET connect_button main +WHEREGADGET host_field main +WHEREGADGET enabled_checkbox main diff --git a/fixtures/classact-app/README.md b/fixtures/classact-app/README.md index 5710013..241139c 100644 --- a/fixtures/classact-app/README.md +++ b/fixtures/classact-app/README.md @@ -2,4 +2,16 @@ Hand-written ClassAct/ReAction test application -- phase 0.1's primary `AmiInspect` fixture, later the server's click-and-assert conformance -target. Not yet implemented. +target. Built from `window.class`/`layout.gadget`/`button.gadget`/ +`string.gadget`/`checkbox.gadget` (not GadTools, unlike +`fixtures/gadtools-app`) -- its three gadgets are all `layout.gadget` +children, the project's own documented "Confirmed limit": permanently +invisible to structural walking on classic AmigaOS 3.x. + +Also implements the `CAAPP.WHERE` ARexx port (issue #49, `manifest/ +SPEC.md`'s "The cooperative geometry port") -- the cooperative escape +hatch for exactly that limit. `CAApp.manifest` (format version 2) +addresses all three gadgets via `WHEREGADGET` records resolved through +this port, rather than the plain `GADGET` records it could never use. +See `src/main.c`'s own header comment and `tests/copperline/ +where-test.py` for the end-to-end confirmation. diff --git a/fixtures/classact-app/src/main.c b/fixtures/classact-app/src/main.c index 349d9b2..c62fd5b 100644 --- a/fixtures/classact-app/src/main.c +++ b/fixtures/classact-app/src/main.c @@ -9,6 +9,13 @@ * * Quits on close-gadget or the button being pressed. * + * Also implements the CAAPP.WHERE ARexx port (issue #49, + * manifest/SPEC.md's "The cooperative geometry port"): the layout- + * child limit above means none of this app's own three gadgets are + * reachable via a plain GADGET manifest entry, so CAApp.manifest (v2) + * names all three as WHEREGADGETs instead, resolved live through this + * port rather than by GA_ID. + * * CRITICAL (cost a long debugging session, 2026-08-05): every library * base below is explicitly initialized (= NULL). An uninitialized * `struct Library *WindowBase;` is a COMMON symbol, which does not stop @@ -22,9 +29,14 @@ * version of this file didn't, and failed exactly that way. */ +#include #include #include +#include +#include +#include +#include #include #include #include @@ -32,6 +44,18 @@ #include #include +/* Included in this exact order (proto/rexxsyslib.h before the rexx/ + * headers, and both before __CLIB_PRAGMA_LIBCALL is defined below) to + * match server/src/arexx.c's own proven-working include order -- + * reordering these was found, empirically, to matter: with rexx/ + * storage.h included first instead, struct RexxMsg came out + * genuinely incomplete in this translation unit (sizeof reporting 0), + * so RexxSysBase's IsRexxMsg() silently operated on garbage instead + * of the real message. */ +#include +#include +#include + #define __CLIB_PRAGMA_LIBCALL #include #include @@ -50,16 +74,56 @@ struct Library *ButtonBase = NULL; struct Library *CheckBoxBase = NULL; struct Library *StringBase = NULL; +/* rexxsyslib.library's own base -- must be named exactly RexxSysBase + * (not static), since 's inline call stubs + * reference this global by name. Same convention server/src/arexx.c + * follows; opening it here (rather than requiring it, like the + * libraries above) is deliberate -- the WHEREPORT it backs is an + * OPTIONAL manifest feature (manifest/SPEC.md), so its absence + * degrades this fixture to "no cooperative geometry port", not a + * launch failure. */ +struct RxsLib *RexxSysBase = NULL; + #define GID_CONNECT 1 #define GID_HOST 2 #define GID_ENABLED 3 +/* The ARexx port name this fixture's CAApp.manifest declares via + * WHEREPORT -- see manifest/SPEC.md's "The cooperative geometry port" + * section and its "Clash guard" note recommending a dedicated, + * app-specific port name rather than reusing a general-purpose one. */ +#define CAAPP_WHERE_PORT "CAAPP.WHERE" + /* PLACETEXT_RIGHT's canonical home is , not any * ReAction header, despite CHECKBOX_TextPlace using the same constant -- * defined directly here rather than pulling in a GadTools header this * app has no other reason to depend on. */ #define CHECKBOX_TEXT_RIGHT 0x0002 +/* File-scope, not main()'s own locals: the WHERE port dispatcher + * (HandleWhereMessage() below) needs to read their live GA_Left/GA_Top/ + * GA_Width/GA_Height on demand, from the same event loop that created + * them, exactly the "the app already holds the pointers it needs for + * its own event dispatch" premise manifest/SPEC.md's "The cooperative + * geometry port" section describes. */ +static Object *g_connectButton = NULL; +static Object *g_hostString = NULL; +static Object *g_enabledCheckbox = NULL; +static struct MsgPort *g_wherePort = NULL; + +/* The WHEREGADGET logical names CAApp.manifest declares, mapped to the + * live object each one queries -- exact match, case-insensitive per + * the WHERE port contract. */ +static const struct { + const char *name; + Object **obj; +} g_whereTable[] = { + { "connect_button", &g_connectButton }, + { "host_field", &g_hostString }, + { "enabled_checkbox", &g_enabledCheckbox }, +}; +#define WHERE_TABLE_COUNT (sizeof(g_whereTable) / sizeof(g_whereTable[0])) + /* Raw dos.library Write() to Output() so failure paths are visible in a * Run-redirected log without depending on stdio buffering. */ static void Diag(const char *msg) @@ -67,12 +131,197 @@ static void Diag(const char *msg) Write(Output(), (APTR)msg, (LONG)strlen(msg)); } +/* Appends msg to a fixed, host-readable log file, independent of this + * process's own Output() stream. Used only for the WHERE end-to-end + * test's one observable (tests/copperline/where-test.py) -- "Run + * >file " was tried first and does NOT reliably route a + * background CLI's own Output() to the given file the way "Run >NIL: + * " reliably discards it (confirmed empirically: the + * redirected file captured only the new CLI's own startup banner, none + * of this program's own Diag() output at all, even lines logged before + * any WHERE handling) -- opening the file directly here sidesteps that + * uncertainty entirely. Silently does nothing if the path can't be + * opened (e.g. SRC: isn't assigned outside the dev harness this test + * runs under) -- an optional diagnostic, not something this fixture's + * own correctness depends on. */ +static void DiagFile(const char *path, const char *msg) +{ + BPTR fh = Open((CONST_STRPTR)path, MODE_READWRITE); + if (fh == 0) { + fh = Open((CONST_STRPTR)path, MODE_NEWFILE); + } + if (fh == 0) { + return; + } + Seek(fh, 0, OFFSET_END); + Write(fh, (APTR)msg, (LONG)strlen(msg)); + Close(fh); +} + +static int CiStreq(const char *a, const char *b) +{ + for (; *a && *b; a++, b++) { + int ca = *a, cb = *b; + if (ca >= 'a' && ca <= 'z') ca -= 32; + if (cb >= 'a' && cb <= 'z') cb -= 32; + if (ca != cb) return 0; + } + return *a == '\0' && *b == '\0'; +} + +static int CiStrnEqAscii(const char *a, const char *b, size_t n) +{ + size_t i; + for (i = 0; i < n; i++) { + int ca = a[i], cb = b[i]; + if (ca == '\0' || cb == '\0') return 0; + if (ca >= 'a' && ca <= 'z') ca -= 32; + if (cb >= 'a' && cb <= 'z') cb -= 32; + if (ca != cb) return 0; + } + return 1; +} + +/* Replies to a WHERE query -- same rm_Result1/rm_Result2/ReplyMsg + * shape as server/src/arexx.c's own AmipArexxReply(), duplicated + * (rather than shared -- this fixture links no AmiPilot server code) + * for the same "genuinely a different program" reason + * gadtools-app/classact-app never link intuition-model. Does not + * DeleteArgstring(rm_Result2) -- ARexx frees it after the caller + * consumes the reply, same note as AmipArexxReply's own. */ +static void ReplyWhere(struct RexxMsg *msg, int rc, const char *text) +{ + msg->rm_Result1 = rc; + msg->rm_Result2 = 0; + if ((msg->rm_Action & RXFF_RESULT) && text != NULL && text[0] != '\0') { + msg->rm_Result2 = (LONG)CreateArgstring((UBYTE *)text, (ULONG)strlen(text)); + } + ReplyMsg((struct Message *)msg); +} + +/* Handles one "WHERE " query against g_whereTable -- + * manifest/SPEC.md's "The cooperative geometry port" contract: reports + * the live GA_Left/GA_Top/GA_Width/GA_Height of the named object as + * " " (RC 0), or RC 10 for an unrecognised command or + * name. GA_Left/GA_Top/GA_Width/GA_Height are documented window- + * relative (including the window's own border/title-bar area), the + * same convention AmipGadgetCenter()/AmipClickWindowRelative() expect + * on the AmiPilot side (server/src/action.c). + * + * Deliberately does NOT gate on rexxsyslib.library's own IsRexxMsg() + * first, unlike server/src/arexx.c's own receiver -- confirmed + * empirically (2026-08-09) that a message built by hand via + * CreateRexxMsg()/FillRexxMsg()/PutMsg() (server/src/where.c's own + * AmipWhereQuery(), the same recipe server/src/muirexx.c's + * AmipMuiRexxSend() already uses) arrives with ln_Type left at + * NT_MESSAGE, not NT_REPLYMSG -- IsRexxMsg() reports such a message as + * NOT a RexxMsg even though it demonstrably is one (RXCOMM correctly + * set in rm_Action, ARG0() reads back the real command text). Real + * ARexx-interpreter-originated messages (a genuine `rx` script's own + * ADDRESS) apparently arrive already marked NT_REPLYMSG through some + * internal rexxsyslib mechanism this project's own hand-built sends + * don't reproduce -- IsRexxMsg() is seemingly meant for a SENDER + * validating its own reply, not a receiver validating an incoming + * command, and arexx.c's receiver-side use of it happens to work only + * because its senders are always real ARexx scripts, never this + * project's own MUIREXX/WHERE bridges. CAAPP.WHERE is a port + * dedicated solely to this one protocol (manifest/SPEC.md's own + * "Clash guard" -- a general-purpose ARexx port sharing this same + * port would need a real validity check here instead), so trusting + * every message that arrives on it is the correct, not merely + * expedient, choice. */ +static void HandleWhereMessage(struct RexxMsg *msg) +{ + const char *cmdline = (const char *)ARG0(msg); + const char *name; + unsigned int i; + + if (cmdline == NULL || !CiStrnEqAscii(cmdline, "WHERE", 5) + || (cmdline[5] != ' ' && cmdline[5] != '\t')) { + ReplyWhere(msg, 10, "unknown command"); + return; + } + name = cmdline + 5; + while (*name == ' ' || *name == '\t') name++; + + for (i = 0; i < WHERE_TABLE_COUNT; i++) { + if (CiStreq(name, g_whereTable[i].name)) { + Object *obj = *g_whereTable[i].obj; + ULONG gx = 0, gy = 0, gw = 0, gh = 0; + char reply[48]; + + GetAttr(GA_Left, obj, &gx); + GetAttr(GA_Top, obj, &gy); + GetAttr(GA_Width, obj, &gw); + GetAttr(GA_Height, obj, &gh); + sprintf(reply, "%ld %ld %ld %ld", (long)gx, (long)gy, (long)gw, (long)gh); + ReplyWhere(msg, 0, reply); + return; + } + } + ReplyWhere(msg, 10, "unknown name"); +} + +/* Creates the WHERE port under one Forbid() (matches server/src/ + * arexx.c's AmipArexxOpen() convention). RexxSysBase == NULL (library + * not present) degrades to no port at all -- graceful, same pattern + * as this file's own IntuitionBase/WindowBase/etc. opens, except this + * one is optional rather than fatal (see RexxSysBase's own doc + * comment above). */ +static struct MsgPort *OpenWherePort(void) +{ + struct MsgPort *port = NULL; + + if (RexxSysBase == NULL) { + return NULL; + } + + Forbid(); + if (FindPort((CONST_STRPTR)CAAPP_WHERE_PORT) == NULL) { + port = CreateMsgPort(); + if (port != NULL) { + port->mp_Node.ln_Name = (char *)CAAPP_WHERE_PORT; + AddPort(port); + } + } + Permit(); + return port; +} + +static void CloseWherePort(struct MsgPort *port) +{ + struct RexxMsg *msg; + + if (port == NULL) { + return; + } + + Forbid(); + RemPort(port); + Permit(); + + while ((msg = (struct RexxMsg *)GetMsg(port)) != NULL) { + if (IsRexxMsg(msg)) { + ReplyWhere(msg, 20, NULL); + } else { + ReplyMsg((struct Message *)msg); + } + } + DeleteMsgPort(port); +} + static void CleanExit(Object *windowObject, int rc) { + CloseWherePort(g_wherePort); + g_wherePort = NULL; + if (windowObject != NULL) { DisposeObject(windowObject); } + if (RexxSysBase != NULL) { + CloseLibrary((struct Library *)RexxSysBase); + } if (StringBase != NULL) { CloseLibrary(StringBase); } @@ -95,9 +344,11 @@ static void CleanExit(Object *windowObject, int rc) exit(rc); } -static void ProcessEvents(Object *windowObject) +static void ProcessEvents(Object *windowObject, struct MsgPort *wherePort) { ULONG windowSignal; + ULONG whereSignal = (wherePort != NULL) ? (1UL << wherePort->mp_SigBit) : 0; + ULONG signals; ULONG result; ULONG code; BOOL done = FALSE; @@ -105,7 +356,18 @@ static void ProcessEvents(Object *windowObject) GetAttr(WINDOW_SigMask, windowObject, &windowSignal); while (!done) { - Wait(windowSignal); + signals = Wait(windowSignal | whereSignal); + + if (whereSignal != 0 && (signals & whereSignal) != 0) { + struct RexxMsg *msg; + while ((msg = (struct RexxMsg *)GetMsg(wherePort)) != NULL) { + HandleWhereMessage(msg); + } + } + + if ((signals & windowSignal) == 0) { + continue; + } while ((result = DoMethod(windowObject, WM_HANDLEINPUT, &code)) != WMHI_LASTMSG) { switch (result & WMHI_CLASSMASK) { @@ -129,9 +391,6 @@ int main(void) struct Window *intuiWindow = NULL; Object *windowObject = NULL; Object *mainLayout = NULL; - Object *connectButton; - Object *hostString; - Object *enabledCheckbox; IntuitionBase = (struct IntuitionBase *)OpenLibrary((CONST_STRPTR)"intuition.library", 37); WindowBase = OpenLibrary((CONST_STRPTR)"window.class", 44); @@ -139,6 +398,7 @@ int main(void) ButtonBase = OpenLibrary((CONST_STRPTR)"gadgets/button.gadget", 44); CheckBoxBase = OpenLibrary((CONST_STRPTR)"gadgets/checkbox.gadget", 44); StringBase = OpenLibrary((CONST_STRPTR)"gadgets/string.gadget", 44); + RexxSysBase = (struct RxsLib *)OpenLibrary((CONST_STRPTR)"rexxsyslib.library", 0); Diag(IntuitionBase != NULL ? "caapp: intuition.library ok\n" : "caapp: intuition.library FAILED\n"); Diag(WindowBase != NULL ? "caapp: window.class ok\n" : "caapp: window.class FAILED\n"); @@ -146,41 +406,43 @@ int main(void) Diag(ButtonBase != NULL ? "caapp: button.gadget ok\n" : "caapp: button.gadget FAILED\n"); Diag(CheckBoxBase != NULL ? "caapp: checkbox.gadget ok\n" : "caapp: checkbox.gadget FAILED\n"); Diag(StringBase != NULL ? "caapp: string.gadget ok\n" : "caapp: string.gadget FAILED\n"); + /* Optional -- see RexxSysBase's own doc comment above. */ + Diag(RexxSysBase != NULL ? "caapp: rexxsyslib.library ok\n" : "caapp: rexxsyslib.library FAILED (no WHERE port)\n"); if (IntuitionBase == NULL || WindowBase == NULL || LayoutBase == NULL || ButtonBase == NULL || CheckBoxBase == NULL || StringBase == NULL) { CleanExit(NULL, RETURN_FAIL); } - connectButton = NewObject(NULL, (CONST_STRPTR)"button.gadget", - GA_ID, GID_CONNECT, - GA_Text, (ULONG)"Connect", - GA_RelVerify, TRUE, - TAG_DONE); + g_connectButton = NewObject(NULL, (CONST_STRPTR)"button.gadget", + GA_ID, GID_CONNECT, + GA_Text, (ULONG)"Connect", + GA_RelVerify, TRUE, + TAG_DONE); /* STRING_GetClass()/CHECKBOX_GetClass(), not NewObject(NULL, "name"): * unlike button.gadget, these classes don't register a public class * name, so lookup-by-name returns NULL (confirmed empirically under * WB 3.2.3 -- button ok, string/checkbox FAILED via the name form). */ - hostString = NewObject(STRING_GetClass(), NULL, - GA_ID, GID_HOST, - GA_Text, (ULONG)"Host:", - STRINGA_TextVal, (ULONG)"", - STRINGA_MaxChars, 64, - TAG_DONE); + g_hostString = NewObject(STRING_GetClass(), NULL, + GA_ID, GID_HOST, + GA_Text, (ULONG)"Host:", + STRINGA_TextVal, (ULONG)"", + STRINGA_MaxChars, 64, + TAG_DONE); - enabledCheckbox = NewObject(CHECKBOX_GetClass(), NULL, - GA_ID, GID_ENABLED, - GA_Text, (ULONG)"Enabled", - CHECKBOX_Checked, FALSE, - CHECKBOX_TextPlace, CHECKBOX_TEXT_RIGHT, - TAG_DONE); + g_enabledCheckbox = NewObject(CHECKBOX_GetClass(), NULL, + GA_ID, GID_ENABLED, + GA_Text, (ULONG)"Enabled", + CHECKBOX_Checked, FALSE, + CHECKBOX_TextPlace, CHECKBOX_TEXT_RIGHT, + TAG_DONE); - Diag(connectButton != NULL ? "caapp: button obj ok\n" : "caapp: button obj FAILED\n"); - Diag(hostString != NULL ? "caapp: string obj ok\n" : "caapp: string obj FAILED\n"); - Diag(enabledCheckbox != NULL ? "caapp: checkbox obj ok\n" : "caapp: checkbox obj FAILED\n"); + Diag(g_connectButton != NULL ? "caapp: button obj ok\n" : "caapp: button obj FAILED\n"); + Diag(g_hostString != NULL ? "caapp: string obj ok\n" : "caapp: string obj FAILED\n"); + Diag(g_enabledCheckbox != NULL ? "caapp: checkbox obj ok\n" : "caapp: checkbox obj FAILED\n"); - if (connectButton == NULL || hostString == NULL || enabledCheckbox == NULL) { + if (g_connectButton == NULL || g_hostString == NULL || g_enabledCheckbox == NULL) { CleanExit(NULL, RETURN_FAIL); } @@ -188,9 +450,9 @@ int main(void) LAYOUT_Orientation, LAYOUT_ORIENT_VERT, LAYOUT_SpaceInner, TRUE, LAYOUT_SpaceOuter, TRUE, - LAYOUT_AddChild, (ULONG)connectButton, - LAYOUT_AddChild, (ULONG)hostString, - LAYOUT_AddChild, (ULONG)enabledCheckbox, + LAYOUT_AddChild, (ULONG)g_connectButton, + LAYOUT_AddChild, (ULONG)g_hostString, + LAYOUT_AddChild, (ULONG)g_enabledCheckbox, TAG_DONE); if (mainLayout == NULL) { @@ -222,9 +484,31 @@ int main(void) CleanExit(windowObject, RETURN_FAIL); } + g_wherePort = OpenWherePort(); + Diag(g_wherePort != NULL ? "caapp: " CAAPP_WHERE_PORT " port ok\n" + : "caapp: " CAAPP_WHERE_PORT " port FAILED\n"); + Diag("caapp: window open, entering event loop\n"); - ProcessEvents(windowObject); + ProcessEvents(windowObject, g_wherePort); + + /* Observable for the WHERE end-to-end test (tests/copperline/ + * where-test.py): proves a TYPE @host_field click-then-type + * genuinely landed in the layout child's own string gadget -- + * GETTEXT can't read it back (the same layout.gadget-child limit + * this whole feature exists to work around), so this Diag() + * line, redirected to a host-readable log by the smoke script, is + * the only way to confirm it from outside. Logged before either + * exit path (close-gadget or the Connect button) so it always + * appears regardless of how the loop ended. */ + { + char logbuf[96]; + STRPTR hostText = NULL; + GetAttr(STRINGA_TextVal, g_hostString, (ULONG *)&hostText); + sprintf(logbuf, "caapp: host=%s\n", hostText != NULL ? (const char *)hostText : ""); + Diag(logbuf); + DiagFile("SRC:build/caapp-log.txt", logbuf); + } DoMethod(windowObject, WM_CLOSE); CleanExit(windowObject, RETURN_OK); diff --git a/host/amipilot/client.py b/host/amipilot/client.py index 8821b47..617362e 100644 --- a/host/amipilot/client.py +++ b/host/amipilot/client.py @@ -2,9 +2,9 @@ (server/WIRE.md framing) matching the verb set `AmiPilotServer` currently implements (TREE/CLICK/TYPE/GETTEXT/MANIFEST/LAUNCH/ FSLIST/FSSTAT/FSMKDIR/FSDELETE/FSGET/MENU/MENUPICK/DRAG/WAITFOR/ -SCREENS/VERSION/QUIT -- see server/README.md; windows/list, find, and -fs-put are still future scope, not invented here ahead of the server -actually offering them). +SCREENS/VERSION/MUIREXX/WHERE/QUIT -- see server/README.md; windows/ +list, find, and fs-put are still future scope, not invented here ahead +of the server actually offering them). Quoting matches the ARexx port's own command grammar (arexx_cmd.c): window-pattern/path arguments containing a space, a literal '"', or a @@ -1080,6 +1080,36 @@ def mui_command( f"MUIREXX {_quote(app_base)} TIMEOUT={int(timeout)} {command}" ).text + def where(self, name: str, *, timeout: float = 10.0) -> tuple[int, int, int, int]: + """WHERE @ [TIMEOUT=] -- diagnostic query of the + cooperative geometry port (issue #49, manifest/SPEC.md's "The + cooperative geometry port"). `name` must be a `WHEREGADGET` + entry in the currently loaded manifest (see `manifest()`), not + a plain `GADGET`. Returns the gadget's current geometry as + `(x, y, w, h)` -- pixels, relative to its own window's + top-left corner INCLUDING the border/title bar (add a window's + own reported `[left, top ...]` from `tree()` to convert to + screen coordinates). + + This is a probe/test primitive, not part of the normal + automation flow: `click_by_name()`/`type_by_name()` already + route through this same query automatically whenever the + manifest resolves `name` to a `WHEREGADGET`, with no separate + call needed. `where()` exists for asserting on geometry + directly, or diagnosing a misbehaving third-party `WHERE` + port. + + Raises `NotFound` if the declared `WHEREPORT` doesn't exist, + `CommandError` if `name` isn't in the manifest, resolves to a + plain `GADGET` instead of a `WHEREGADGET`, the port reports an + unknown name, or its reply doesn't parse as exactly four + integers, `Timeout` if no reply arrived within `timeout` + seconds, and `ActionFailed` if the server itself couldn't + allocate the ARexx message (out of memory).""" + text = self._run(f"WHERE @{name} TIMEOUT={int(timeout)}").text + x, y, w, h = (int(v) for v in text.split()) + return (x, y, w, h) + def quit(self) -> None: """QUIT -- shuts the server down cleanly. The connection is still open afterward (the server replies before exiting); call diff --git a/host/tests/test_client.py b/host/tests/test_client.py index def1b20..75e9f82 100644 --- a/host/tests/test_client.py +++ b/host/tests/test_client.py @@ -1174,5 +1174,62 @@ def test_alloc_fail_raises_action_failed(self): c.mui_command("MUIDEMO", "quit") +class Where(unittest.TestCase): + """Tests for the WHERE wire verb (where()) -- the cooperative + geometry port diagnostic (issue #49). See manifest/SPEC.md's "The + cooperative geometry port" for the underlying contract.""" + + def test_sends_at_name_verbatim(self): + payload = b"10 20 100 14" + c = client_with(b"RC 0 %d\n%s" % (len(payload), payload)) + c.where("connect_button") + self.assertEqual( + c._wire._t.sent[0], b"WHERE @connect_button TIMEOUT=10\n" + ) + + def test_honours_custom_timeout(self): + payload = b"10 20 100 14" + c = client_with(b"RC 0 %d\n%s" % (len(payload), payload)) + c.where("connect_button", timeout=30) + self.assertEqual( + c._wire._t.sent[0], b"WHERE @connect_button TIMEOUT=30\n" + ) + + def test_returns_parsed_geometry(self): + payload = b"10 20 100 14" + c = client_with(b"RC 0 %d\n%s" % (len(payload), payload)) + self.assertEqual(c.where("connect_button"), (10, 20, 100, 14)) + + def test_port_not_found_raises_not_found(self): + payload = b"WHERE port not found" + c = client_with(b"RC 5 %d\n%s" % (len(payload), payload)) + with self.assertRaises(NotFound): + c.where("connect_button") + + def test_unknown_name_raises_command_error(self): + payload = b"no such name in manifest: nosuchgadget" + c = client_with(b"RC 10 %d\n%s" % (len(payload), payload)) + with self.assertRaises(CommandError): + c.where("nosuchgadget") + + def test_malformed_reply_raises_command_error(self): + payload = b'malformed WHERE reply (expected "x y w h")' + c = client_with(b"RC 10 %d\n%s" % (len(payload), payload)) + with self.assertRaises(CommandError): + c.where("connect_button") + + def test_no_reply_raises_timeout(self): + payload = b"no reply from WHERE port within TIMEOUT" + c = client_with(b"RC 15 %d\n%s" % (len(payload), payload)) + with self.assertRaises(Timeout): + c.where("connect_button") + + def test_alloc_fail_raises_action_failed(self): + payload = b"could not allocate an ARexx message (out of memory)" + c = client_with(b"RC 20 %d\n%s" % (len(payload), payload)) + with self.assertRaises(ActionFailed): + c.where("connect_button") + + if __name__ == "__main__": unittest.main() diff --git a/manifest/README.md b/manifest/README.md index 23186ad..408e753 100644 --- a/manifest/README.md +++ b/manifest/README.md @@ -5,16 +5,19 @@ machine-readable manifest an application publishes mapping logical names (`connect_button`) to window/gadget IDs — tier 1 of AmiPilot's locator model, and the tier with zero runtime cost and zero fragility. -- **[`SPEC.md`](SPEC.md)** — the contract itself, format version 1: - file format, resolution semantics, naming/shipping conventions, and - the versioning policy. +- **[`SPEC.md`](SPEC.md)** — the contract itself, format versions 1 + and 2: file format, resolution semantics, naming/shipping + conventions, the cooperative geometry port (`WHEREPORT`/ + `WHEREGADGET`), and the versioning policy. - **Real examples** live with the fixtures they describe, exactly where a shipping application would put its own: [`fixtures/gadtools-app/GTApp.manifest`](../fixtures/gadtools-app/GTApp.manifest) - (the full shape) and + (the full version-1 shape) and [`fixtures/classact-app/CAApp.manifest`](../fixtures/classact-app/CAApp.manifest) - (the honest-limits shape: a window whose gadgets are structurally - unreachable names no gadgets). + (the version-2, cooperative-geometry-port shape: a window whose + gadgets are structurally unreachable, addressed via `WHEREGADGET` + instead of `GADGET` — see [SPEC.md's "The cooperative geometry + port"](SPEC.md#the-cooperative-geometry-port-where)). - **[Quirk profiles](SPEC.md#quirk-profiles-the-same-format-for-apps-you-dont-control)** — the same format, same parser, no new machinery, but authored by a user or the community for a third-party application rather than by diff --git a/manifest/SPEC.md b/manifest/SPEC.md index 1f1879e..5055a82 100644 --- a/manifest/SPEC.md +++ b/manifest/SPEC.md @@ -1,4 +1,4 @@ -# The AmiPilot manifest contract, format version 1 +# The AmiPilot manifest contract, format versions 1 and 2 A manifest is a small, machine-readable text file an application ships alongside its binary, mapping **stable logical names** to the window @@ -26,7 +26,7 @@ window. reviewable. - **Versioned from day one**: the first record declares the format version. Consumers reject versions they don't speak rather than - guessing. This file documents version **1**. + guessing. This file documents versions **1** and **2**. ## File format @@ -40,28 +40,44 @@ Record types, in the order they must appear: ``` MANIFEST APP +WHEREPORT WINDOW GADGET +WHEREGADGET ``` - **`MANIFEST`** — must be the first record. `` is a - positive integer; this spec is version `1`. A consumer that doesn't - speak the declared version must reject the whole file (with a clear - error), not skim it for records it recognises. + positive integer; this spec documents `1` and `2`. A consumer that + doesn't speak the declared version must reject the whole file (with a + clear error), not skim it for records it recognises. A version-`1` + file must not contain `WHEREPORT`/`WHEREGADGET` records — those + require version `2` (see "Versioning policy" below). - **`APP`** — the application's name, informational (error messages, tooling output). Exactly one. +- **`WHEREPORT`** (version 2 only) — declares the name of the ARexx + port this application exposes to answer `WHERE` queries (see "The + cooperative geometry port" below). At most one; if present, it must + appear before any `WHEREGADGET` record. Optional — a version-2 file + with no `WHEREPORT` simply has no `WHEREGADGET` entries either. - **`WINDOW`** — declares a logical window name and the title substring that locates it (the same first-match-wins substring matching AmiPilot's other locators use). At least one. - **`GADGET`** — declares a logical gadget name: which logical window it lives in, and its `GA_ID` there. The window must have been declared before it. +- **`WHEREGADGET`** (version 2 only, requires a `WHEREPORT` declared + above it) — declares a logical gadget name resolved not by `GA_ID` + but by querying the declared `WHEREPORT` at action time (see below). + No `GA_ID` field — there deliberately isn't one, since a + `WHEREGADGET` exists precisely for gadgets with no reachable one. Logical names are `[a-z0-9_]+` (lowercase by convention; consumers match them case-insensitively). Names must be unique within their kind -(no two windows with the same logical name; no two gadgets either — -gadget names are globally unique in the file, not per-window, so a -script can say `@connect_button` without qualifying the window). +(no two windows with the same logical name), and `GADGET`/`WHEREGADGET` +names share one namespace — no two gadget records of either kind may +share a name, gadget names are globally unique in the file, not +per-window, so a script can say `@connect_button` without qualifying +the window or caring which of the two record types resolved it. ## Example @@ -75,6 +91,20 @@ GADGET host_field main 2 GADGET enabled_checkbox main 3 ``` +A version-2 example, for an app whose gadgets sit behind a +`layout.gadget` wall (see "The cooperative geometry port" below): + +``` +; CAApp.manifest -- ships next to the CAApp binary. +MANIFEST 2 +APP CAApp +WHEREPORT CAAPP.WHERE +WINDOW main "AmiPilot ClassAct Fixture" +WHEREGADGET connect_button main +WHEREGADGET host_field main +WHEREGADGET enabled_checkbox main +``` + ## Resolution semantics Resolving a logical gadget name yields a `(title-substring, GA_ID)` @@ -85,14 +115,23 @@ never cached. The manifest pins the *identity* of a target; it says nothing about position, size, label, or ordering, which is exactly why relayout and relabelling can't break it. -What a manifest can name is bounded by what AmiPilot's walker can reach: -a gadget invisible to structural walking (e.g. a `layout.gadget` child -on classic OS 3.x — see the project's documented limits) can't be -clicked by `GA_ID` no matter what the manifest says. An application -whose scriptable gadgets sit behind that limit needs to restructure -(attach them where they're reachable) before a manifest helps — the -manifest format deliberately has no way to express "unreachable but -trust me", because that would be a lie waiting to be shipped. +What a plain `GADGET` record can name is bounded by what AmiPilot's +walker can reach: a gadget invisible to structural walking (e.g. a +`layout.gadget` child on classic OS 3.x — see the project's documented +limits) can't be clicked by `GA_ID` no matter what the manifest says. +Version 1 of this format deliberately had no way to express +"unreachable but trust me", because that would have been a lie waiting +to be shipped. Version 2's `WHEREGADGET` record is the honest answer to +that limit, not an exception to it: it doesn't ask AmiPilot to trust an +unreachable `GA_ID` — it names a gadget whose *geometry* the +application itself will report, live, on request. See "The cooperative +geometry port" below. + +Resolving a `WHEREGADGET` name yields `(title-substring, wherePort)` +instead of `(title-substring, GA_ID)`. The consumer locates the window +by title substring exactly as above, then queries `wherePort` for the +gadget's current geometry — again at action time, against the live +application, never cached. ## Naming and shipping conventions @@ -106,6 +145,95 @@ trust me", because that would be a lie waiting to be shipped. it; renaming a gadget's on-screen label is not a change at all. This asymmetry is the entire point. +## The cooperative geometry port (WHERE) + +A `window.class`/`layout.gadget` window attaches only its single +top-level layout object to `window->FirstGadget` — the layout's own +button/string/checkbox children aren't individually walkable, and +there's no public API to enumerate them on classic OS 3.x (see the +project's documented "Confirmed limit"). This blocks a plain `GADGET` +record for any such gadget, permanently — no manifest can fix an +enumeration limit. + +The escape hatch: the application itself already holds a live object +pointer to every gadget it created (it needs them for its own event +dispatch). It can expose a small, optional ARexx port that answers a +`WHERE ` query by calling +`GetAttr(GA_Left/GA_Top/GA_Width/GA_Height)` on its own object and +reporting the live, current geometry back. AmiPilot resolves a +`WHEREGADGET` name to that geometry and then acts with a genuine +`input.device` click at the resolved coordinates — discovery is +cooperative (the app tells AmiPilot where things are), but **actuation +stays real input through the real event path**, the same as every +other locator tier. No coordinates ever appear in a script; they are +resolved live, at action time, by the application itself, so relayout +and font changes can't break anything — the same immunity a plain +manifest already gives `GA_ID`-addressed gadgets, extended to the one +place structural walking can't reach. + +### Request + +A standard ARexx command message (`RXCOMM|RXFF_RESULT`, the same +message shape ARexx's own `ADDRESS` mechanism uses), with the command +string: + +``` +WHERE +``` + +`` matches case-insensitively and is otherwise passed +through verbatim (no quoting rules beyond ARexx's own). + +### Reply + +- **Success**: `rm_Result1` (the RC) is `0`; `rm_Result2` is an + argstring of exactly four whitespace-separated decimal integers, + `" "`, and nothing else. Units are pixels, relative to + the gadget's own window's top-left corner **including the window's + border and title bar** — i.e. exactly what + `GetAttr(GA_Left/GA_Top/GA_Width/GA_Height)` already returns; a + consumer converts to screen coordinates by adding the window's own + `LeftEdge`/`TopEdge` and nothing else (do not add `BorderLeft`/ + `BorderTop` — they're already folded in). +- **Unknown name**: `rm_Result1` is nonzero (`10` by convention, + matching this project's own RC scale); `rm_Result2` may optionally + carry a short reason string. +- The application must reply to every `WHERE` message it receives, from + the same task that owns the objects being queried (an ARexx message + handled off-task risks reading geometry mid-relayout). A sub-second + response is expected — AmiPilot's own default query timeout is 10 + seconds, generous enough for a busy app but not for a hung one. + +### Clash guard: pick a dedicated port name + +`WHERE` is an ordinary ARexx command string on whatever port the +manifest's `WHEREPORT` record names — nothing about this contract +reserves the word globally. If an application points `WHEREPORT` at a +general-purpose port that also implements its own command vocabulary, +a genuine collision (an existing `WHERE` command doing something else +entirely) is possible, and would silently misdirect every `WHEREGADGET` +click on that app. To make that risk structural rather than incidental: + +- **An application implementing this contract MUST make `WHERE` behave + exactly as specified above on the port its manifest names** — it may + not repurpose the word for something else on that same port. +- **Applications SHOULD expose a dedicated port for this purpose** + (e.g. `.WHERE`, matching the naming convention this spec's + own example manifest uses) rather than reusing a port that already + serves a broader, app-specific command set — the smaller the + vocabulary sharing that port, the smaller the chance any future + command it gains collides with this one. +- AmiPilot's own port resolution for `WHERE` queries matches the + declared port name **exactly** — it does not apply MUIREXX's own + `.1` fallback probe (a MUI-specific naming convention that + doesn't apply here), so a manifest that names the wrong port fails + loudly (port-not-found) rather than silently guessing at a related + one. Combined with the strict four-integer reply format above, a + reply that doesn't parse as expected is a hard error, not a silent + misclick — an accidental clash with an unrelated `WHERE` command on + the same port is far more likely to surface as an obvious rejected + reply than as a plausible-looking wrong coordinate. + ## Quirk profiles: the same format for apps you don't control Nothing about the `MANIFEST ` wire verb (`server/WIRE.md`, @@ -151,17 +279,23 @@ it changes in the same commit as the `GA_ID`s it describes and a third-party file can silently drift out of date. The "Resolution semantics" section's limit above applies here too, and matters more: a quirk profile can *record* a `GA_ID` for a gadget invisible to -structural walking, but recording it doesn't make it reachable — the -same "no way to express 'unreachable but trust me'" honesty applies -regardless of who wrote the file. +structural walking, but recording it doesn't make it reachable — a +plain `GADGET` record naming such a gadget is still a lie waiting to be +shipped, `WHEREGADGET` or not. A version-2 quirk profile can name a +`WHEREGADGET` only for an application that genuinely implements the +`WHERE` port itself; a third party cannot retrofit cooperative geometry +onto a binary that doesn't offer it. ## Versioning policy -- Format version bumps only for changes that would make a version-1 +- Format version bumps only for changes that would make an older parser misread a file (new record types, field-order changes). Adding - a new *optional* record type is still a version bump — version-1 - consumers must be able to trust that a file they accept contains - nothing they silently skipped. + a new *optional* record type is still a version bump — a version-1 + consumer must be able to trust that a file it accepts contains + nothing it silently skipped. Version 2 added `WHEREPORT`/ + `WHEREGADGET`; a version-1 file must not contain either record, and a + consumer that only speaks version 1 correctly rejects any file that + declares version 2. - This spec lives at `manifest/SPEC.md` in the AmiPilot repository and is versioned with it; released spec versions never change meaning after the fact. diff --git a/server/README.md b/server/README.md index 7874e65..da42700 100644 --- a/server/README.md +++ b/server/README.md @@ -865,6 +865,99 @@ Lands in phase 0.2 onward -- see forward correctly, not just in the single-screen case every prior on-target check has ever exercised. +- **Cooperative geometry port (issue #49, not yet released):** `WHERE @ [TIMEOUT=]` + is the diagnostic form of the escape hatch for gadgets structural + walking can never reach -- `layout.gadget` children on classic OS + 3.x (the project's own documented "Confirmed limit"; see + `docs/implementation-plan.md`'s "future tier between 1 and 2" design + note, now implemented). A manifest gadget declared with + `WHEREGADGET` (format version 2, `manifest/SPEC.md`) instead of + `GADGET` has no `GA_ID` at all; resolving its name queries the + manifest's declared `WHEREPORT` -- a small, optional ARexx port the + *application itself* exposes, answering `WHERE ` with its own + live `GetAttr(GA_Left/GA_Top/GA_Width/GA_Height)` geometry (window- + relative, including the border/title-bar area -- the exact + convention `AmipGadgetCenter()` already uses for ordinary gadgets, + `server/src/action.c`). `WHERE`'s own RESULT is the raw + `" "` text; it exists mainly as a test/debugging probe + -- `CLICK @name`/`TYPE @name` route through the identical query + automatically whenever the manifest resolves the name to a + `WHEREGADGET`, then act with a genuine `input.device` click via the + new `AmipClickWindowRelative()` (`server/src/action.c`), the same + real-input path every other locator tier uses. **Discovery is + cooperative; actuation stays real input** -- unlike `MUIREXX`, where + the target's own port does the acting too. No coordinates ever + appear in a script: they're resolved live, at action time, so + relayout and font changes can't break anything, the same immunity a + plain manifest already gives `GA_ID`-addressed gadgets. + + `GETTEXT`/`DRAG` have no `WHERE`-based path in this cut -- a + `WHEREGADGET` name given to either is `RC 10` with an explicit + "geometry only" message, a stated limit rather than a silent + fallback to nothing. `server/include/where.h`/`server/src/where.c` + hold the query primitive (`AmipWhereQuery()`); it deliberately does + **not** share `muirexx.c`'s `AmipMuiRexxSend()`, even though the + underlying `RexxMsg` send/poll mechanics are the same shape -- + `WHEREPORT` name resolution is exact-match only, with no `.1` + fallback the way `MUIREXX`'s MUI-specific port-slot convention gets + (`manifest/SPEC.md`'s "Clash guard" section explains why: a + `WHEREPORT` that doesn't exist under the exact declared spelling + must fail loudly, not silently probe a related name that happens to + exist for an unrelated reason). + + RC mapping: port not found → `RC 5`; the target's own reply RC + nonzero (unknown name) → `RC 10`; no reply within `TIMEOUT` → `RC + 15`; a reply that doesn't parse as exactly four decimal integers → + `RC 10` ("malformed WHERE reply", the *application's* WHERE + implementation being at fault, not this bridge); this server's own + `RexxMsg` allocation failing → `RC 20`. + + **Confirmed live** against `fixtures/classact-app`'s own new + `CAAPP.WHERE` port (`tests/copperline/where-test.py`, + `run_where_check` in `tests/copperline/run.sh`): all three of that + fixture's gadgets (button/string/checkbox, all `layout.gadget` + children, all invisible to the walker -- `CAApp.golden` is + unchanged, proving it) are addressed purely via `WHEREGADGET`; + `WHERE @connect_button` returns geometry that lands inside the + window's own reported bounds; an unknown name and a `GETTEXT` on a + `WHEREGADGET` both reject cleanly; `TYPE @host_field` genuinely + lands text in the layout child's own string gadget (confirmed via + the fixture's own log line, since `GETTEXT` can't read it back -- + see `where-test.py`'s header for why); and `CLICK @connect_button` + reaches and presses the real button, confirmed the same way every + other teardown check in this suite is -- `EXPECT=NOWINDOW` catching + the window actually closing. + + **A real bug found building this** (2026-08-09): a `RexxMsg` + constructed by hand via `CreateRexxMsg()`/`FillRexxMsg()`/`PutMsg()` + -- the same recipe `MUIREXX`'s own `AmipMuiRexxSend()` already uses + -- arrives at the receiver with its node type left at `NT_MESSAGE`, + not `NT_REPLYMSG`. `rexxsyslib.library`'s own `IsRexxMsg()` reports + such a message as not a `RexxMsg` at all, even though `rm_Action` + correctly carries `RXCOMM` and `ARG0()` reads back the real command + text -- confirmed by direct inspection (dumping `ln_Type`/ + `rm_Action` from the fixture's own receiving side) that a message + built exactly this way is structurally sound in every way + `IsRexxMsg()` doesn't check. A real ARexx interpreter's own + outgoing command messages (a genuine `rx` script's `ADDRESS`) + apparently arrive already marked `NT_REPLYMSG` through some internal + `rexxsyslib` mechanism this project's own hand-built sends don't + reproduce -- `IsRexxMsg()` seems intended for a *sender* validating + its own reply, not a receiver validating an incoming command, and + `server/src/arexx.c`'s own receiver-side use of it only ever works + because its senders are real ARexx scripts, never this project's own + bridges. Fixed by having `CAAPP.WHERE` -- a port dedicated solely to + this one protocol, per the "Clash guard" convention above -- trust + every message that arrives on it rather than gating on `IsRexxMsg()` + first; see the doc comment on `HandleWhereMessage()` in + `fixtures/classact-app/src/main.c` for the full account. + `MUIREXX`'s own `AmipMuiRexxSend()` has this same latent gap, masked + there only because MUI-Demo's built-in ARexx handling never calls + `IsRexxMsg()` on what it receives either -- not fixed here, since + MUIREXX genuinely works against every real target tried and isn't + this issue's scope, but worth knowing if a future MUIREXX target + ever does check. + ## Phase 0.2 (shipped) - `src/action.c` + `include/action_engine.h` -- the action engine's diff --git a/server/WIRE.md b/server/WIRE.md index 4167b79..ed4e913 100644 --- a/server/WIRE.md +++ b/server/WIRE.md @@ -151,7 +151,7 @@ A client's first command after opening the transport SHOULD be ``` AMIPILOT . PROTOCOL 1 -STABLE VERSION TREE CLICK TYPE GETTEXT MANIFEST LAUNCH FSLIST FSSTAT FSMKDIR FSDELETE FSGET FSPUT WBLAUNCH MENU MENUPICK DRAG WINDOWMOVE WINDOWSIZE WAITFOR SCREENS SCREENSHOT AUTH MUIREXX QUIT +STABLE VERSION TREE CLICK TYPE GETTEXT MANIFEST LAUNCH FSLIST FSSTAT FSMKDIR FSDELETE FSGET FSPUT WBLAUNCH MENU MENUPICK DRAG WINDOWMOVE WINDOWSIZE WAITFOR SCREENS SCREENSHOT AUTH MUIREXX WHERE QUIT ``` - Line 1: server version (from `version.mk`) and the wire protocol diff --git a/server/include/action_engine.h b/server/include/action_engine.h index 055936b..6d0b0df 100644 --- a/server/include/action_engine.h +++ b/server/include/action_engine.h @@ -79,6 +79,17 @@ BOOL AmipGadgetCenter(struct Window *window, struct Gadget *gadget, WORD *xOut, * target screen/window forward. */ BOOL AmipClickGadget(struct Window *window, struct Gadget *gadget); +/* The WHEREGADGET counterpart to AmipClickGadget() above (issue #49, + * manifest/SPEC.md's "The cooperative geometry port"): clicks the + * center of a window-relative rectangle (x, y, w, h -- the same + * convention AmipGadgetCenter() uses, including borders/title-bar; a + * WHERE port's own reply is already in this form) instead of a live + * struct Gadget*, after bringing the target screen/window forward. Used + * when a gadget is invisible to structural walking and its geometry + * came from the application's own cooperative ARexx port rather than a + * GA_Left/GA_Top read here. */ +BOOL AmipClickWindowRelative(struct Window *window, WORD x, WORD y, WORD w, WORD h); + /* --- drag: press, move, release ------------------------------------ */ /* The raw two-point drag primitive: moves to (x1, y1), presses the left diff --git a/server/include/arexx_cmd.h b/server/include/arexx_cmd.h index fcc9551..e1180f5 100644 --- a/server/include/arexx_cmd.h +++ b/server/include/arexx_cmd.h @@ -64,6 +64,22 @@ typedef enum { AMIP_AREXX_CMD_MUIREXX, /* MUIREXX [TIMEOUT=] -- * the MUI-ARexx bridge tier (phase 0.5); see * server/include/muirexx.h */ + AMIP_AREXX_CMD_WHERE, /* WHERE @ [TIMEOUT=] -- + * diagnostic form of the cooperative + * geometry port (issue #49): queries a + * WHEREGADGET manifest entry's declared + * WHEREPORT and returns the raw + * " " reply; manifest-only + * (no classic + * form -- there is nothing to resolve + * without a WHEREPORT). See + * server/include/where.h and + * manifest/SPEC.md's "WHERE port" section. + * CLICK/TYPE @ route through the + * same query automatically when the + * manifest resolves the name to a + * WHEREGADGET -- this verb exists only as + * a standalone diagnostic/test probe. */ AMIP_AREXX_CMD_WINDOWMOVE, /* WINDOWMOVE [SCREEN=] -- * moves a whole window by a real title-bar * drag; classic form only, no "@name" (a @@ -682,6 +698,18 @@ typedef struct { * nothing generic enough to build a CLICK/TYPE-shaped verb on top of * exists). * + * WHERE takes exactly "@" (manifest form only -- no classic + * alternative; there is no live gadget to + * name numerically here, only a manifest entry naming a WHEREPORT), then + * an optional trailing "TIMEOUT=" (seconds; default 10, same idiom as + * MUIREXX's own query timeout -- reuses expectTimeout). Returns the raw + * " " reply as its RESULT text. RC_ERROR if the name isn't + * in the manifest, or resolves to a plain GADGET rather than a + * WHEREGADGET (this verb is WHEREGADGET-only, by design -- CLICK/TYPE + * already handle a plain GADGET @name without going anywhere near a + * port); RC mapping for the port query itself is in + * server/include/where.h. + * * FSPUT takes (parsed exactly like FSLIST's own), then a * required token (decimal). A negative count, or one * beyond AMIP_AREXX_MAX_FSPUT_DRAIN, is rejected outright here at diff --git a/server/include/manifest.h b/server/include/manifest.h index b811720..9fd74f3 100644 --- a/server/include/manifest.h +++ b/server/include/manifest.h @@ -1,16 +1,19 @@ /* manifest.h -- portable parser for the AmiPilot manifest contract - * (manifest/SPEC.md, format version 1). Pure C, no Amiga types, no file - * I/O of its own (the caller reads the file and hands over the text) -- - * same portable-core split as arexx_cmd.h, and for the same reason: the - * host Python client's tests can cross-check against this parser's - * behavior, and nothing here needs an Amiga to run. */ + * (manifest/SPEC.md, format versions 1 and 2). Pure C, no Amiga types, + * no file I/O of its own (the caller reads the file and hands over the + * text) -- same portable-core split as arexx_cmd.h, and for the same + * reason: the host Python client's tests can cross-check against this + * parser's behavior, and nothing here needs an Amiga to run. */ #ifndef AMIPILOT_MANIFEST_H #define AMIPILOT_MANIFEST_H -/* The format version this parser speaks. A file declaring any other +/* Format versions this parser speaks. A file declaring any other * MANIFEST version is rejected whole, per the spec -- never skimmed for - * recognisable records. */ -#define AMIP_MANIFEST_FORMAT_VERSION 1 + * recognisable records. Version 2 adds WHEREPORT/WHEREGADGET (the + * cooperative geometry port, manifest/SPEC.md's "WHERE port" section); + * a version-1 file may not use them. */ +#define AMIP_MANIFEST_FORMAT_VERSION_MIN 1 +#define AMIP_MANIFEST_FORMAT_VERSION_MAX 2 /* Fixed caps: static allocation suits both the 68000 target and the * actual shape of real manifests (a handful of windows, dozens of @@ -20,6 +23,7 @@ #define AMIP_MANIFEST_MAX_NAME 32 /* logical names */ #define AMIP_MANIFEST_MAX_TITLE 96 /* window title substrings */ #define AMIP_MANIFEST_MAX_APPNAME 32 +#define AMIP_MANIFEST_MAX_PORT 32 /* WHEREPORT ARexx port name */ typedef struct { char name[AMIP_MANIFEST_MAX_NAME]; @@ -29,11 +33,15 @@ typedef struct { typedef struct { char name[AMIP_MANIFEST_MAX_NAME]; int windowIndex; /* into windows[] */ - long gadgetId; + long gadgetId; /* unused when viaWherePort is set */ + int viaWherePort; /* WHEREGADGET, not GADGET -- resolve geometry + * via wherePort at action time, not GA_ID */ } AmipManifestGadget; typedef struct { char appName[AMIP_MANIFEST_MAX_APPNAME]; + int declaredVersion; + char wherePort[AMIP_MANIFEST_MAX_PORT]; /* '\0' if no WHEREPORT */ int windowCount; int gadgetCount; AmipManifestWindow windows[AMIP_MANIFEST_MAX_WINDOWS]; @@ -48,10 +56,14 @@ typedef struct { int AmipManifestParse(const char *text, AmipManifest *out, char *errBuf, int errBufCap); -/* Resolves a logical gadget name (case-insensitive, per the spec) to its - * window title substring + GA_ID. Returns 0 and fills both out - * parameters on success; -1 if the name isn't in the manifest. */ +/* Resolves a logical gadget name (case-insensitive, per the spec). For + * a plain GADGET entry, fills outTitleSubstring/outGadgetId and clears + * *outViaWherePort. For a WHEREGADGET entry, fills outTitleSubstring + * and sets *outViaWherePort nonzero; outGadgetId is untouched (the + * caller must consult manifest->wherePort instead). Any out pointer may + * be NULL. Returns 0 on success; -1 if the name isn't in the manifest. */ int AmipManifestResolve(const AmipManifest *manifest, const char *gadgetName, - const char **outTitleSubstring, long *outGadgetId); + const char **outTitleSubstring, long *outGadgetId, + int *outViaWherePort); #endif /* AMIPILOT_MANIFEST_H */ diff --git a/server/include/where.h b/server/include/where.h new file mode 100644 index 0000000..810bb24 --- /dev/null +++ b/server/include/where.h @@ -0,0 +1,63 @@ +/* where.h -- the cooperative geometry port query (issue #49, + * docs/implementation-plan.md's "future tier between 1 and 2"). A + * WHEREGADGET manifest entry (manifest/SPEC.md) names an ARexx port an + * application exposes itself, answering "WHERE " with its + * own live GetAttr(GA_Left/GA_Top/GA_Width/GA_Height) geometry. This + * module sends that query and parses the reply; the caller (amipilot + * server's own HandleCommand()) turns the resulting geometry into a + * real input.device click via AmipClickWindowRelative() + * (action_engine.h). + * + * Deliberately NOT built on top of muirexx.c's AmipMuiRexxSend(): that + * function's port-name resolution tries the declared name verbatim, + * then falls back to ".1" -- a real MUI naming convention that + * has no place here (manifest/SPEC.md's "Clash guard" section). A + * WHEREPORT name that doesn't exist under the exact spelling the + * manifest declared must fail loudly (port-not-found), not silently + * probe a related name -- so this module resolves the port itself, + * separately, with no fallback, even though the actual RexxMsg + * send/poll/reply mechanics below are otherwise the same shape as + * muirexx.c's own (a second, deliberate copy -- same "separate copies + * on purpose" convention arexx_cmd.c/manifest.c's shared portable + * helpers already follow, here because the two are subtly different on + * purpose, not by oversight). */ +#ifndef AMIPILOT_WHERE_H +#define AMIPILOT_WHERE_H + +#include + +typedef enum { + AMIP_WHERE_OK = 0, /* sent, replied, app RC was 0 -- x/y/w/h filled in */ + AMIP_WHERE_APP_ERROR, /* sent, replied, app RC was nonzero -- the + * TARGET app's own rejection (usually + * "unknown name"), not a transport + * problem; appText filled in if the app + * gave a reason */ + AMIP_WHERE_PORT_NOT_FOUND, /* no ARexx port found under the exact + * declared name -- no ".1" fallback, see + * this header's own doc comment above */ + AMIP_WHERE_TIMEOUT, /* sent, but no reply within timeoutSeconds */ + AMIP_WHERE_BAD_REPLY, /* replied with RC 0, but rm_Result2 did + * not parse as exactly four decimal + * integers -- a malformed WHERE + * implementation, not a normal error */ + AMIP_WHERE_ALLOC_FAIL /* CreateMsgPort()/CreateRexxMsg()/ + * FillRexxMsg() failed (out of memory) */ +} AmipWhereResult; + +/* Sends "WHERE " to the ARexx port named `portName` (exact + * match only -- see doc comment above), polling for up to + * `timeoutSeconds` (0 = a 10s default, matching MUIREXX's own). On + * AMIP_WHERE_OK, *x, *y, *w, *h are the parsed window-relative geometry + * (manifest/SPEC.md's "The cooperative geometry port" section: pixels, + * including the window's own border/title bar -- add the window's + * LeftEdge/TopEdge, nothing else, to get screen coordinates). On + * AMIP_WHERE_APP_ERROR, appText (if non-NULL/nonzero-cap) is filled + * with whatever reason text the app gave (empty string if none). On + * every other outcome, none of the out parameters are touched. */ +AmipWhereResult AmipWhereQuery(const char *portName, const char *logicalName, + long timeoutSeconds, + long *x, long *y, long *w, long *h, + char *appText, size_t appTextCap); + +#endif /* AMIPILOT_WHERE_H */ diff --git a/server/src/action.c b/server/src/action.c index a948dcc..3c9b213 100644 --- a/server/src/action.c +++ b/server/src/action.c @@ -527,6 +527,27 @@ BOOL AmipClickGadget(struct Window *window, struct Gadget *gadget) return AmipClickAt(window->WScreen, centerX, centerY, AMIP_BUTTON_LEFT); } +BOOL AmipClickWindowRelative(struct Window *window, WORD x, WORD y, WORD w, WORD h) +{ + if (window == NULL) { + return FALSE; + } + + BringWindowForward(window); + + /* Same window-relative-including-borders convention as + * AmipGadgetCenter() above (see its comment) -- the geometry here + * comes from a WHERE port's own GetAttr(GA_Left/GA_Top/GA_Width/ + * GA_Height) reply (manifest/SPEC.md's "The cooperative geometry + * port" section) rather than a live struct Gadget*, but the + * conversion to a screen-relative click point is identical: add + * the window's own LeftEdge/TopEdge, nothing else. */ + return AmipClickAt(window->WScreen, + (WORD)(window->LeftEdge + x + w / 2), + (WORD)(window->TopEdge + y + h / 2), + AMIP_BUTTON_LEFT); +} + BOOL AmipDragAt(struct Screen *screen, WORD x1, WORD y1, WORD x2, WORD y2) { if (!AmipMoveMouseTo(screen, x1, y1)) { diff --git a/server/src/amipilotserver/main.c b/server/src/amipilotserver/main.c index b538325..e0b8ad8 100644 --- a/server/src/amipilotserver/main.c +++ b/server/src/amipilotserver/main.c @@ -54,6 +54,7 @@ #include "serial.h" #include "tcp.h" #include "wblaunch.h" +#include "where.h" #define STR(s) #s #define XSTR(s) STR(s) @@ -659,6 +660,112 @@ static char g_treeBuf[AMIP_TREE_BUF_SIZE]; #define AMIP_TCP_DEFAULT_PASSWORD "amipilot" static char g_tcpPassword[AMIP_TCP_PASSWORD_MAX] = AMIP_TCP_DEFAULT_PASSWORD; +/* Issue #49: handles a CLICK/TYPE/WHERE verb whose "@name" locator + * resolved to a WHEREGADGET (the manifest-resolution block ahead of + * HandleCommand()'s own verb switch already filled in + * cmd->windowPattern/screenPattern -- the window itself is still found + * structurally, only the gadget's geometry comes from the port) rather + * than a plain GADGET. Queries g_manifest.wherePort for cmd- + * >manifestName's current geometry (AmipWhereQuery(), where.h), then + * either reports it raw (WHERE) or acts on it with a real + * AmipClickWindowRelative() (CLICK/TYPE) -- the same "cooperative + * discovery, real actuation" split manifest/SPEC.md's own "The + * cooperative geometry port" section documents. Returns the + * AMIP_AREXX_RC_* code; on any non-OK/non-plain-click outcome the + * result text is left in g_resultBuf and *resultOut is pointed at it. */ +static int HandleWhereAction(AmipArexxParsed *cmd, const char **resultOut) +{ + struct Window *w; + long x = 0, y = 0, gw = 0, gh = 0; + long queryTimeout; + char appText[128]; + AmipWhereResult outcome; + + w = AmipFindWindow((CONST_STRPTR)cmd->screenPattern, (CONST_STRPTR)cmd->windowPattern); + if (w == NULL) { + return AMIP_AREXX_RC_WARN; + } + + /* Only the standalone WHERE verb takes an explicit TIMEOUT= for + * the port query itself; CLICK/TYPE reuse expectTimeout for their + * OWN, different purpose (CLICK's post-click EXPECT= wait), so + * they always use AmipWhereQuery's own 10s default here instead. */ + queryTimeout = (cmd->type == AMIP_AREXX_CMD_WHERE) ? cmd->expectTimeout : 0; + + outcome = AmipWhereQuery(g_manifest.wherePort, cmd->manifestName, queryTimeout, + &x, &y, &gw, &gh, appText, sizeof(appText)); + + switch (outcome) { + case AMIP_WHERE_OK: + break; + case AMIP_WHERE_APP_ERROR: + snprintf(g_resultBuf, sizeof(g_resultBuf), + "WHERE port: no such name%s%s", + appText[0] != '\0' ? ": " : "", appText); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_ERROR; + case AMIP_WHERE_PORT_NOT_FOUND: + strncpy(g_resultBuf, "WHERE port not found", sizeof(g_resultBuf) - 1); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_WARN; + case AMIP_WHERE_TIMEOUT: + strncpy(g_resultBuf, "no reply from WHERE port within TIMEOUT", + sizeof(g_resultBuf) - 1); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_TIMEOUT; + case AMIP_WHERE_BAD_REPLY: + snprintf(g_resultBuf, sizeof(g_resultBuf), + "malformed WHERE reply (expected \"x y w h\"), got: %s", appText); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_ERROR; + case AMIP_WHERE_ALLOC_FAIL: + default: + strncpy(g_resultBuf, "could not allocate an ARexx message (out of memory)", + sizeof(g_resultBuf) - 1); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_FAIL; + } + + /* The query can take real time (up to its own timeout) -- re-check + * the window is still open before acting on/reporting a possibly + * stale target, same recheck CLICK/TYPE's classic path performs + * right after ResolveTargetGadget(). */ + if (!AmipIsWindowOpen(w)) { + return AMIP_AREXX_RC_WARN; + } + + if (cmd->type == AMIP_AREXX_CMD_WHERE) { + snprintf(g_resultBuf, sizeof(g_resultBuf), "%ld %ld %ld %ld", x, y, gw, gh); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_OK; + } + + if (!AmipClickWindowRelative(w, (WORD)x, (WORD)y, (WORD)gw, (WORD)gh)) { + return AMIP_AREXX_RC_FAIL; + } + + if (cmd->type == AMIP_AREXX_CMD_TYPE) { + if (!AmipTypeString((CONST_STRPTR)cmd->text)) { + return AMIP_AREXX_RC_FAIL; + } + return AMIP_AREXX_RC_OK; + } + + /* CLICK: honor EXPECT= exactly like the classic path's own tail + * (the click already happened above regardless of EXPECT=). */ + if (cmd->expectMode == 1) { + if (!WaitForWindowPattern(NULL, (CONST_STRPTR)cmd->expectPattern, + cmd->expectTimeout, TRUE)) { + return AMIP_AREXX_RC_TIMEOUT; + } + } else if (cmd->expectMode == 2) { + if (!WaitForWindowClosed(w, cmd->expectTimeout)) { + return AMIP_AREXX_RC_TIMEOUT; + } + } + return AMIP_AREXX_RC_OK; +} + /* Executes one parsed command -- the single dispatch both transports * share (ARexx RESULT string and wire payload are the same bytes; see * server/WIRE.md). Returns the AMIP_AREXX_RC_* code, points *resultOut @@ -688,6 +795,10 @@ static int HandleCommand(AmipArexxParsed *cmd, const char **resultOut, { int rc = AMIP_AREXX_RC_OK; const char *result = NULL; + int viaWherePort = 0; /* set below when "@name" resolves to a + * WHEREGADGET (issue #49) -- consulted by + * CLICK/TYPE/WHERE's own cases in the verb + * switch further down */ *resultLenOut = 0; @@ -730,25 +841,62 @@ static int HandleCommand(AmipArexxParsed *cmd, const char **resultOut, * same windowPattern/gadgetId fields the classic form fills, so the * verb handlers below run identically for both. Unknown name / no * manifest loaded are both script errors (RC 10), same class as a - * bad argument. */ - if (cmd->manifestName[0] != '\0') { + * bad argument. + * + * A name resolving to a WHEREGADGET (viaWherePort set, issue #49) + * carries no GA_ID at all -- cmd->gadgetId is left untouched. + * CLICK/TYPE/WHERE route around ResolveTargetGadget() entirely for + * such a name (see HandleWhereAction() below, called from those + * verbs' own cases); GETTEXT/DRAG have no WHERE-based path in this + * cut and reject it here, immediately, as an honest stated limit + * rather than falling through into ResolveTargetGadget() with a + * meaningless zero gadgetId. */ + { const char *title; - long id; + long id = 0; - if (!g_manifestLoaded) { - strncpy(g_resultBuf, "no manifest loaded", sizeof(g_resultBuf) - 1); - *resultOut = g_resultBuf; - return AMIP_AREXX_RC_ERROR; - } - if (AmipManifestResolve(&g_manifest, cmd->manifestName, &title, &id) != 0) { - snprintf(g_resultBuf, sizeof(g_resultBuf), - "no such name in manifest: %s", cmd->manifestName); + if (cmd->manifestName[0] != '\0') { + if (!g_manifestLoaded) { + strncpy(g_resultBuf, "no manifest loaded", sizeof(g_resultBuf) - 1); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_ERROR; + } + if (AmipManifestResolve(&g_manifest, cmd->manifestName, &title, &id, + &viaWherePort) != 0) { + snprintf(g_resultBuf, sizeof(g_resultBuf), + "no such name in manifest: %s", cmd->manifestName); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_ERROR; + } + strncpy(cmd->windowPattern, title, sizeof(cmd->windowPattern) - 1); + cmd->windowPattern[sizeof(cmd->windowPattern) - 1] = '\0'; + cmd->gadgetId = id; + + if (viaWherePort && (cmd->type == AMIP_AREXX_CMD_GETTEXT + || cmd->type == AMIP_AREXX_CMD_DRAG)) { + snprintf(g_resultBuf, sizeof(g_resultBuf), + "@%s resolves via a WHERE port (geometry only) -- " + "%s cannot act on it", cmd->manifestName, + cmd->type == AMIP_AREXX_CMD_GETTEXT ? "GETTEXT" : "DRAG"); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_ERROR; + } + if (!viaWherePort && cmd->type == AMIP_AREXX_CMD_WHERE) { + snprintf(g_resultBuf, sizeof(g_resultBuf), + "@%s is a plain GADGET, not a WHEREGADGET -- " + "nothing to query", cmd->manifestName); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_ERROR; + } + } else if (cmd->type == AMIP_AREXX_CMD_WHERE) { + /* Parser guarantees manifestName is always set for WHERE + * (arexx_cmd.c rejects any non-"@name" form at parse time) + * -- this branch is unreachable, kept only so the switch + * below never sees WHERE with an empty windowPattern. */ + strncpy(g_resultBuf, "WHERE requires @", sizeof(g_resultBuf) - 1); *resultOut = g_resultBuf; return AMIP_AREXX_RC_ERROR; } - strncpy(cmd->windowPattern, title, sizeof(cmd->windowPattern) - 1); - cmd->windowPattern[sizeof(cmd->windowPattern) - 1] = '\0'; - cmd->gadgetId = id; } /* DRAG's SECOND locator ("TO @") -- same manifest @@ -756,22 +904,34 @@ static int HandleCommand(AmipArexxParsed *cmd, const char **resultOut, * dragToGadgetId is resolved against the SAME window as the * source (arexx_cmd.h's DRAG doc comment), so a destination whose * own manifest entry names a DIFFERENT window is a real, - * explicit error rather than a silent wrong-window drag. */ + * explicit error rather than a silent wrong-window drag. A + * destination resolving via a WHERE port has no GA_ID either, and + * DRAG has no WHERE-based path (see above) -- rejected the same + * way. */ if (cmd->type == AMIP_AREXX_CMD_DRAG && cmd->dragToManifestName[0] != '\0') { const char *destTitle; long destId; + int destViaWherePort = 0; if (!g_manifestLoaded) { strncpy(g_resultBuf, "no manifest loaded", sizeof(g_resultBuf) - 1); *resultOut = g_resultBuf; return AMIP_AREXX_RC_ERROR; } - if (AmipManifestResolve(&g_manifest, cmd->dragToManifestName, &destTitle, &destId) != 0) { + if (AmipManifestResolve(&g_manifest, cmd->dragToManifestName, &destTitle, + &destId, &destViaWherePort) != 0) { snprintf(g_resultBuf, sizeof(g_resultBuf), "no such name in manifest: %s", cmd->dragToManifestName); *resultOut = g_resultBuf; return AMIP_AREXX_RC_ERROR; } + if (destViaWherePort) { + snprintf(g_resultBuf, sizeof(g_resultBuf), + "drag destination @%s resolves via a WHERE port (geometry " + "only) -- DRAG cannot act on it", cmd->dragToManifestName); + *resultOut = g_resultBuf; + return AMIP_AREXX_RC_ERROR; + } if (strcmp((const char *)cmd->windowPattern, destTitle) != 0) { snprintf(g_resultBuf, sizeof(g_resultBuf), "drag destination @%s is in a different window (%s) than the source (%s)", @@ -782,6 +942,17 @@ static int HandleCommand(AmipArexxParsed *cmd, const char **resultOut, cmd->dragToGadgetId = destId; } + /* CLICK/TYPE @name resolving to a WHEREGADGET, and the standalone + * WHERE verb (always WHEREGADGET-only, enforced above), route + * around the verb switch below entirely -- ResolveTargetGadget() + * has nothing to walk to for these (issue #49). */ + if (cmd->type == AMIP_AREXX_CMD_WHERE + || ((cmd->type == AMIP_AREXX_CMD_CLICK || cmd->type == AMIP_AREXX_CMD_TYPE) + && viaWherePort)) { + rc = HandleWhereAction(cmd, &result); + goto done; + } + switch (cmd->type) { case AMIP_AREXX_CMD_MANIFEST: rc = LoadManifest(cmd->path, g_resultBuf, sizeof(g_resultBuf)); @@ -808,7 +979,7 @@ static int HandleCommand(AmipArexxParsed *cmd, const char **resultOut, "STABLE VERSION TREE CLICK TYPE GETTEXT MANIFEST LAUNCH " "FSLIST FSSTAT FSMKDIR FSDELETE FSGET FSPUT WBLAUNCH MENU " "MENUPICK DRAG WINDOWMOVE WINDOWSIZE WAITFOR SCREENS " - "SCREENSHOT AUTH MUIREXX QUIT\n"); + "SCREENSHOT AUTH MUIREXX WHERE QUIT\n"); result = g_resultBuf; break; @@ -1306,6 +1477,7 @@ static int HandleCommand(AmipArexxParsed *cmd, const char **resultOut, break; } +done: if (result != NULL && *resultLenOut == 0) { *resultLenOut = (ULONG)strlen(result); } diff --git a/server/src/arexx_cmd.c b/server/src/arexx_cmd.c index 26a527d..050a1c4 100644 --- a/server/src/arexx_cmd.c +++ b/server/src/arexx_cmd.c @@ -222,6 +222,7 @@ int AmipArexxParse(const char *cmdline, AmipArexxParsed *out) else if (ci_streq(kw, "WINDOWMOVE")) out->type = AMIP_AREXX_CMD_WINDOWMOVE; else if (ci_streq(kw, "WINDOWSIZE")) out->type = AMIP_AREXX_CMD_WINDOWSIZE; else if (ci_streq(kw, "MUIREXX")) out->type = AMIP_AREXX_CMD_MUIREXX; + else if (ci_streq(kw, "WHERE")) out->type = AMIP_AREXX_CMD_WHERE; else if (ci_streq(kw, "QUIT")) out->type = AMIP_AREXX_CMD_QUIT; else { out->type = AMIP_AREXX_CMD_UNKNOWN; return -1; } @@ -510,6 +511,36 @@ int AmipArexxParse(const char *cmdline, AmipArexxParsed *out) return 0; } + if (out->type == AMIP_AREXX_CMD_WHERE) { + int trunc; + + /* Manifest form only -- see arexx_cmd.h's doc comment on + * AmipArexxParse: there is no classic + * alternative for this verb. */ + p = skip_ws(p); + if (*p != '@') { + out->type = AMIP_AREXX_CMD_UNKNOWN; + return -1; + } + p++; + p = read_token(p, out->manifestName, sizeof(out->manifestName), &trunc); + if (fail_if_trunc(trunc, out)) return -1; + if (out->manifestName[0] == '\0') { + out->type = AMIP_AREXX_CMD_UNKNOWN; + return -1; + } + + p = skip_ws(p); + if (ci_streq_prefix(p, "TIMEOUT=")) { + char numbuf[16]; + p += 8; /* strlen("TIMEOUT=") */ + p = read_token(p, numbuf, sizeof(numbuf), &trunc); + if (fail_if_trunc(trunc, out)) return -1; + out->expectTimeout = strtol(numbuf, NULL, 10); + } + return 0; + } + if (out->type == AMIP_AREXX_CMD_WAITFOR) { int trunc; p = skip_ws(p); diff --git a/server/src/manifest.c b/server/src/manifest.c index 06f044d..18f9c88 100644 --- a/server/src/manifest.c +++ b/server/src/manifest.c @@ -115,16 +115,20 @@ int AmipManifestParse(const char *text, AmipManifest *out, if (ci_streq(kw, "MANIFEST")) { char ver[8]; + int verNum; p = skip_ws(p); p = read_field(p, ver, sizeof(ver)); if (sawVersion) { fail(errBuf, errBufCap, line, "duplicate MANIFEST record"); return -1; } - if (atoi(ver) != AMIP_MANIFEST_FORMAT_VERSION) { + verNum = atoi(ver); + if (verNum < AMIP_MANIFEST_FORMAT_VERSION_MIN + || verNum > AMIP_MANIFEST_FORMAT_VERSION_MAX) { fail(errBuf, errBufCap, line, "unsupported manifest format version"); return -1; } + out->declaredVersion = verNum; sawVersion = 1; } else if (!sawVersion) { /* Spec: MANIFEST must be the first record; anything else @@ -203,6 +207,60 @@ int AmipManifestParse(const char *text, AmipManifest *out, } g->windowIndex = winIndex; g->gadgetId = strtol(idField, NULL, 10); + g->viaWherePort = 0; + out->gadgetCount++; + } else if (ci_streq(kw, "WHEREPORT")) { + if (out->declaredVersion < 2) { + fail(errBuf, errBufCap, line, "WHEREPORT requires MANIFEST 2"); + return -1; + } + if (out->wherePort[0] != '\0') { + fail(errBuf, errBufCap, line, "duplicate WHEREPORT record"); + return -1; + } + p = skip_ws(p); + p = read_field(p, out->wherePort, sizeof(out->wherePort)); + if (out->wherePort[0] == '\0') { + fail(errBuf, errBufCap, line, "WHEREPORT needs a port name"); + return -1; + } + } else if (ci_streq(kw, "WHEREGADGET")) { + AmipManifestGadget *g; + char winName[AMIP_MANIFEST_MAX_NAME]; + int winIndex; + if (out->declaredVersion < 2) { + fail(errBuf, errBufCap, line, "WHEREGADGET requires MANIFEST 2"); + return -1; + } + if (out->wherePort[0] == '\0') { + fail(errBuf, errBufCap, line, "WHEREGADGET requires a WHEREPORT declared above it"); + return -1; + } + if (out->gadgetCount >= AMIP_MANIFEST_MAX_GADGETS) { + fail(errBuf, errBufCap, line, "too many GADGET/WHEREGADGET records"); + return -1; + } + g = &out->gadgets[out->gadgetCount]; + p = skip_ws(p); + p = read_field(p, g->name, sizeof(g->name)); + p = skip_ws(p); + p = read_field(p, winName, sizeof(winName)); + if (!valid_logical_name(g->name)) { + fail(errBuf, errBufCap, line, "WHEREGADGET logical name must be [a-z0-9_]+"); + return -1; + } + winIndex = find_window(out, winName); + if (winIndex < 0) { + fail(errBuf, errBufCap, line, "WHEREGADGET names a WINDOW not declared above it"); + return -1; + } + if (find_gadget(out, g->name) >= 0) { + fail(errBuf, errBufCap, line, "duplicate GADGET/WHEREGADGET logical name"); + return -1; + } + g->windowIndex = winIndex; + g->gadgetId = 0; + g->viaWherePort = 1; out->gadgetCount++; } else { /* Unknown record type: version-1 consumers must reject, not @@ -239,19 +297,25 @@ int AmipManifestParse(const char *text, AmipManifest *out, } int AmipManifestResolve(const AmipManifest *manifest, const char *gadgetName, - const char **outTitleSubstring, long *outGadgetId) + const char **outTitleSubstring, long *outGadgetId, + int *outViaWherePort) { int gi; + const AmipManifestGadget *g; if (manifest == NULL || gadgetName == NULL) return -1; gi = find_gadget(manifest, gadgetName); if (gi < 0) return -1; + g = &manifest->gadgets[gi]; if (outTitleSubstring != NULL) { - *outTitleSubstring = manifest->windows[manifest->gadgets[gi].windowIndex].titleSubstring; + *outTitleSubstring = manifest->windows[g->windowIndex].titleSubstring; + } + if (outViaWherePort != NULL) { + *outViaWherePort = g->viaWherePort; } - if (outGadgetId != NULL) { - *outGadgetId = manifest->gadgets[gi].gadgetId; + if (outGadgetId != NULL && !g->viaWherePort) { + *outGadgetId = g->gadgetId; } return 0; } diff --git a/server/src/where.c b/server/src/where.c new file mode 100644 index 0000000..4ce7bbc --- /dev/null +++ b/server/src/where.c @@ -0,0 +1,195 @@ +/* where.c -- see where.h. AmigaOS only (never built into a host tool), + * same split as muirexx.c/arexx.c. */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "where.h" + +/* Opened once by amipilotserver's main.c, shared across every file that + * talks to rexxsyslib.library -- arexx.c is the one owner; declared + * extern here, not redefined, same as muirexx.c's own. */ +extern struct RxsLib *RexxSysBase; + +#define AMIP_WHERE_POLL_TICKS 5 /* ~100ms, same granularity MUIREXX's own polling uses */ +#define AMIP_WHERE_DEFAULT_TIMEOUT 10 /* seconds, matching MUIREXX's own default */ + +/* Reads one decimal integer (optional leading '-', at least one digit) + * starting at *p, skipping any leading whitespace first; advances *p + * past it. Returns 0 (leaving *p unmoved past the whitespace skip) if + * no digit is found. Hand-rolled rather than sscanf("%ld%n", ...): + * libnix's own minimal sscanf does not honor "%n" (confirmed live -- + * it silently leaves the count untouched, which made an earlier + * version of this parser accept every reply as "malformed" since the + * untouched consumed-count of 0 made the trailing-content check see + * the whole string as junk) -- see the libnix skill's own notes on + * libnix's minimal stdio. */ +static int ReadLong(const char **p, long *out) +{ + const char *s = *p; + int neg = 0; + long value = 0; + int sawDigit = 0; + + while (*s == ' ' || *s == '\t') s++; + if (*s == '-') { + neg = 1; + s++; + } + while (*s >= '0' && *s <= '9') { + value = value * 10 + (*s - '0'); + s++; + sawDigit = 1; + } + if (!sawDigit) { + return 0; + } + *out = neg ? -value : value; + *p = s; + return 1; +} + +/* Strictly parses " " -- exactly four decimal integers, + * separated by whitespace, with nothing else (leading/trailing + * whitespace tolerated). A malformed reply is a bug in the target + * application's own WHERE implementation, not an ordinary error, so + * this deliberately does not try to salvage a partial parse. */ +static int ParseGeometry(const char *text, long *x, long *y, long *w, long *h) +{ + long vx, vy, vw, vh; + const char *p; + + if (text == NULL) return 0; + p = text; + if (!ReadLong(&p, &vx)) return 0; + if (!ReadLong(&p, &vy)) return 0; + if (!ReadLong(&p, &vw)) return 0; + if (!ReadLong(&p, &vh)) return 0; + while (*p == ' ' || *p == '\t') p++; + if (*p != '\0') return 0; + + *x = vx; *y = vy; *w = vw; *h = vh; + return 1; +} + +AmipWhereResult AmipWhereQuery(const char *portName, const char *logicalName, + long timeoutSeconds, + long *x, long *y, long *w, long *h, + char *appText, size_t appTextCap) +{ + struct MsgPort *replyPort; + struct MsgPort *target; + struct RexxMsg *msg; + struct RexxMsg *reply; + char command[64 + 32]; /* "WHERE " + a manifest logical name */ + ULONG ticksTotal; + ULONG ticksWaited = 0; + AmipWhereResult outcome; + + if (RexxSysBase == NULL) { + return AMIP_WHERE_ALLOC_FAIL; + } + + /* Exact match only -- no ".1" fallback (see where.h's own + * doc comment on why this deliberately does not share + * muirexx.c's FindTargetPort()). */ + Forbid(); + target = FindPort((CONST_STRPTR)portName); + Permit(); + if (target == NULL) { + return AMIP_WHERE_PORT_NOT_FOUND; + } + + replyPort = CreateMsgPort(); + if (replyPort == NULL) { + return AMIP_WHERE_ALLOC_FAIL; + } + + msg = CreateRexxMsg(replyPort, NULL, NULL); + if (msg == NULL) { + DeleteMsgPort(replyPort); + return AMIP_WHERE_ALLOC_FAIL; + } + + snprintf(command, sizeof(command), "WHERE %s", logicalName); + msg->rm_Args[0] = (STRPTR)command; + if (!FillRexxMsg(msg, 1, 0)) { + DeleteRexxMsg(msg); + DeleteMsgPort(replyPort); + return AMIP_WHERE_ALLOC_FAIL; + } + msg->rm_Action = RXCOMM | RXFF_RESULT; + + /* Re-resolve under the SAME Forbid() as the send -- closes the gap + * a quitting target could otherwise fall into, same reasoning as + * muirexx.c's own AmipMuiRexxSend(). */ + Forbid(); + target = FindPort((CONST_STRPTR)portName); + if (target != NULL) { + PutMsg(target, (struct Message *)msg); + } + Permit(); + + if (target == NULL) { + DeleteRexxMsg(msg); + DeleteMsgPort(replyPort); + return AMIP_WHERE_PORT_NOT_FOUND; + } + + ticksTotal = (ULONG)(timeoutSeconds > 0 ? timeoutSeconds : AMIP_WHERE_DEFAULT_TIMEOUT) * 50; + for (;;) { + reply = (struct RexxMsg *)GetMsg(replyPort); + if (reply != NULL) { + break; + } + if (ticksWaited >= ticksTotal) { + /* Deliberately leaked, same as muirexx.c's own timeout + * path -- the target may still own/reply to *msg, so + * deleting it out from under an in-flight send would be + * the real bug. */ + DeleteMsgPort(replyPort); + return AMIP_WHERE_TIMEOUT; + } + Delay(AMIP_WHERE_POLL_TICKS); + ticksWaited += AMIP_WHERE_POLL_TICKS; + } + + if (appText != NULL && appTextCap > 0) { + appText[0] = '\0'; + } + + if (reply->rm_Result1 == 0) { + const char *replyText = (reply->rm_Result2 != 0) + ? (const char *)reply->rm_Result2 : ""; + if (ParseGeometry(replyText, x, y, w, h)) { + outcome = AMIP_WHERE_OK; + } else { + outcome = AMIP_WHERE_BAD_REPLY; + if (appText != NULL && appTextCap > 0) { + strncpy(appText, replyText, appTextCap - 1); + appText[appTextCap - 1] = '\0'; + } + } + } else { + outcome = AMIP_WHERE_APP_ERROR; + if (appText != NULL && appTextCap > 0 && reply->rm_Result2 != 0) { + strncpy(appText, (const char *)reply->rm_Result2, appTextCap - 1); + appText[appTextCap - 1] = '\0'; + } + } + + if (reply->rm_Result2 != 0) { + DeleteArgstring((UBYTE *)reply->rm_Result2); + } + DeleteRexxMsg(reply); + DeleteMsgPort(replyPort); + + return outcome; +} diff --git a/tests/copperline/README.md b/tests/copperline/README.md index 4b6700f..dc408b2 100644 --- a/tests/copperline/README.md +++ b/tests/copperline/README.md @@ -218,6 +218,42 @@ manual Amiberry verification (`server/README.md`'s SCREENSHOT section): a known `x%4` pen-ramp pattern painted on a genuine P96 CLUT screen decodes back exactly. +### Cooperative geometry port / `WHERE` (issue #49) + +`run_where_check` drives `where-test.py` against `fixtures/classact-app`'s +new `CAAPP.WHERE` ARexx port — the first thing this fixture ever +exposed over ARexx at all, and the first end-to-end confirmation that +a manifest's `WHEREGADGET` records genuinely resolve to real clicks, +not just plausible-looking geometry. All three of the fixture's +gadgets (button/string/checkbox) are `layout.gadget` children, +permanently invisible to structural walking, so `CAApp.manifest` +(now format version 2) addresses every one of them this way instead +of the plain `GADGET` records it never had. + +**A real bug found running this live** (2026-08-09), not caught by +any amount of reading the NDK: a `RexxMsg` built by hand via +`CreateRexxMsg()`/`FillRexxMsg()`/`PutMsg()` — exactly the recipe +`server/src/muirexx.c`'s own `AmipMuiRexxSend()` already used +successfully against real MUI-Demo — arrives at the receiver with its +node type left at `NT_MESSAGE`, not `NT_REPLYMSG`. +`rexxsyslib.library`'s own `IsRexxMsg()` call reports such a message +as not a genuine `RexxMsg` at all, even though every other field +(`rm_Action`'s `RXCOMM` bit, `ARG0()`'s command text) is exactly +correct. This was invisible in the MUIREXX check because MUI-Demo's +own ARexx handling never calls `IsRexxMsg()` on what it receives — +only `CAAPP.WHERE`, written to match `server/src/arexx.c`'s own +receiver-side convention of gating on it, ever exposed the gap. +Root-caused by direct inspection: a debug build of the fixture dumped +`ln_Type`/`rm_Action` straight from the message it received, off a +port dedicated to nothing else, ruling out every "wrong message" or +"wrong port" theory before landing on the real one. Fixed by having +`CAAPP.WHERE` trust any message that arrives on its own dedicated +port rather than gating on `IsRexxMsg()` at all — see the doc comment +on `HandleWhereMessage()` in `fixtures/classact-app/src/main.c` for +the full account, and `server/README.md`'s own WHERE section for what +this means for `MUIREXX` (same latent gap, not fixed there since it +isn't this issue's scope and no real MUI target has ever tripped it). + ## Ad hoc smoke testing (debugging, new fixtures) For anything `run.sh` doesn't already assert on, write diff --git a/tests/copperline/run.sh b/tests/copperline/run.sh index f074ad5..9b7234f 100755 --- a/tests/copperline/run.sh +++ b/tests/copperline/run.sh @@ -1847,6 +1847,110 @@ EOF fi } +# --- WHERE / cooperative geometry port check (issue #49) ------------------ +# fixtures/classact-app's three gadgets are all layout.gadget children -- +# permanently unreachable by GA_ID (the project's documented "Confirmed +# limit") -- so CAApp.manifest (format version 2) names every one as a +# WHEREGADGET, resolved through the fixture's own CAAPP.WHERE ARexx port +# instead. Launched directly by this check's own smoke script (same +# pattern run_mui_check uses for MUI-Demo). CAApp writes its own +# host-readable log itself (build/caapp-log.txt, opened directly via +# dos.library, not via "Run >file" -- see fixtures/classact-app/src/ +# main.c's DiagFile() for why the latter didn't reliably work) -- the +# only external way to confirm TYPE @host_field's text genuinely landed +# in a gadget GETTEXT can't read back (see tests/copperline/where- +# test.py's own header for the full rationale). +run_where_check() { + echo "run.sh: cooperative geometry port (WHERE)" + + rm -f "$BUILD/where-result.txt" "$BUILD/marker-where-ready.txt" "$BUILD/caapp-log.txt" + cat > "$SMOKE_SCRIPT" <NIL: SRC:build/fixtures/CAApp +Wait 5 +Run >NIL: SRC:build/AmiPilotServer SERIAL +Wait 5 +Echo "READY" >SRC:build/marker-where-ready.txt +EOF + + info="$REPO_ROOT/build/.copperline-ctl-info-$$.json" + rm -f "$info" + "$COPPERLINE" --config "$CONFIG" --model A1200 --chipset AGA --cpu 68020 --chip 2M --accelerator 8M \ + --noaudio --serial tcp --control :0 --control-info "$info" \ + > "$REPO_ROOT/build/.copperline-log-$$.txt" 2>&1 & + COPPERLINE_PID=$! + + tries=0 + while [ ! -f "$info" ]; do + tries=$((tries + 1)) + if [ "$tries" -gt 100 ]; then + echo "run.sh: FAIL (where): copperline never wrote $info" + FAILED=1 + kill "$COPPERLINE_PID" 2>/dev/null || true + COPPERLINE_PID="" + return + fi + sleep 0.1 + done + + "$COPPERLINE_CTL" --info "$info" run_until '{"seconds": 600}' > /dev/null 2>&1 & + + tries=0 + while [ ! -f "$BUILD/marker-where-ready.txt" ]; do + tries=$((tries + 1)) + if [ "$tries" -gt 150 ]; then + echo "run.sh: FAIL (where): guest never became ready -- crash or hang" + FAILED=1 + kill "$COPPERLINE_PID" 2>/dev/null || true + COPPERLINE_PID="" + rm -f "$info" + return + fi + sleep 0.5 + done + + python3 "$REPO_ROOT/tests/copperline/where-test.py" 127.0.0.1:1234 \ + > "$BUILD/where-result.txt" 2>&1 || true + + # Give CAApp's own process a moment to actually exit and flush its + # redirected log after the click that closes its window -- the + # Python script's own CLICK-VIA-WHERE/WINDOW-GONE checks only prove + # the WINDOW closed, not that the guest process (and its Run-owned + # output redirection) has finished tearing down yet. + sleep 2 + + kill "$COPPERLINE_PID" 2>/dev/null || true + COPPERLINE_PID="" + rm -f "$info" + + ok=1 + for pattern in \ + 'WHERE-GEOMETRY PASS' \ + 'UNKNOWN-NAME PASS' \ + 'GETTEXT-LIMIT PASS' \ + 'TYPE-VIA-WHERE SENT' \ + 'CLICK-VIA-WHERE PASS' \ + 'WINDOW-GONE PASS'; do + if ! grep -qF "$pattern" "$BUILD/where-result.txt" 2>/dev/null; then + echo "run.sh: FAIL (where): expected line not found: $pattern" + ok=0 + fi + done + if ! grep -qF "caapp: host=AmigaTest" "$BUILD/caapp-log.txt" 2>/dev/null; then + echo "run.sh: FAIL (where): CAApp's own log never showed the typed text landing in host_field" + ok=0 + fi + + if [ "$ok" -eq 1 ]; then + echo "run.sh: PASS (cooperative geometry port)" + else + echo "run.sh: --- actual output ---" + sed 's/^/run.sh: /' "$BUILD/where-result.txt" 2>/dev/null || echo "run.sh: (empty)" + echo "run.sh: --- CAApp's own log ---" + sed 's/^/run.sh: /' "$BUILD/caapp-log.txt" 2>/dev/null || echo "run.sh: (empty)" + FAILED=1 + fi +} + # --- pytest release gate (phase 0.3, host/amipilot/pytest_plugin.py) ------ # The literal phase 0.3 release gate (docs/implementation-plan.md): "a # host pytest clicks a button and asserts a label changed, @@ -1910,6 +2014,7 @@ run_screens_check run_windowmoveresize_check run_requester_check run_golden_check +run_where_check run_pytest_release_gate_check if [ "$FAILED" -eq 0 ]; then @@ -1931,6 +2036,7 @@ if [ "$FAILED" -eq 0 ]; then "$BUILD"/menu-result.txt "$BUILD"/marker-menu-ready.txt \ "$BUILD"/screens-result.txt "$BUILD"/marker-screens-ready.txt \ "$BUILD"/golden-result.txt "$BUILD"/marker-golden-ready.txt \ + "$BUILD"/where-result.txt "$BUILD"/marker-where-ready.txt "$BUILD"/caapp-log.txt \ "$BUILD"/pytest-result.txt echo "run.sh: all fixtures PASS" exit 0 diff --git a/tests/copperline/where-test.py b/tests/copperline/where-test.py new file mode 100644 index 0000000..d4f7519 --- /dev/null +++ b/tests/copperline/where-test.py @@ -0,0 +1,110 @@ +#!/usr/bin/env python3 +"""Drives the cooperative geometry port (WHERE, issue #49) end to end +against fixtures/classact-app for the on-target regression check +(tests/copperline/run.sh) -- see manifest/SPEC.md's "The cooperative +geometry port" and server/include/where.h for the full design +rationale. + +Target: fixtures/classact-app's own CAAPP.WHERE ARexx port, launched +directly by run.sh's smoke script (same pattern run_mui_check uses for +MUI-Demo). All three of CAApp's gadgets (connect_button/host_field/ +enabled_checkbox) are children of a window.class window's +layout.gadget -- permanently invisible to structural walking (the +project's documented "Confirmed limit") -- so CAApp.manifest (format +version 2) names every one of them as a WHEREGADGET instead of a plain +GADGET. + +Confirms: WHERE @connect_button returns four plausible integers inside +the window's own TREE-reported bounds; an unknown manifest name and a +GETTEXT on a WHEREGADGET (a stated limit -- geometry only, no read-back) +both raise CommandError (RC 10); TYPE @host_field genuinely lands text +in the layout child's own string gadget (confirmed indirectly, via the +fixture's own "caapp: host=..." diagnostic log line -- GETTEXT can't +read this back, so this is the only external confirmation available); +and CLICK @connect_button really reaches and presses the real button, +confirmed the same way every other teardown check in this suite does: +CLICK's own EXPECT=NOWINDOW waiting for the window to close. + +Prints one greppable line per stage (run.sh asserts on them) and exits +non-zero on any failure. +""" + +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "..", "host")) + +from amipilot import Amipilot, CommandError, NotFound # noqa: E402 +from amipilot.wire import WireError # noqa: E402 + +WINDOW = "AmiPilot ClassAct Fixture" +MANIFEST_PATH = "SRC:fixtures/classact-app/CAApp.manifest" +TYPED_TEXT = "AmigaTest" + + +def main() -> int: + hostport = sys.argv[1] if len(sys.argv) > 1 else "127.0.0.1:1234" + host, port = hostport.rsplit(":", 1) + + client = Amipilot.connect_with_retry(host, int(port), deadline_seconds=30, + connect_timeout=20) + print(f"HANDSHAKE SERVER={client.info.server_version} " + f"PROTOCOL={client.info.protocol}") + + report = client.manifest(MANIFEST_PATH) + print(f"MANIFEST-LOADED {report}") + + window = client.tree(WINDOW) + win_w, win_h = window.width, window.height + + x, y, w, h = client.where("connect_button") + print(f"WHERE-GEOMETRY RESULT={x} {y} {w} {h}") + if not (0 <= x < win_w and 0 <= y < win_h and w > 0 and h > 0 + and x + w <= win_w and y + h <= win_h): + print("WHERE-GEOMETRY FAIL (geometry outside window bounds " + f"{win_w}x{win_h})") + return 1 + print("WHERE-GEOMETRY PASS") + + try: + client.where("nosuchgadget") + print("UNKNOWN-NAME FAIL") + return 1 + except CommandError: + print("UNKNOWN-NAME PASS") + + try: + client.get_text_by_name("host_field") + print("GETTEXT-LIMIT FAIL") + return 1 + except CommandError: + print("GETTEXT-LIMIT PASS") + + client.type_by_name("host_field", TYPED_TEXT) + print("TYPE-VIA-WHERE SENT") + + try: + client.click_by_name("connect_button", expect="nowindow", timeout=15) + print("CLICK-VIA-WHERE PASS") + except Exception as e: # noqa: BLE001 -- report whatever went wrong, then fail + print(f"CLICK-VIA-WHERE FAIL {e}") + return 1 + + try: + client.tree(WINDOW) + print("WINDOW-GONE FAIL") + return 1 + except NotFound: + print("WINDOW-GONE PASS") + + client.quit() + client.close() + return 0 + + +if __name__ == "__main__": + try: + sys.exit(main()) + except WireError as e: + print(f"WHERE-TEST ERROR: {e}") + sys.exit(1) diff --git a/userdocs/ARexx-Reference.md b/userdocs/ARexx-Reference.md index 2218d2d..3ab2a9a 100644 --- a/userdocs/ARexx-Reference.md +++ b/userdocs/ARexx-Reference.md @@ -60,6 +60,7 @@ for disambiguating two same-titled windows on different screens; see | `WAITFOR` | `[SCREEN=] WINDOW= [TIMEOUT=]` or `[SCREEN=] NOWINDOW= [TIMEOUT=]` or `[SCREEN=] ( \| ROLE= [LABEL=] [INDEX=]) TEXT= [TIMEOUT=]` or `@ TEXT= [TIMEOUT=]` or `[SCREEN=] REQUESTER [TIMEOUT=]` | Polls (server-side, one round trip) until a window matching `` appears, until none does, until a gadget's text exactly equals ``, or until a genuine Intuition Requester appears (window-attached only — issue #52's detection-only slice). `TIMEOUT` defaults to 10 seconds. `RC=15` if the condition never becomes true in time — a new RC distinct from "nothing matched" (`RC=5`) or "the action itself failed" (`RC=20`). See [Wait/expectation primitives](#waitexpectation-primitives). | | `AUTH` | `` | Authenticates a TCP connection (no effect on ARexx or serial.device, which have their own implicit trust boundaries). Until it succeeds, the TCP transport refuses every command except `VERSION`/`AUTH`/`QUIT` with `RC=10`. See [Securing TCP](Wire-Protocol.md#securing-tcp). | | `MUIREXX` | ` [TIMEOUT=] ` | Sends `` verbatim to a MUI application's own ARexx port. The MUI-ARexx bridge tier — see [Driving MUI applications](#driving-mui-applications). | +| `WHERE` | `@ [TIMEOUT=]` | Diagnostic query of the cooperative geometry port: returns ``'s current `" "` geometry from the application's own declared `WHEREPORT`. Manifest-only, `WHEREGADGET` names only — see [Driving layout.gadget-only applications](#driving-layoutgadget-only-applications). | | `QUIT` | (none) | Shuts the commodity down cleanly. | The same command set is also reachable from a host machine over @@ -89,6 +90,14 @@ touch anything, and then only the manifest. Using `@name` with no manifest loaded, or with a name the manifest doesn't define, is `RC=10` with the reason in `RESULT`. +A format-version-2 manifest can also declare a name via `WHEREGADGET` +instead of `GADGET` — for a gadget structural walking can never reach +at all (a `layout.gadget` child on classic OS 3.x). `CLICK @name`/ +`TYPE @name` work exactly the same either way; see [Driving +layout.gadget-only applications](#driving-layoutgadget-only-applications) +for what's different underneath and `GETTEXT`/`DRAG`'s own honest +limit against such a name. + ## Tier-2 semantic locators For an application with no manifest, `CLICK`/`TYPE`/`GETTEXT` also @@ -262,6 +271,61 @@ replied within `TIMEOUT` (default 10 seconds); `RC=20` only if `AmiPilotServer` itself couldn't allocate the ARexx message (its own resource problem, not the target's). +## Driving layout.gadget-only applications + +A `window.class` window attaches only its single top-level layout +object to `window->FirstGadget` — its own button/string/checkbox +children aren't individually walkable, and classic OS 3.x has no +public API to enumerate them (see [Locator Tiers and +Limits](Locator-Tiers-and-Limits.md)). A plain `GADGET` manifest entry +can't name such a gadget; a `WHEREGADGET` entry can, if the +application itself cooperates. + +An application implementing this (issue #49) exposes a small, +optional ARexx port answering `WHERE ` with that gadget's own +live geometry — it already holds the object pointer for its own event +dispatch, so it just reads `GetAttr(GA_Left/GA_Top/GA_Width/ +GA_Height)` and reports back. Its manifest declares the port's name +with `WHEREPORT`: + +``` +MANIFEST 2 +APP MyApp +WHEREPORT MYAPP.WHERE +WINDOW main "My App" +WHEREGADGET connect_button main +``` + +Once that manifest is loaded, `CLICK @connect_button`/ +`TYPE @connect_button ...` work exactly like they would for a plain +`GADGET` name — AmiPilot queries the port for the gadget's current +geometry, then clicks it with a genuine `input.device` event. **Only +discovery is cooperative; the click is real input**, unlike `MUIREXX` +above, where the target's own port does the acting too. No coordinate +ever appears in the script — it's resolved live, at action time, so +relayout and font changes can't break anything. + +`WHERE @ [TIMEOUT=]` is the standalone diagnostic form — +useful for confirming a third-party port answers correctly, or for +asserting on geometry directly: + +```rexx +'WHERE @connect_button' +SAY 'RESULT is "x y w h": 'RESULT +``` + +`GETTEXT`/`DRAG` have no path through a `WHEREPORT` — a `WHEREGADGET` +name given to either is `RC=10` with an explicit "geometry only" +message, an honest stated limit rather than a silent fallback. +`RC=5` if the declared `WHEREPORT` doesn't exist (checked by its +*exact* declared name — no `.1`-slot fallback the way `MUIREXX`'s +MUI-specific convention gets); `RC=10` if the port itself reports the +name unknown, or its reply doesn't parse as exactly four integers; +`RC=15` if it never replies within `TIMEOUT` (default 10 seconds). +Full contract for application authors: [`manifest/SPEC.md`'s "The +cooperative geometry port" +section](https://github.com/sidick/amipilot/blob/main/manifest/SPEC.md). + ## Example ```rexx diff --git a/userdocs/Changelog.md b/userdocs/Changelog.md index 46388c3..5cee054 100644 --- a/userdocs/Changelog.md +++ b/userdocs/Changelog.md @@ -7,6 +7,25 @@ the repository's [`docs/implementation-plan.md`](https://github.com/sidick/amipilot/blob/main/docs/implementation-plan.md) for the full engineering detail and phase sequencing behind each one. +## Unreleased + +- **`WHERE`, the cooperative geometry port** (issue #49): the honest + escape hatch for gadgets nested inside a `window.class` window's + `layout.gadget` — permanently invisible to structural walking on + classic AmigaOS 3.x, so no plain manifest entry could ever name + them. An application implementing this exposes a small, optional + ARexx port answering `WHERE ` with a gadget's own live + geometry (it already holds the object pointer for its own event + dispatch); a format-version-2 manifest names such a gadget with + `WHEREGADGET` instead of `GADGET`. `CLICK`/`TYPE @name` then work + exactly as they would for any other manifest name — discovery is + cooperative, but the click itself is still genuine `input.device` + input, unlike `MUIREXX`, where the target's own port does the + acting too. Verified end to end against `fixtures/classact-app`'s + own new `CAAPP.WHERE` port, whose three gadgets are now addressed + entirely via `WHEREGADGET` (its manifest previously, deliberately, + named none at all). See [ARexx Reference](ARexx-Reference.md#driving-layoutgadget-only-applications). + ## v1.0 — 2026-08-09 The first full release: everything the implementation plan's 1.0 gate diff --git a/userdocs/Locator-Tiers-and-Limits.md b/userdocs/Locator-Tiers-and-Limits.md index a668ddf..e6597ab 100644 --- a/userdocs/Locator-Tiers-and-Limits.md +++ b/userdocs/Locator-Tiers-and-Limits.md @@ -14,13 +14,18 @@ defines four locator tiers: **1** (manifest logical names — or a community-authored [quirk profile](https://github.com/sidick/amipilot/blob/main/manifest/SPEC.md#quirk-profiles-the-same-format-for-apps-you-dont-control), same format), **2** (semantic — window pattern + role/`GA_ID`), **3** (the MUI-ARexx bridge), and **4** (raw coordinates, the fragile floor). +Between 1 and 2 sits a fifth, cooperative variant of tier 1 (issue +#49): a `WHEREGADGET` manifest entry, resolved by querying an ARexx +port the *target application itself* exposes for a gadget's live +geometry, rather than a `GA_ID` — see [ARexx Reference](ARexx-Reference.md#driving-layoutgadget-only-applications). This table is the honest answer to "which tier actually reaches my application's UI today": | UI built from | Reaches | Why | | --- | --- | --- | | Plain GadTools gadgets (button/checkbox/string/integer/slider), top-level `BOOPSI`/ReAction gadgets attached directly to a window (`button.gadget`, `string.gadget`, etc., not nested inside a layout) | **Tier 1** with a manifest/quirk profile, or **Tier 2** without one | Fully classified and structurally reachable — see "What's classified today" below. | -| A `window.class` + `layout.gadget` window's nested button/string/checkbox children | **Tier 4 only** (raw coordinates) | The confirmed `layout.gadget` limit below — invisible to structural walking, so tiers 1–3 can't name them at all, not even via a quirk profile (there's no `GA_ID` to record). The design doc's "cooperative geometry port" (a design note, not yet scheduled) is the intended fix. | +| A `window.class` + `layout.gadget` window's nested button/string/checkbox children, where the application implements a `WHERE` port | **Tier 1 (cooperative)** — a `WHEREGADGET` manifest entry | The confirmed `layout.gadget` limit below — invisible to structural walking, so plain tiers 1–3 can't name them at all (not even via a quirk profile, since there's no `GA_ID` to record). The application's own cooperation closes the gap: it reports its own gadgets' geometry live, and AmiPilot still does the actual clicking with real `input.device` input. | +| The same nested children, where the application does **not** implement a `WHERE` port | **Tier 4 only** (raw coordinates) | Nothing changes without the target's own cooperation — a third party can't retrofit a `WHERE` port onto a binary that doesn't offer one, the same way a quirk profile can't retrofit a `GA_ID` onto an unreachable gadget. | | MUI applications | **Tier 3** (`MUIREXX`) for whatever the app's own ARexx commands cover, **Tier 4** for anything else | `intuition-model`'s class-name walker has no MUI recognition, so Tier 2 doesn't reach MUI gadgets at all — confirmed live that a MUI window attaches only its own single top-level object to `window->FirstGadget`, the same limit `window.class`/`layout.gadget` has. Tier 3 (see [ARexx Reference](ARexx-Reference.md#driving-mui-applications)) drives through the ARexx port every MUI app carries automatically, but MUI's own *built-in* support there is a small, universal set (`quit`/`hide`/`show`/`activate`/`deactivate`/`info`/`help`) — confirmed against AmigaOS 3.2's own MUI-Demo, which registers zero commands beyond it. Genuine per-widget interaction needs the target application to have added its own ARexx commands; `MUIREXX` passes them through, it can't invent them. | | Custom-rendered UIs (games, hand-rolled bitmap rendering) | **Tier 4 only** (raw coordinates) | Out of scope by definition — nothing structural exists for a walker to find, and there's no ARexx port to assume. | @@ -86,6 +91,20 @@ invisible to structural walking regardless of who wrote the file down (see the [quirk profiles section](https://github.com/sidick/amipilot/blob/main/manifest/SPEC.md#quirk-profiles-the-same-format-for-apps-you-dont-control) of the manifest spec). +This particular limit now has a real escape hatch, though only for +applications that opt into it: a `WHEREGADGET` manifest entry +(format version 2) resolves such a gadget by querying a small, +optional ARexx port the application itself exposes, reporting its own +live geometry — see [Driving layout.gadget-only +applications](ARexx-Reference.md#driving-layoutgadget-only-applications). +It doesn't change what `AmiInspect`/structural walking can *see* (a +`WHEREGADGET`-addressed gadget still never appears in a `TREE`/ +`AmiInspect` dump — this is a discovery mechanism cooperating with the +application, not a walker enhancement), only what `CLICK`/`TYPE` can +*act on*. A third party still can't retrofit this onto a binary that +doesn't implement the port, the same honest boundary a quirk profile +already has. + **Custom-rendered UIs are invisible.** Anything an application draws directly into a bitmap rather than building from real gadget structures has nothing for a structural walker to find. This applies to games and diff --git a/userdocs/Wire-Protocol.md b/userdocs/Wire-Protocol.md index ff4764f..d93bfcd 100644 --- a/userdocs/Wire-Protocol.md +++ b/userdocs/Wire-Protocol.md @@ -110,7 +110,7 @@ $ nc 127.0.0.1 1234 VERSION RC 0 217 AMIPILOT 1.0 PROTOCOL 1 -STABLE VERSION TREE CLICK TYPE GETTEXT MANIFEST LAUNCH FSLIST FSSTAT FSMKDIR FSDELETE FSGET FSPUT WBLAUNCH MENU MENUPICK DRAG WINDOWMOVE WINDOWSIZE WAITFOR SCREENS SCREENSHOT AUTH MUIREXX QUIT +STABLE VERSION TREE CLICK TYPE GETTEXT MANIFEST LAUNCH FSLIST FSSTAT FSMKDIR FSDELETE FSGET FSPUT WBLAUNCH MENU MENUPICK DRAG WINDOWMOVE WINDOWSIZE WAITFOR SCREENS SCREENSHOT AUTH MUIREXX WHERE QUIT GETTEXT GadTools 2 RC 0 10 aminet.net From e0a5825bc6800b6a4ec505be8f0792e5ef2756e2 Mon Sep 17 00:00:00 2001 From: Simon Dick Date: Sun, 9 Aug 2026 12:08:38 +0100 Subject: [PATCH 2/3] Record that IsRexxMsg() genuinely can't be satisfied by a hand-built RexxMsg send Investigated whether the WHERE port's own receiver could stay a fully standard, unmodified ARexx port (IsRexxMsg()-gated, no special knowledge required) by fixing the sender instead -- tried both the commonly-cited ln_Type = NT_REPLYMSG pre-marking and a genuine CreateArgstring()-backed rm_Args[0] in AmipWhereQuery(). Neither changed the outcome: PutMsg() itself resets the node type regardless of what's set beforehand, so a message built via CreateRexxMsg()/ FillRexxMsg()/PutMsg() from ordinary C code cannot be made indistinguishable from a live ARexx interpreter's own send by any public API tried. No functional change (where.c is back to its prior form) -- just recording the investigation so this isn't re-attempted, and being explicit that any third-party WHERE implementation (e.g. AmiAuth) needs a receive loop that doesn't call IsRexxMsg(), the same as CAAPP.WHERE's own. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018CbPo4nMpnM5JQX3LKE5JP --- fixtures/classact-app/src/main.c | 56 ++++++++++++++++++++------------ server/include/where.h | 11 ++++++- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/fixtures/classact-app/src/main.c b/fixtures/classact-app/src/main.c index c62fd5b..0565d4b 100644 --- a/fixtures/classact-app/src/main.c +++ b/fixtures/classact-app/src/main.c @@ -209,27 +209,36 @@ static void ReplyWhere(struct RexxMsg *msg, int rc, const char *text) * on the AmiPilot side (server/src/action.c). * * Deliberately does NOT gate on rexxsyslib.library's own IsRexxMsg() - * first, unlike server/src/arexx.c's own receiver -- confirmed - * empirically (2026-08-09) that a message built by hand via + * first, unlike server/src/arexx.c's own receiver. Two rounds of + * experimentation (2026-08-09) against this exact port, with a + * completely ordinary, unmodified IsRexxMsg() gate restored each + * time, ruled out every sender-side fix tried: neither pre-marking + * the outgoing message's own node type NT_REPLYMSG (the commonly + * cited technique for a hand-built ARexx command send) nor using a + * genuine CreateArgstring()-allocated rm_Args[0] (instead of a raw C + * string pointer) made IsRexxMsg() accept a message built via * CreateRexxMsg()/FillRexxMsg()/PutMsg() (server/src/where.c's own - * AmipWhereQuery(), the same recipe server/src/muirexx.c's - * AmipMuiRexxSend() already uses) arrives with ln_Type left at - * NT_MESSAGE, not NT_REPLYMSG -- IsRexxMsg() reports such a message as - * NOT a RexxMsg even though it demonstrably is one (RXCOMM correctly - * set in rm_Action, ARG0() reads back the real command text). Real - * ARexx-interpreter-originated messages (a genuine `rx` script's own - * ADDRESS) apparently arrive already marked NT_REPLYMSG through some - * internal rexxsyslib mechanism this project's own hand-built sends - * don't reproduce -- IsRexxMsg() is seemingly meant for a SENDER - * validating its own reply, not a receiver validating an incoming - * command, and arexx.c's receiver-side use of it happens to work only - * because its senders are always real ARexx scripts, never this - * project's own MUIREXX/WHERE bridges. CAAPP.WHERE is a port - * dedicated solely to this one protocol (manifest/SPEC.md's own - * "Clash guard" -- a general-purpose ARexx port sharing this same - * port would need a real validity check here instead), so trusting - * every message that arrives on it is the correct, not merely - * expedient, choice. */ + * AmipWhereQuery() -- the same recipe server/src/muirexx.c's + * AmipMuiRexxSend() already uses). The receiver's own ln_Type read + * stayed NT_MESSAGE regardless of what the sender set beforehand -- + * consistent with PutMsg() itself resetting it, a real Exec message- + * queueing behavior a real ARexx interpreter's own outgoing command + * sends must go through some other, non-public mechanism to avoid. + * IsRexxMsg() (a real rexxsyslib.library call, not just a header- + * field check) is seemingly reachable in its "true" state only for + * messages a live Rexx interpreter task itself constructs -- not + * achievable for this project's own hand-built sends by any public + * API combination tried. arexx.c's own receiver-side use of + * IsRexxMsg() only ever works because its senders are real ARexx + * scripts (`rx`), never this project's own MUIREXX/WHERE bridges. + * CAAPP.WHERE is a port dedicated solely to this one protocol + * (manifest/SPEC.md's own "Clash guard" -- a general-purpose port + * sharing this same MsgPort would need a real discriminator here + * instead), so trusting every message that arrives on it is the + * correct, verified choice -- not a shortcut. Any third-party + * application implementing WHERE needs the same: a receive loop that + * does NOT call IsRexxMsg() on what it gets, on a port used for + * nothing else. */ static void HandleWhereMessage(struct RexxMsg *msg) { const char *cmdline = (const char *)ARG0(msg); @@ -359,6 +368,13 @@ static void ProcessEvents(Object *windowObject, struct MsgPort *wherePort) signals = Wait(windowSignal | whereSignal); if (whereSignal != 0 && (signals & whereSignal) != 0) { + /* No IsRexxMsg() gate here -- see HandleWhereMessage()'s + * own doc comment for why not, and why that's the + * correct choice for a port dedicated solely to this one + * protocol, not a shortcut. Every message on this port + * must still be replied by its receiver, or the sender + * leaks/hangs waiting for a reply that never comes -- + * HandleWhereMessage() always does, on every path. */ struct RexxMsg *msg; while ((msg = (struct RexxMsg *)GetMsg(wherePort)) != NULL) { HandleWhereMessage(msg); diff --git a/server/include/where.h b/server/include/where.h index 810bb24..02f1c86 100644 --- a/server/include/where.h +++ b/server/include/where.h @@ -20,7 +20,16 @@ * muirexx.c's own (a second, deliberate copy -- same "separate copies * on purpose" convention arexx_cmd.c/manifest.c's shared portable * helpers already follow, here because the two are subtly different on - * purpose, not by oversight). */ + * purpose, not by oversight). + * + * A receiver implementing WHERE must NOT gate incoming messages on + * rexxsyslib.library's own IsRexxMsg() -- confirmed experimentally + * (2026-08-09, including two separate attempted sender-side fixes, + * neither of which changed the outcome) that a message built here via + * CreateRexxMsg()/FillRexxMsg()/PutMsg() never satisfies it, for + * reasons this module's own send code can't control. See the doc + * comment on fixtures/classact-app/src/main.c's HandleWhereMessage() + * for the full investigation and what a receiver should do instead. */ #ifndef AMIPILOT_WHERE_H #define AMIPILOT_WHERE_H From 9f1914d1258e4650ee16575026ef858c4a87794e Mon Sep 17 00:00:00 2001 From: Simon Dick Date: Sun, 9 Aug 2026 12:18:37 +0100 Subject: [PATCH 3/3] Fix the real root cause: validate incoming WHERE messages via RXCOMM, not IsRexxMsg() The previous commit's "trust every message on a dedicated port" workaround was correct in that it made the tests pass, but not in its diagnosis. IsRexxMsg() checks ln_Type == NT_REPLYMSG, which ReplyMsg() sets -- the right question for a sender inspecting its own reply, not a receiver validating an incoming, not-yet-answered command (which is legitimately still NT_MESSAGE regardless of how carefully the sender is built, confirmed by ruling out two separate sender-side fixes live: pre-marking ln_Type before sending, and using a genuine CreateArgstring()-backed rm_Args[0]). RXCOMM is the field that actually means "this is a command invocation" -- CAAPP.WHERE now checks rm_Action & RXCOMM instead, a completely ordinary, one-condition receive-loop check any third-party ARexx port (including AmiAuth's) can use, not a dedicated-port special case. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018CbPo4nMpnM5JQX3LKE5JP --- CLAUDE.md | 24 +++++++---- fixtures/classact-app/src/main.c | 73 +++++++++++++++----------------- server/README.md | 44 ++++++++++--------- server/include/where.h | 22 ++++++---- tests/copperline/README.md | 41 ++++++++++++------ 5 files changed, 112 insertions(+), 92 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9f1ca23..5701816 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,15 +48,21 @@ muirexx.c`) already used successfully against real MUI-Demo -- arrives at the receiver with its node type left at `NT_MESSAGE`, not `NT_REPLYMSG`; `rexxsyslib.library`'s own `IsRexxMsg()` reports such a message as not a genuine `RexxMsg` at all despite every other field -being correct, confirmed by direct inspection of the raw message from -the fixture's own receiving side. This was invisible in the existing -MUIREXX check only because MUI-Demo's own ARexx handling never calls -`IsRexxMsg()` on what it receives -- fixed by having `CAAPP.WHERE` -(a port dedicated solely to this one protocol) trust any message that -arrives on it rather than gating on `IsRexxMsg()`, see the doc comment -on `HandleWhereMessage()` in `fixtures/classact-app/src/main.c`. -`MUIREXX` has this same latent gap, not fixed there since no real MUI -target has ever tripped it and it's outside this issue's scope. +being correct. Two attempted sender-side fixes (pre-marking +`ln_Type = NT_REPLYMSG` before sending; using a genuine +`CreateArgstring()`-backed `rm_Args[0]`) changed nothing. Root cause: +`IsRexxMsg()` checks `ln_Type == NT_REPLYMSG`, which `ReplyMsg()` +sets -- the right check for a *sender* inspecting its own reply, not +a *receiver* validating an incoming, not-yet-answered command, which +is legitimately still `NT_MESSAGE` regardless of how the sender is +built. The field that actually means "this is a command invocation" +is `RXCOMM` itself (already set correctly by every sender here) -- +fixed by having `CAAPP.WHERE` validate incoming messages with +`rm_Action & RXCOMM` instead of `IsRexxMsg()`, see the doc comment on +`HandleWhereMessage()` in `fixtures/classact-app/src/main.c`. A +genuinely standard, ordinary ARexx port -- nothing dedicated-port-only +about this fix, any third-party implementer (e.g. AmiAuth) needs only +the same one-condition change. Phase 0.5 (reliability and reach into the wider ecosystem) before it: `WAITFOR` (including its diff --git a/fixtures/classact-app/src/main.c b/fixtures/classact-app/src/main.c index 0565d4b..063558a 100644 --- a/fixtures/classact-app/src/main.c +++ b/fixtures/classact-app/src/main.c @@ -208,37 +208,28 @@ static void ReplyWhere(struct RexxMsg *msg, int rc, const char *text) * same convention AmipGadgetCenter()/AmipClickWindowRelative() expect * on the AmiPilot side (server/src/action.c). * - * Deliberately does NOT gate on rexxsyslib.library's own IsRexxMsg() - * first, unlike server/src/arexx.c's own receiver. Two rounds of - * experimentation (2026-08-09) against this exact port, with a - * completely ordinary, unmodified IsRexxMsg() gate restored each - * time, ruled out every sender-side fix tried: neither pre-marking - * the outgoing message's own node type NT_REPLYMSG (the commonly - * cited technique for a hand-built ARexx command send) nor using a - * genuine CreateArgstring()-allocated rm_Args[0] (instead of a raw C - * string pointer) made IsRexxMsg() accept a message built via - * CreateRexxMsg()/FillRexxMsg()/PutMsg() (server/src/where.c's own - * AmipWhereQuery() -- the same recipe server/src/muirexx.c's - * AmipMuiRexxSend() already uses). The receiver's own ln_Type read - * stayed NT_MESSAGE regardless of what the sender set beforehand -- - * consistent with PutMsg() itself resetting it, a real Exec message- - * queueing behavior a real ARexx interpreter's own outgoing command - * sends must go through some other, non-public mechanism to avoid. - * IsRexxMsg() (a real rexxsyslib.library call, not just a header- - * field check) is seemingly reachable in its "true" state only for - * messages a live Rexx interpreter task itself constructs -- not - * achievable for this project's own hand-built sends by any public - * API combination tried. arexx.c's own receiver-side use of - * IsRexxMsg() only ever works because its senders are real ARexx - * scripts (`rx`), never this project's own MUIREXX/WHERE bridges. - * CAAPP.WHERE is a port dedicated solely to this one protocol - * (manifest/SPEC.md's own "Clash guard" -- a general-purpose port - * sharing this same MsgPort would need a real discriminator here - * instead), so trusting every message that arrives on it is the - * correct, verified choice -- not a shortcut. Any third-party - * application implementing WHERE needs the same: a receive loop that - * does NOT call IsRexxMsg() on what it gets, on a port used for - * nothing else. */ + * Gates on rm_Action & RXCOMM ("a command-level invocation", + * rexx/storage.h) in the caller (ProcessEvents() below), NOT + * rexxsyslib.library's own IsRexxMsg() the way server/src/arexx.c's + * receiver does -- found empirically (2026-08-09) that IsRexxMsg() + * checks ln_Type == NT_REPLYMSG, which ReplyMsg() sets (a documented + * Exec behavior); it answers "has this message already been + * replied", the right question for a SENDER inspecting what it got + * back on its own reply port, not for a RECEIVER validating an + * incoming, not-yet-answered command. A message built via + * CreateRexxMsg()/FillRexxMsg()/PutMsg() -- ours (server/src/where.c's + * AmipWhereQuery(), the same recipe server/src/muirexx.c's + * AmipMuiRexxSend() uses) and, most likely, any well-behaved + * third-party sender's -- is correctly NOT yet "replied" at the + * moment its receiver first sees it, so IsRexxMsg() correctly (if + * unhelpfully, for this purpose) reports false regardless of how + * carefully the sender is built. RXCOMM is the field actually meant + * to answer "is this a command invocation I should act on" -- see + * rexx/storage.h's own comment on it. arexx.c's receiver-side + * IsRexxMsg() gate happens to work for its own senders (real ARexx + * scripts via `rx`) but is checking the wrong condition for this + * purpose too; not touched here since it's outside this issue's + * scope and works for what it actually receives. */ static void HandleWhereMessage(struct RexxMsg *msg) { const char *cmdline = (const char *)ARG0(msg); @@ -310,7 +301,7 @@ static void CloseWherePort(struct MsgPort *port) Permit(); while ((msg = (struct RexxMsg *)GetMsg(port)) != NULL) { - if (IsRexxMsg(msg)) { + if (msg->rm_Action & RXCOMM) { ReplyWhere(msg, 20, NULL); } else { ReplyMsg((struct Message *)msg); @@ -368,16 +359,18 @@ static void ProcessEvents(Object *windowObject, struct MsgPort *wherePort) signals = Wait(windowSignal | whereSignal); if (whereSignal != 0 && (signals & whereSignal) != 0) { - /* No IsRexxMsg() gate here -- see HandleWhereMessage()'s - * own doc comment for why not, and why that's the - * correct choice for a port dedicated solely to this one - * protocol, not a shortcut. Every message on this port - * must still be replied by its receiver, or the sender - * leaks/hangs waiting for a reply that never comes -- - * HandleWhereMessage() always does, on every path. */ struct RexxMsg *msg; while ((msg = (struct RexxMsg *)GetMsg(wherePort)) != NULL) { - HandleWhereMessage(msg); + /* rm_Action & RXCOMM, not IsRexxMsg() -- see + * HandleWhereMessage()'s own doc comment for why. + * Every message on this port must still be replied + * by its receiver, or the sender leaks/hangs waiting + * for a reply that never comes. */ + if (msg->rm_Action & RXCOMM) { + HandleWhereMessage(msg); + } else { + ReplyMsg((struct Message *)msg); + } } } diff --git a/server/README.md b/server/README.md index da42700..f6465d5 100644 --- a/server/README.md +++ b/server/README.md @@ -935,28 +935,30 @@ Lands in phase 0.2 onward -- see not `NT_REPLYMSG`. `rexxsyslib.library`'s own `IsRexxMsg()` reports such a message as not a `RexxMsg` at all, even though `rm_Action` correctly carries `RXCOMM` and `ARG0()` reads back the real command - text -- confirmed by direct inspection (dumping `ln_Type`/ - `rm_Action` from the fixture's own receiving side) that a message - built exactly this way is structurally sound in every way - `IsRexxMsg()` doesn't check. A real ARexx interpreter's own - outgoing command messages (a genuine `rx` script's `ADDRESS`) - apparently arrive already marked `NT_REPLYMSG` through some internal - `rexxsyslib` mechanism this project's own hand-built sends don't - reproduce -- `IsRexxMsg()` seems intended for a *sender* validating - its own reply, not a receiver validating an incoming command, and - `server/src/arexx.c`'s own receiver-side use of it only ever works - because its senders are real ARexx scripts, never this project's own - bridges. Fixed by having `CAAPP.WHERE` -- a port dedicated solely to - this one protocol, per the "Clash guard" convention above -- trust - every message that arrives on it rather than gating on `IsRexxMsg()` - first; see the doc comment on `HandleWhereMessage()` in + text. Two attempted sender-side fixes (pre-marking the outgoing + message's own node type `NT_REPLYMSG`, and using a genuine + `CreateArgstring()`-allocated `rm_Args[0]` instead of a raw pointer) + changed nothing -- the receiver's own `ln_Type` stayed `NT_MESSAGE` + regardless. The actual explanation: `IsRexxMsg()` checks + `ln_Type == NT_REPLYMSG`, which `ReplyMsg()` sets -- the right + question for a *sender* inspecting what came back on its own reply + port, not for a *receiver* validating an incoming, not-yet-answered + command, which is correctly still `NT_MESSAGE` at that point no + matter how the sender is built. The field actually meaning "this is + a command invocation" is `RXCOMM` itself (`rexx/storage.h`'s own + comment on it), which every correct sender -- ours included -- + already sets. Fixed by having `CAAPP.WHERE` validate incoming + messages with `rm_Action & RXCOMM` instead of `IsRexxMsg()`; see the + doc comment on `HandleWhereMessage()` in `fixtures/classact-app/src/main.c` for the full account. - `MUIREXX`'s own `AmipMuiRexxSend()` has this same latent gap, masked - there only because MUI-Demo's built-in ARexx handling never calls - `IsRexxMsg()` on what it receives either -- not fixed here, since - MUIREXX genuinely works against every real target tried and isn't - this issue's scope, but worth knowing if a future MUIREXX target - ever does check. + `server/src/arexx.c`'s own receiver-side `IsRexxMsg()` gate checks + the same condition; it's never actually been observed to reject a + real ARexx script's (`rx`) own command (a genuine `rx`/interpreter + send is a different code path than this project's own hand-built + `CreateRexxMsg()`/`PutMsg()` sends, and exactly why it satisfies + `IsRexxMsg()` wasn't chased down here) -- not touched in this PR, + since it's outside this issue's scope and continues to work + correctly for what it actually receives. ## Phase 0.2 (shipped) diff --git a/server/include/where.h b/server/include/where.h index 02f1c86..40c36c5 100644 --- a/server/include/where.h +++ b/server/include/where.h @@ -22,14 +22,20 @@ * helpers already follow, here because the two are subtly different on * purpose, not by oversight). * - * A receiver implementing WHERE must NOT gate incoming messages on - * rexxsyslib.library's own IsRexxMsg() -- confirmed experimentally - * (2026-08-09, including two separate attempted sender-side fixes, - * neither of which changed the outcome) that a message built here via - * CreateRexxMsg()/FillRexxMsg()/PutMsg() never satisfies it, for - * reasons this module's own send code can't control. See the doc - * comment on fixtures/classact-app/src/main.c's HandleWhereMessage() - * for the full investigation and what a receiver should do instead. */ + * A receiver implementing WHERE should validate an incoming message + * with `rm_Action & RXCOMM` ("a command-level invocation", + * rexx/storage.h), NOT rexxsyslib.library's own IsRexxMsg() -- + * confirmed experimentally (2026-08-09) that IsRexxMsg() checks + * ln_Type == NT_REPLYMSG, which ReplyMsg() sets: the right question + * for a SENDER inspecting what came back on its own reply port, not + * for a RECEIVER validating an incoming, not-yet-answered command, + * which is correctly still NT_MESSAGE at that point regardless of how + * carefully the sender is built. This module's own send + * (CreateRexxMsg()/FillRexxMsg()/PutMsg(), the same recipe muirexx.c's + * AmipMuiRexxSend() uses) already sets RXCOMM correctly, matching any + * genuine ARexx command invocation -- see the doc comment on + * fixtures/classact-app/src/main.c's HandleWhereMessage() for the + * full account. */ #ifndef AMIPILOT_WHERE_H #define AMIPILOT_WHERE_H diff --git a/tests/copperline/README.md b/tests/copperline/README.md index dc408b2..95e2673 100644 --- a/tests/copperline/README.md +++ b/tests/copperline/README.md @@ -239,20 +239,33 @@ node type left at `NT_MESSAGE`, not `NT_REPLYMSG`. `rexxsyslib.library`'s own `IsRexxMsg()` call reports such a message as not a genuine `RexxMsg` at all, even though every other field (`rm_Action`'s `RXCOMM` bit, `ARG0()`'s command text) is exactly -correct. This was invisible in the MUIREXX check because MUI-Demo's -own ARexx handling never calls `IsRexxMsg()` on what it receives — -only `CAAPP.WHERE`, written to match `server/src/arexx.c`'s own -receiver-side convention of gating on it, ever exposed the gap. -Root-caused by direct inspection: a debug build of the fixture dumped -`ln_Type`/`rm_Action` straight from the message it received, off a -port dedicated to nothing else, ruling out every "wrong message" or -"wrong port" theory before landing on the real one. Fixed by having -`CAAPP.WHERE` trust any message that arrives on its own dedicated -port rather than gating on `IsRexxMsg()` at all — see the doc comment -on `HandleWhereMessage()` in `fixtures/classact-app/src/main.c` for -the full account, and `server/README.md`'s own WHERE section for what -this means for `MUIREXX` (same latent gap, not fixed there since it -isn't this issue's scope and no real MUI target has ever tripped it). +correct. Root-caused by direct inspection: a debug build of the +fixture dumped `ln_Type`/`rm_Action` straight from the message it +received, off a port dedicated to nothing else, ruling out every +"wrong message" or "wrong port" theory. Two attempted sender-side +fixes — pre-marking the outgoing message's own node type +`NT_REPLYMSG` (the commonly cited technique for a hand-sent ARexx +command), and using a genuine `CreateArgstring()`-backed `rm_Args[0]` +instead of a raw C string pointer — changed nothing; the receiver's +own `ln_Type` stayed `NT_MESSAGE` regardless, each rebuilt and +retested live. The actual explanation: `IsRexxMsg()` checks +`ln_Type == NT_REPLYMSG`, which `ReplyMsg()` sets — the right +question for a *sender* inspecting what came back on its own reply +port, not a *receiver* validating an incoming, not-yet-answered +command, which is legitimately still `NT_MESSAGE` at that point no +matter how carefully the sender is built. `RXCOMM` (`rexx/storage.h`'s +own "a command-level invocation") is the field that actually answers +the receiver's real question, and every sender here already sets it +correctly. Fixed by having `CAAPP.WHERE` validate incoming messages +with `rm_Action & RXCOMM` instead of `IsRexxMsg()` — a genuinely +ordinary, one-condition receive-loop change any third-party ARexx port +implementation can make, not a dedicated-port workaround; see the doc +comment on `HandleWhereMessage()` in +`fixtures/classact-app/src/main.c` for the full account, and +`server/README.md`'s own WHERE section for what this means for +`server/src/arexx.c`'s own receiver (checks the same wrong condition, +but has never been observed to actually reject a real ARexx script's +command, so left unchanged here — outside this issue's scope). ## Ad hoc smoke testing (debugging, new fixtures)