macOS bring-up: the designer starts, survives adding an object, and reads correctly

.gitignore

@@ -261,4 +261,9 @@ build/
 
 # Python Tools for Visual Studio (PTVS)
 __pycache__/
-*.pyc
+*.pyc
+# Assistant access config — carries a live MCP bearer token.
+.mcp.json
+
+# The Debug build tree, beside the Release one above.
+build-debug/

CMakeLists.txt

@@ -352,6 +352,47 @@ if(OES_USE_FIREBIRD)
             "/opt/firebird/lib"
     )
     add_compile_definitions(OES_USE_FIREBIRD)
+
+    if(APPLE)
+        # ⭐ THE FRAMEWORK CANNOT LOAD ITSELF — an rpath of ours has to stand in for the one
+        # the package forgot.
+        #
+        # The official macOS Firebird package ships
+        # `Firebird.framework/Versions/A/Resources/lib/libfbclient.dylib` linked against
+        # `@rpath/lib/libtommath.dylib` while carrying NO LC_RPATH of its own. `@rpath` is
+        # resolved against the load commands of the image that INITIATES the load — here
+        # backend's `wxDynamicLibrary::Load` (firebirdInterface.cpp) — so with nothing to
+        # substitute, dyld gives up:
+        #
+        #   Library not loaded: @rpath/lib/libtommath.dylib
+        #   Reason: no LC_RPATH's found
+        #
+        # The symptom is indistinguishable from "Firebird is not installed": Init() returns
+        # false on the failed Load and the startup dialog says the infobase could not be
+        # opened, with the framework sitting right there on disk. Naming the framework's
+        # Resources directory in OUR rpath makes `@rpath/lib/libtommath.dylib` resolve to
+        # the file that is already beside fbclient. Costs nothing when Firebird is absent —
+        # a non-existent rpath entry is simply skipped.
+        #
+        # BOTH directories are needed, for two different lookups:
+        #   Resources     — resolves `@rpath/lib/libtommath.dylib` (and fbclient's other
+        #                   siblings) once fbclient itself is in.
+        #   Resources/lib — lets the FIRST Load in ibInterfaceFirebird::Init() succeed on the
+        #                   bare name `libfbclient.dylib`. Without it that call fails, wx logs
+        #                   "Failed to load shared library" at error level, and the user is
+        #                   shown an alert about a library that the very next line loads
+        #                   successfully through the hard-coded framework path.
+        #
+        # Set before the engine's add_subdirectory() calls so every target below inherits it;
+        # wxWidgets is added earlier and is deliberately untouched.
+        foreach(_fb_rpath
+                "/Library/Frameworks/Firebird.framework/Versions/A/Resources/lib"
+                "/Library/Frameworks/Firebird.framework/Versions/A/Resources")
+            list(APPEND CMAKE_BUILD_RPATH   "${_fb_rpath}")
+            list(APPEND CMAKE_INSTALL_RPATH "${_fb_rpath}")
+        endforeach()
+        unset(_fb_rpath)
+    endif()
     if(FIREBIRD_LIBRARY)
         message(STATUS "Firebird client found (optional): ${FIREBIRD_LIBRARY}")
     else()

CONTRIBUTING.md

@@ -86,6 +86,21 @@ placement and file layout vary a little by area, and the local convention wins.
 prefix for public classes, `m_` for members, `s_` for statics, `g_` for compile-time
 constants; tabs for indentation.
 
+**Comments carry a marker, and the marker means something.** Four of them are used throughout
+`src/engine`, and they are not decoration:
+
+| | |
+|---|---|
+| ⭐ | A deliberate decision, with the reason it was made that way rather than the obvious way. |
+| ⭐⭐ | The central point of the file or function — what it exists for. |
+| ⚠ | A limit or caveat: where the thing above stops applying, and what breaks past it. |
+| 🛑 | A defect that actually happened, and what it cost. Not a hypothetical. |
+
+The marker is followed by one statement in capitals, then the explanation in ordinary prose;
+where it matters, the date and who decided. The point of the grammar is that the four are not
+interchangeable: ⭐ says *this was chosen*, 🛑 says *this was paid for*. Reach for 🛑 only when
+something really did go wrong, or it stops meaning anything.
+
 **Say why in the commit message, not what.** The diff already says what changed. What it
 cannot say is what was wrong before, what you measured, and what you decided against — and
 that is the part somebody will need in a year. Long commit messages are normal here.

src/engine/backend/appData.cpp

@@ -944,7 +944,46 @@ long ibApplicationData::RunApplication(const wxString& strAppName, const wxStrin
 	// file/ibuser/ibpwd/locale/debug). enterprise.exe / designer.exe /
 	// daemon.exe declare these as the long name of their legacy short
 	// options, so one builder feeds every parser.
-	wxString executeCmd = strAppName + wxT(' ');
+
+	// Resolve the binary next to this one. A bare "enterprise" is looked up on PATH and in the
+	// working directory, and it is in neither; on macOS it is not even a plain file, since the
+	// sibling is enterprise.app and the executable sits inside it. Without this the fork succeeds,
+	// the exec fails, and wxExecute still returns a pid — so the caller is told the application
+	// started while nothing runs.
+	wxFileName home = wxFileName::DirName(
+		wxFileName(wxStandardPaths::Get().GetExecutablePath()).GetPath());
+
+#ifdef __WXOSX__
+	// This binary is itself inside a bundle, so its siblings are three levels up.
+	if (home.GetDirCount() >= 2
+		&& home.GetDirs().Last().IsSameAs(wxT("MacOS"))
+		&& home.GetDirs()[home.GetDirCount() - 2].IsSameAs(wxT("Contents"))) {
+		home.RemoveLastDir();
+		home.RemoveLastDir();
+		home.RemoveLastDir();
+	}
+#endif
+
+	wxFileName binary(home);
+	binary.SetFullName(strAppName);
+
+#ifdef __WXOSX__
+	if (!binary.FileExists()) {
+		wxFileName bundled(home);
+		bundled.AppendDir(strAppName + wxT(".app"));
+		bundled.AppendDir(wxT("Contents"));
+		bundled.AppendDir(wxT("MacOS"));
+		bundled.SetFullName(strAppName);
+		if (bundled.FileExists())
+			binary = bundled;
+	}
+#endif
+
+	// The bare name stays the fallback, so a layout this does not recognise behaves as before.
+	// Quoted because a resolved path can contain a space and wxExecute splits on those.
+	wxString executeCmd = binary.FileExists()
+		? (wxT('"') + binary.GetFullPath() + wxT("\" "))
+		: (strAppName + wxT(' '));
 
 	if (m_strFile.IsEmpty()) {
 
@@ -983,20 +1022,26 @@ long ibApplicationData::RunApplication(const wxString& strAppName, const wxStrin
 	const long execute = wxExecute(executeCmd);
 
 	if (searchDebug) {
+		// Scan in rounds, the same way the manifest path below does, and for the same
+		// reason: one sweep races the process that was just started. Its debug server
+		// is created near the end of bootstrap, after the database is open, so the
+		// first sweep finds nothing listening and its threads exit on connect-refused
+		// with the counter spent - which is what lets the next SearchServer call make
+		// fresh ones. A single sweep followed by a 1.5-second wait, which is what stood
+		// here, could only ever succeed on a fast warm start.
+		const int kRounds       = 60;   // 60 rounds * 250ms = ~15s, and it stops on success
+		const int kRoundSleepMs = 250;
 
-		unsigned short num_attempts = 0;
-
-		debugClient->SearchServer(true);
-		while (debugClient != nullptr) {
+		for (int round = 0; round < kRounds; ++round) {
 
-			if (debugClient->GetConnectionSuccess())
+			if (debugClient == nullptr)
 				break;
 
-			if (num_attempts > 300)
-				break;
+			debugClient->SearchServer(true);
+			wxMilliSleep(kRoundSleepMs);
 
-			num_attempts++;
-			wxMilliSleep(5);
+			if (debugClient->GetConnectionSuccess())
+				break;
 		}
 	}
 

src/engine/backend/databaseLayer/databaseMaterializeBuilder.cpp

@@ -970,8 +970,8 @@ ibQueryRelPtr RenderMaterializedRead(const ibMaterializeReadSpec& spec, const wx
 		}
 	}
 
-	for (const auto& f : spec.m_filters)
-		q.Where(ibBinOp(ibQueryBinOp::Eq, ibCol(f.first), ibConst(f.second)));
+	for (const ibQueryExprPtr& f : spec.m_filters)
+		if (f) q.Where(f);
 
 	if (anyAggregate) {
 		for (const ibQueryExprPtr& k : keys) q.GroupBy(k);

src/engine/backend/databaseLayer/databaseMaterializeBuilder.h

@@ -302,7 +302,9 @@ struct ibMaterializeReadSpec
 	ibValue  m_fromGrain;
 
 	std::vector<wxString> m_keyColumns;    // projected, and grouped by when any column aggregates
-	std::vector<std::pair<wxString, ibValue>> m_filters;   // column = value, applied INSIDE
+	// Conditions applied INSIDE. Whole predicates rather than column/value pairs: a logical column
+	// can be several physical fields, and which ones is a metadata question this floor cannot ask.
+	std::vector<ibQueryExprPtr> m_filters;
 
 	std::vector<ibMaterializeReadColumn> m_columns;
 	bool m_dropZeroRows = false;           // keep only rows where some reported figure is non-zero

src/engine/backend/debugger/debugClient.cpp

@@ -523,13 +523,17 @@ void ibDebuggerClient::EvaluateToolTip(const wxString& strFileName, const wxStri
 // open forms (Max, 2026-09-02). That is the whole advantage: the situation being investigated is
 // reproduced where it actually happens, not in a copy that behaves nearly the same. What makes it
 // safe to do that in somebody's live session is the other half — the transaction the far end wraps
-// it in, which is rolled back whatever the code does.
-void ibDebuggerClient::RunSandbox(const wxString& code)
+// it in, which is rolled back unless this call says otherwise.
+//
+// ⚠ The flag goes LAST so an older runtime, which stops reading after the string, rolls back —
+// it cannot be talked into keeping writes by a newer designer.
+void ibDebuggerClient::RunSandbox(const wxString& code, bool keepWrites)
 {
 	if (ibDebuggerClient::IsEnterLoop()) {
 		ibWriterMemory commandChannel;
 		commandChannel.w_u16(CommandId_RunSandbox);
 		commandChannel.w_stringZ(code);
+		commandChannel.w_u8(keepWrites ? 1 : 0);
 		SendCommand(commandChannel.pointer(), commandChannel.size());
 	}
 }

src/engine/backend/debugger/debugClient.h

@@ -310,9 +310,10 @@ class BACKEND_API ibDebuggerClient {
 	//evaluate for tooltip
 	void EvaluateToolTip(const wxString& strFileName, const wxString& strModuleName, const wxString& strExpression);
 
-	// Arbitrary code, run in the stopped runtime inside a transaction the far end always rolls
-	// back. The answer arrives as OnSandboxResult.
-	void RunSandbox(const wxString& code);
+	// Arbitrary code, run in the stopped runtime inside a transaction the far end rolls back —
+	// unless `keepWrites` asks it to commit. Off by default: undoing is what makes this safe in a
+	// live session. A failed run is rolled back either way. The answer arrives as OnSandboxResult.
+	void RunSandbox(const wxString& code, bool keepWrites = false);
 
 	// ⭐ ASK THE RUNNING APPLICATION FOR A PICTURE OF ITS WINDOW. `reason` is shown to the person on
 	// the other end, who decides — the answer arrives as OnScreenshot, empty when they declined.

src/engine/backend/debugger/debugServer.cpp

@@ -115,17 +115,31 @@ bool ibDebuggerServer::CreateServer(const wxString& hostName, unsigned short sta
 	}
 
 	if (wait) {
+		// Hold the bootstrap until the debug client is on the socket, so breakpoints
+		// exist before OnStart runs. Bounded, because a process started with --debug
+		// and no designer would otherwise never finish starting; when the bound
+		// expires it goes on without a debugger. An assertion used to stand here and
+		// it fired on a race nobody at the window could act on - the designer scans
+		// the ports while this process is still connecting to its database, so the
+		// two miss each other on a slow start.
+		const int kWaitForClientMs = 30000;
+		int waited = 0;
 
 		while (m_socketConnectionThread != nullptr) {
 
 			if (m_bUseDebug || m_socketConnectionThread->m_acceptConnection)
 				break;
 
+			if (waited >= kWaitForClientMs) {
+				ibJournalWarning(wxT("debugger"),
+					wxT("no debug client connected within %d ms - starting without a debugger"),
+					kWaitForClientMs);
+				break;
+			}
+
 			wxMilliSleep(5);
+			waited += 5;
 		}
-
-		wxASSERT_MSG(m_socketConnectionThread != nullptr
-			&& m_socketConnectionThread->m_socket != nullptr, _("Client not connected!"));
 	}
 	else {
 		// Non-blocking server (wes / designer auto-debug): wait for the
@@ -949,7 +963,11 @@ void ibDebuggerServer::ibDebuggerServerConnection::EntryClient()
 				m_socket->SetOption(SOL_SOCKET, SO_KEEPALIVE, &flag, sizeof(flag));
 			}
 
-			if (m_socket != nullptr || m_waitConnection)
+			// Keep trying. Accept(true) comes back after the socket's own ten-second
+			// timeout, and a debugger that has not arrived yet is the ordinary case.
+			// Breaking on that timeout is what left m_socket null while the flag below
+			// said a connection had been accepted.
+			if (m_socket != nullptr)
 				break;
 		}
 
@@ -1324,6 +1342,12 @@ void ibDebuggerServer::ibDebuggerServerConnection::RecvCommand(void* pointer, un
 		wxString code;
 		commandReader.r_stringZ(code);
 
+		// Optional on the wire: a client that predates the flag sends the string and stops, so
+		// elapsed() is 0 and this stays false. An old peer, either direction, means rollback.
+		bool keepWrites = false;
+		if (commandReader.elapsed() >= 1)
+			keepWrites = commandReader.r_u8() != 0;
+
 		// ⚠ ANSWERED EVEN WHEN IT CANNOT RUN — see the note on the evaluation above. A caller that
 		// waits deserves a sentence, and "not parked" is a sentence it can act on; silence is one
 		// it can only time out on.
@@ -1356,8 +1380,13 @@ void ibDebuggerServer::ibDebuggerServerConnection::RecvCommand(void* pointer, un
 						ibValueSystemFunction::Message(text, status); });
 			};
 
-			tell(_("An assistant is running code here in a sandbox - nothing it writes is kept."),
-				ibStatusMessage::ibStatusMessage_Information);
+			// The sentence follows the flag: "nothing is kept", said while the transaction is about
+			// to be committed, is the one wrong thing to tell somebody here.
+			tell(keepWrites
+				? _("An assistant is running code here and keeping what it writes - this changes the base.")
+				: _("An assistant is running code here in a sandbox - nothing it writes is kept."),
+				keepWrites ? ibStatusMessage::ibStatusMessage_Warning
+				           : ibStatusMessage::ibStatusMessage_Information);
 
 			// …AND WHAT IT WAS. A person who is told that "code is running" and not WHICH code has
 			// been told the alarming half and none of the useful one; they are sitting in front of
@@ -1475,8 +1504,16 @@ void ibDebuggerServer::ibDebuggerServerConnection::RecvCommand(void* pointer, un
 				// when the thing being investigated is a timeout.
 				elapsed = std::chrono::steady_clock::now() - began;
 
+				// A failed run is rolled back whatever was asked: ran is false when the code threw
+				// partway, and committing would keep the half that got through — a document posted
+				// with its movements missing. The flag means "keep it if it worked".
+				const bool keeping = keepWrites && ran;
+
 				try {
-					layer->RollBack();
+					if (keeping)
+						layer->Commit();
+					else
+						layer->RollBack();
 				}
 				catch (...) {
 					// A rollback that cannot run is not something the caller can act on, and saying
@@ -1502,10 +1539,14 @@ void ibDebuggerServer::ibDebuggerServerConnection::RecvCommand(void* pointer, un
 				const wxString shown = answer.length() > 500
 					? answer.Left(497) + wxT("...") : answer;
 
+				// The ending says whether the base moved, which is the fact worth having.
 				tell(ran
-					? wxString::Format(
-						_("The sandbox finished and everything it wrote was rolled back. %s"), shown)
-					: wxString::Format(_("The sandbox stopped: %s"), shown),
+					? wxString::Format(keepWrites
+						? _("The code finished and what it wrote was kept. %s")
+						: _("The sandbox finished and everything it wrote was rolled back. %s"), shown)
+					: wxString::Format(keepWrites
+						? _("The code stopped and nothing was kept - a failed run is always rolled back. %s")
+						: _("The sandbox stopped: %s"), shown),
 					ran ? ibStatusMessage::ibStatusMessage_Information
 					    : ibStatusMessage::ibStatusMessage_Error);
 			}

src/engine/backend/mcp/mcpDebugBridge.cpp

@@ -114,8 +114,8 @@ bool ibMcpDebugBridge::Evaluate(const wxString& expression, wxString& answer, in
 	return arrived;
 }
 
-bool ibMcpDebugBridge::Sandbox(const wxString& code, bool& ran, wxString& answer, wxString& json,
-	std::vector<wxString>& printed, wxLongLong_t& microseconds, int timeoutMs)
+bool ibMcpDebugBridge::Sandbox(const wxString& code, bool keepWrites, bool& ran, wxString& answer,
+	wxString& json, std::vector<wxString>& printed, wxLongLong_t& microseconds, int timeoutMs)
 {
 	ran = false;
 	microseconds = 0;
@@ -138,7 +138,7 @@ bool ibMcpDebugBridge::Sandbox(const wxString& code, bool& ran, wxString& answer
 		m_sandboxMicroseconds = 0;
 	}
 
-	debugClient->RunSandbox(code);
+	debugClient->RunSandbox(code, keepWrites);
 
 	std::unique_lock<std::mutex> lock(m_mutex);
 	const bool arrived = m_answered.wait_for(lock,

src/engine/backend/mcp/mcpDebugBridge.h

@@ -95,14 +95,17 @@ class BACKEND_API ibMcpDebugBridge : public ibDebuggerClientBridge {
 	// printable form. Same road the watch window uses.
 	bool Unfold(const wxString& expression, std::vector<Local>& members, int timeoutMs = 3000);
 
-	// ⭐⭐ CODE, NOT AN EXPRESSION — run in the stopped runtime and then UNDONE. The far end wraps it
-	// in a transaction it always rolls back, so the base the person is using is not changed by an
-	// experiment run inside their own session. Longer by default than an evaluation: this may write
-	// documents, post them and read them back, which is work rather than a lookup.
+	// ⭐⭐ CODE, NOT AN EXPRESSION — run in the stopped runtime and, by default, UNDONE. The far end
+	// wraps it in a transaction, so the base the person is using is not changed by an experiment run
+	// inside their own session. Longer by default than an evaluation: this may write documents, post
+	// them and read them back, which is work rather than a lookup.
+	//
+	// `keepWrites` COMMITS instead — the caller saying this run is meant to change the base rather
+	// than to ask it something. Off unless asked for; a failed run is rolled back either way.
 	//
 	// `microseconds` comes back with the rest: how long the code itself took, timed in the process
 	// that ran it. Measuring from this side would be measuring the socket and the wait.
-	bool Sandbox(const wxString& code, bool& ran, wxString& answer, wxString& json,
+	bool Sandbox(const wxString& code, bool keepWrites, bool& ran, wxString& answer, wxString& json,
 		std::vector<wxString>& printed, wxLongLong_t& microseconds, int timeoutMs = 30000);
 
 	// ⭐⭐ ASK THE RUNNING APPLICATION FOR A PICTURE OF ITS WINDOW, and wait for the answer. `reason`