subagent transcript

session 18e25685 · workflow wf_23df5b1c
← 18e25685 · archive
delegated task
Review the wasm-port changes to FreeCAD core for correctness and #ifdef discipline (desktop builds must stay green). Read and assess: - /home/magik6k/
▸ Magik
Review the wasm-port changes to FreeCAD core for correctness and #ifdef discipline (desktop builds must stay green). Read and assess: - /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/App/Application.cpp and Application.h — the bootstrapEarly() lazy-singleton mechanism, idempotent LoadParameters/setupPythonTypes, HOME/passwd guards, recompute-worker-thread skip on wasm. - /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Base/Interpreter.cpp — the wasm PyConfig / module_search_paths / PYTHONPATH handling. - /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Main/MainGui.cpp and MainCmd.cpp — early message handler, putenv guard, inittab registration, freecad_run_python export. - /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/Gui/FcWasmProcess.h/.cpp — the QProcess stand-in. - /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/FCConfig.h — the __EMSCRIPTEN__ -> FC_OS_LINUX+FC_OS_WASM mapping. - /home/magik6k/lcad-wasm/freecad-port/FreeCAD/src/App/ApplicationDirectories.cpp, src/Base/SystemHandler.cpp, src/Gui/Assistant.h, src/Mod/Start/App/ThumbnailSource.cpp. Use `cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git diff --ignore-all-space c3cd817f27^ HEAD -- <file>` to see only real changes. Look for: changes NOT guarded by #ifdef FC_OS_WASM/__EMSCRIPTEN__ that could break desktop; correctness bugs; fragile assumptions; missing error handling. Report concrete findings with file:line evidence.
Fable
thinking
I'll start by looking at the git history and examining the diffs for each file to understand what changed.
Fable
I'll analyze the wasm-port changes across the listed files. Let me start by examining the git context and diffs.
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git log --oneline -5 c3cd817f27^..HEAD 2>&1 | head -20; echo \"---COMMIT---\"; git show --stat c3cd817f27 2>&1 | head -40", "description": "Inspect git history around the commi…
tool result
439ed31cb5 wasm: keep ASYNCIFY link config (JSPI documented as forward path)
6ed7f660f2 wasm: update vendored Coin (WebGL2 glue port) + Quarter context sync
f87496364c wasm: Stage 4 WebGL viewport — fixed-function emulator + Coin glue port
123e7c2888 wasm: FreeCAD GUI fully boots in the browser (Stage 3 gate PASSED)
9e3f90a6e2 wasm: asyncify Qt — GUI renders in browser, modal dialogs work
---COMMIT---
commit c3cd817f279301f8a7acdd1381babc66789472c4
Author: Claude <noreply@anthropic.com>
Date:     …[truncated]
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git diff --ignore-all-space --stat c3cd817f27^ HEAD 2>&1 | tail -40", "description": "Get diff stat overview"}
tool result
 CMakeLists.txt                          |  17 +
 cMake/FreeCAD_Helpers/SetupBoost.cmake  |   7 +-
 cMake/FreeCAD_Helpers/SetupCoin3D.cmake |   7 +
 cMake/FreeCAD_Helpers/SetupQt.cmake     |   5 +
 src/3rdParty/coin                       |   2 +-
 src/App/Application.cpp                 |  57 +++-
 src/App/Application.h                   |  15 +
 src/App/ApplicationDirectories.cpp      |  23 +-
 src/Base/Interpreter.cpp                |  46 ++-
 src/Base/SystemHandler.cpp              |   4 +-
   …[truncated]
Fable
Let me look at the diffs for the specific files mentioned.
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git diff --ignore-all-space c3cd817f27^ HEAD -- src/App/Application.cpp src/App/Application.h 2>&1", "description": "Diff Application.cpp/h"}
tool result
diff --git a/src/App/Application.cpp b/src/App/Application.cpp
index 7436f1b309..7251dc36a2 100644
--- a/src/App/Application.cpp
+++ b/src/App/Application.cpp
@@ -415,10 +415,37 @@ Application::Application(std::map<std::string,std::string> &mConfig)
     mpcPramManager["User parameter"] = _pcUserParamMngr;
 
     _stopRecomputeThread = false;
+#ifndef FC_OS_WASM
+    // Single-threaded wasm build: no worker; queueRecomputeRequest() always
+    // takes the inline path (canRecomputeRequestOnWorke  …[truncated]
Fable
thinking
Looking at the Application.cpp changes, I'm noticing that `setupPythonTypes()` now has a Python initialization check in the constructor, but it's not wrapped in the WASM preprocessor guard. This means on desktop builds, the behavior changes — previously this was called unconditionally, but now it depends on whether Python is initialized at the point when Application is constructed.

The flow becomes: the constructor checks if Python is initialized and calls `setupPythonTypes()` if true, then `initApplication()` calls it again explicitly. Since there's an idempotent static flag guarding the actual registration, the net effect is that it runs once regardless. This should be fine as long as Pyt  …
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git diff --ignore-all-space c3cd817f27^ HEAD -- src/Base/Interpreter.cpp 2>&1", "description": "Diff Interpreter.cpp"}
tool result
diff --git a/src/Base/Interpreter.cpp b/src/Base/Interpreter.cpp
index a2a18a4131..48f2877532 100644
--- a/src/Base/Interpreter.cpp
+++ b/src/Base/Interpreter.cpp
@@ -620,6 +620,43 @@ void initInterpreter(int argc, char* argv[])
     PyConfig_InitIsolatedConfig(&config);
     config.isolated = 0;
     config.user_site_directory = 1;
+#ifdef FC_OS_WASM
+    // The wasm build has no baked-in installation prefix that exists at run
+    // time. Set the stdlib search paths explicitly from PYTHO  …[truncated]
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git diff --ignore-all-space c3cd817f27^ HEAD -- src/Main/MainGui.cpp src/Main/MainCmd.cpp 2>&1", "description": "Diff MainGui/MainCmd"}
tool result
diff --git a/src/Main/MainCmd.cpp b/src/Main/MainCmd.cpp
index 13bf26b9ec..8d6da2e1a6 100644
--- a/src/Main/MainCmd.cpp
+++ b/src/Main/MainCmd.cpp
@@ -46,6 +46,25 @@
 using App::Application;
 using Base::Console;
 
+#ifdef __EMSCRIPTEN__
+#include <emscripten.h>
+#include <Python.h>
+
+// Defined in the generated WasmInittab.cpp; registers the statically linked
+// Python extension modules before the interpreter is initialized.
+void freecadWasmRegisterInittab();
+
+extern "C" {
+//   …[truncated]
Fable
thinking
I'm noting that the MainCmd.cpp change introduces a `catch (const std::exception& e)` block that isn't wrapped in `#ifdef __EMSCRIPTEN__`, and it gets inserted before the catch-all handler. I need to verify the ordering of catch blocks since more-specific exception types like Base::Exception should come before the generic std::exception catch.
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && sed -n '90,175p' src/Main/MainCmd.cpp", "description": "Read MainCmd try/catch chain"}
tool result
#endif

    // Name and Version of the Application
    App::Application::Config()["ExeName"] = "FreeCAD";
    App::Application::Config()["ExeVendor"] = "FreeCAD";
    App::Application::Config()["AppDataSkipVendor"] = "true";

    // set the banner (for logging and console)
    App::Application::Config()["CopyrightInfo"] = sBanner;

    try {
        // Init phase ===========================================================
        // sets the default run mode for FC, starts with comma  …[truncated]
Fable
thinking
I'm thinking through the exception hierarchy and catch ordering. The `catch (const std::exception&)` block comes after `Base::Exception`, which is the right placement since `Base::Exception` likely inherits from `std::exception` and needs to be caught first to avoid the more general handler catching it. This narrows what was previously an ambiguous catch-all for standard exceptions.

The exit codes are consistent — both the old catch-all and the new `std::exception` handler return 101, so that's not a breaking change. For the wasm build, I'm noticing that `freecad_run_python` uses `Base::PyGILStateLocker` and `Base::Console`, but the header includes should be fine since this is only a compil  …
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git diff --ignore-all-space c3cd817f27^ HEAD -- src/Gui/FcWasmProcess.h src/Gui/FcWasmProcess.cpp src/FCConfig.h 2>&1", "description": "Diff FcWasmProcess and FCConfig"}
tool result
diff --git a/src/FCConfig.h b/src/FCConfig.h
index a7915be954..600714f385 100644
--- a/src/FCConfig.h
+++ b/src/FCConfig.h
@@ -34,7 +34,16 @@
 // switching the operating systems
 
 // First check for *WIN64* since the *WIN32* are also set on 64-bit platforms
-#if defined(WIN64) || defined(_WIN64) || defined(__WIN64__)
+#if defined(__EMSCRIPTEN__)
+// WebAssembly port: emscripten's libc is POSIX-shaped; the Linux code paths
+// are the closest fit. FC_OS_WASM marks the few places that must   …[truncated]
Fable
thinking
I'm noticing that FcWasmProcess redefines the `readyRead()` signal even though QIODevice already declares it in the base class, which could cause shadowing or ambiguity issues. Since this is wasm-only code, it would only fail to compile in that specific environment rather than on desktop platforms.
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && git diff --ignore-all-space c3cd817f27^ HEAD -- src/App/ApplicationDirectories.cpp src/Base/SystemHandler.cpp src/Gui/Assistant.h src/Mod/Start/App/ThumbnailSource.cpp src/Mod/…
tool result
diff --git a/src/App/ApplicationDirectories.cpp b/src/App/ApplicationDirectories.cpp
index dd1fe3ad7d..7fa4356604 100644
--- a/src/App/ApplicationDirectories.cpp
+++ b/src/App/ApplicationDirectories.cpp
@@ -354,7 +354,12 @@ void ApplicationDirectories::configureHelpDirectory(const std::map<std::string,s
 fs::path ApplicationDirectories::getUserHome()
 {
     fs::path path;
-#if defined(FC_OS_LINUX) || defined(FC_OS_CYGWIN) || defined(FC_OS_BSD) || defined(FC_OS_MACOSX)
+#if defined(FC_OS_WASM)
+  …[truncated]
Fable
thinking
I'm noticing a potential issue with FileUtilities.cpp—it's missing an ifdef guard that would protect the changes for desktop builds only. The code is replacing `.toTimeZone(QTimeZone::utc())` with `.toUTC()` and removing the QTimeZone include, but I need to verify that `.toUTC()` is actually equivalent and won't break anything. Q_OS_WASM is a Qt-defined macro that gets set during wasm builds, so that's valid, though it's inconsistent with the codebase's usual FC_OS_WASM convention. On desktop, Q_OS_WASM won't be defined, so the desktop path gets taken correctly — though I should verify that the necessary Qt header is included to define it on wasm. In ApplicationDirectories.cpp, the guard ord  …
Fable
Let me investigate the unguarded changes' desktop impact. Checking callers of `LoadParameters`, `setupPythonTypes`, and the constructor path.
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && grep -rn \"LoadParameters\\|setupPythonTypes\" src/ --include=*.cpp --include=*.h | grep -v \"Fc_wasm\" 2>&1", "description": "Find callers of LoadParameters/setupPythonTypes"}
tool result
src/App/Application.cpp:429:        setupPythonTypes();
src/App/Application.cpp:445:    LoadParameters();
src/App/Application.cpp:461:void Application::setupPythonTypes()
src/App/Application.cpp:2930:    LoadParameters();
src/App/Application.cpp:3048:    setupPythonTypes();
src/App/Application.cpp:3291:void Application::LoadParameters()
src/App/Application.h:991:    static void setupPythonTypes();
src/App/Application.h:1014:    static void LoadParameters();
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && grep -rn \"new Application\\|Application(mConfig)\\|_pcUserParamMngr\\s*=\\|_pcSysParamMngr\\s*=\" src/App/Application.cpp 2>&1", "description": "Find Application construction …
tool result
src/App/Application.cpp:446:    _pcSingleton = new Application(mConfig);
src/App/Application.cpp:2083:    _pcSysParamMngr = nullptr;
src/App/Application.cpp:2084:    _pcUserParamMngr = nullptr;
src/App/Application.cpp:3019:        Application::_pcSingleton = new Application(mConfig);
src/App/Application.cpp:3046:        Application::_pcSingleton = new Application(mConfig);
src/App/Application.cpp:3308:    _pcSysParamMngr = ParameterManager::Create();
src/App/Application.cpp:3311:    _pcUserParam  …[truncated]
Fable
thinking
I'm checking how the parameter managers are being reset to null and whether that affects the idempotency guard in LoadParameters. Looking at line 2930 to see if there's another LoadParameters call in a desktop code path.
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && sed -n '2075,2090p;2920,2935p' src/App/Application.cpp 2>&1", "description": "Context around param reset and LoadParameters call at 2930"}
tool result
                Base::Console().log("Saving %s...\n", it.first.c_str());
                it.second->SaveDocument();
                Base::Console().log("Saving %s...done\n", it.first.c_str());
            }
        }
    }

    paramMgr.clear();
    _pcSysParamMngr = nullptr;
    _pcUserParamMngr = nullptr;

#ifdef FC_DEBUG
    // Do this only in debug mode for memory leak checkers
    cleanupUnits();
#endif

                              mConfig["BuildVersionPoint"].c_str(),
                     …[truncated]
Fable
thinking
I'm tracing where `LoadParameters` gets called on desktop—there's one in `initConfig` during startup, and another in the constructor that's actually wrapped in a WASM-specific ifdef, so I need to verify which call sites actually execute on the desktop build.

The idempotency guard checks if both managers already exist and bails out early if they do. During the initial startup call at line 2930, they should be null, so the guard passes through and creates them. The destructor nulls them during shutdown, so if there were a re-initialization after a full cleanup, the guard would allow it. But I'm wondering whether `initConfig` could be called multiple times in a single process, or if there's a   …
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && sed -n '3000,3055p' src/App/Application.cpp 2>&1", "description": "Read initApplication region"}
tool result
    mConfig["BOOST_VERSION"] = BOOST_LIB_VERSION;
    mConfig["PYTHON_VERSION"] = PY_VERSION;
    mConfig["QT_VERSION"] = QT_VERSION_STR;
    mConfig["COIN3D_VERSION"] = fcCoin3dVersion;
    mConfig["COIN3D_SOURCE"] = fcCoin3dSource;
    mConfig["PIVY_VERSION"] = fcPivyVersion;
    mConfig["PIVY_SOURCE"] = fcPivySource;
    mConfig["EIGEN_VERSION"] = fcEigen3Version;
    mConfig["PYSIDE_VERSION"] = fcPysideVersion;
#ifdef SMESH_VERSION_STR
    mConfig["SMESH_VERSION"] = SMESH_VERSION_STR;
#endif  …[truncated]
Fable
thinking
I'm noticing something odd about the initialization order — at line 3019, there's a new Application being constructed with mConfig, but this happens in what looks like the verbose/version check path, which is in a different function than where initApplication actually starts. The question is whether the Python interpreter has been initialized at that point.

Looking at the code change, the old constructor would have unconditionally called setupPythonTypes(), which would crash if Py wasn't initialized. But the new code guards this with `if (Py_IsInitialized())`, so if Py isn't ready yet, setupPythonTypes gets skipped. For the verbose/version path that just prints and exits, this is actually s  …
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && grep -n \"Application::init\\b\\|::initConfig\\|::initApplication\\|initInterpreter\\|Interpreter().init\\|void Application::init\" src/App/Application.cpp | head -30 2>&1", "d…
tool result
441:    // main() and before App::Application::init(). Some of them read the
2165:void Application::init(int argc, char ** argv)
2185:void Application::initTypes()
2766:void Application::initConfig(int argc, char ** argv)
2869:    std::string pythonpath = Base::Interpreter().init(argc,argv);
3031:void Application::initApplication()
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && sed -n '2165,2185p' src/App/Application.cpp 2>&1", "description": "Read Application::init"}
tool result
void Application::init(int argc, char ** argv)
{
    try {
        Base::SystemHandler::installNewHandler();
        Base::SystemHandler::installSegfaultHandler();

        initTypes();

        initConfig(argc,argv);
        initApplication();
        initExceptions();
    }
    catch (...) {
        // force the log to flush
        destructObserver();
        throw;
    }
}

// clang-format off
void Application::initTypes()
Fable
thinking
Tracing through the initialization sequence, the Python interpreter gets initialized at line 2869 within `initConfig()`, which happens before both the verbose/version output at line 3019 and the `initApplication()` call at line 3046, so Python is already available at both of those points.

On desktop, `Py_IsInitialized()` in the constructor will always return true, making the `initApplication()` call idempotent—no actual behavior change there. The `LoadParameters` call at line 2930 also falls within `initConfig()` after the interpreter initializes, so that's fine too.

The Application.cpp changes look safe for desktop. Now examining the broader diff, I notice FileUtilities.cpp has an unguard  …
Fable
Let me verify the `findHomePath` #if/#elif ordering and check ThumbnailSource includes for `Q_OS_WASM`.
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && grep -n \"^#if \\|^#elif \\|^#else\\|^#endif\\|findHomePath\" src/App/ApplicationDirectories.cpp | sed -n '1,40p' 2>&1; echo \"=== ThumbnailSource includes ===\"; sed -n '1,40p…
tool result
34:#if defined(FC_OS_LINUX) || defined(FC_OS_MACOSX) || defined(FC_OS_BSD)
36:#endif
53:#if defined(FC_OS_WIN32)
55:#else
57:#endif
318:#else
320:#endif
332:#else
334:#endif
348:#else
350:#endif
357:#if defined(FC_OS_WASM)
362:#elif defined(FC_OS_LINUX) || defined(FC_OS_CYGWIN) || defined(FC_OS_BSD) || defined(FC_OS_MACOSX)
374:#else
376:#endif
385:#if defined(FC_OS_WIN32)  // This is ONLY used on Windows now, so don't even compile it elsewhere
397:#endif
415:#if defined(FC_OS_LINUX) || defined(  …[truncated]
Fable
thinking
I'm tracing through the findHomePath function definition around line 671 and trying to identify which preprocessor conditional guards it—there's an `#if defined(__OpenBSD__)` visible at line 665, but I need to determine if that's the relevant guard or if there's another one wrapping the function itself.

Looking at the elif chain structure, I can see WASM at line 703 comes before Linux at line 719, which means the WASM branch will take precedence when both are defined—that's the right behavior. But I still need to find what the initial `#if` is that starts this entire conditional block before the WASM elif.
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && sed -n '640,672p' src/App/ApplicationDirectories.cpp 2>&1", "description": "Read findHomePath opening if"}
tool result
        else {
            newPath = path / versionStringForPath(major, minor);
        }
        Base::Console().message("Migrating config from %s to %s\n",
                                Base::FileInfo::pathToString(path),
                                Base::FileInfo::pathToString(newPath));
        if (fs::exists(newPath)) {
            continue;  // Ignore an existing path: not an error, just a migration that was already done
        }
        fs::create_directories(newPath);
        auto  …[truncated]
Fable
thinking
I'm checking the preprocessor conditionals to verify the platform detection order—WASM gets selected before Linux on non-OpenBSD systems, which is correct, and desktop Linux falls through to the right branch. Now I'm looking at where ThumbnailSource.cpp includes the QProcess header.

The file includes QProcess unconditionally at line 29, but doesn't explicitly include QtGlobal. However, Q_OS_WASM gets defined transitively through other Qt headers like QFile and QMutex, so the guard works correctly—it's defined on WASM builds and undefined on desktop. On WASM, the force-include of FcWasmProcess.h aliases QProcess, while on desktop it's the normal implementation.

Looking at the concrete issue  …
Fable
⚙ Bash {"command": "cd /home/magik6k/lcad-wasm/freecad-port/FreeCAD && sed -n '1,50p' src/Main/MainCmd.cpp | grep -n \"include\" 2>&1", "description": "Check MainCmd includes for Interpreter/PyGILStateLocker"}
tool result
25:#include "../FCConfig.h"
28:# include <config.h>
31:#include <Build/Version.h>  // For FCCopyrightYear
33:#include <cstdio>
34:#include <ostream>
35:#include <QString>
38:#include <Base/Console.h>
39:#include <Base/Exception.h>
40:#include <Base/Interpreter.h>
43:#include <App/Application.h>
44:#include <App/ProgramInformation.h>
50:#include <emscripten.h>
Fable
thinking
Base/Interpreter.h is already included, so I have access to Base::PyGILStateLocker and can confirm freecad_run_python is safe for wasm. After reviewing the changes thoroughly, the desktop discipline looks solid—the key unguarded modifications are all behaviorally equivalent and safe, though there are a few minor wasm-correctness considerations to note. I'm organizing my findings now, starting with Application.cpp where an unguarded `if (Py_IsInitialized())` check wraps some initialization logic.

The interpreter gets initialized early in initConfig before both Application constructions, so there's no desktop regression there. However, the setupPythonTypes and LoadParameters functions now rel  …
Fable
I have completed a thorough review. Let me compile the findings. Desktop-discipline verification summary: the interpreter is initialized at `Base::Interpreter().init()` (Application.cpp:2869) inside `initConfig`, which runs before both `new Application` sites (3019, 3046), so `Py_IsInitialized()` is always true on desktop and the new constructor guard, the idempotent `setupPythonTypes`, and `initApplication`'s explicit `setupPythonTypes()` call collapse to exactly one registration — no desktop behavior change. `LoadParameters()` has only two internal callers (2930 desktop, 445 wasm-only `bootstrapEarly`), so its new idempotency guard never fires twice on desktop. The `#if defined(FC_OS_WASM)` branches correctly precede the `FC_OS_LINUX` branches everywhere (both are defined on wasm).
Fable
⚙ StructuredOutput {"findings": [{"severity": "info", "category": "ifdef-discipline", "summary": "Desktop builds verified unaffected by the Application/Interpreter/Main bootstrap changes", "detail": "All desktop-affecting unguarded changes reduce to no-ops or…
tool result
Structured output provided successfully