Skip to main content

Velox Codebase Review TODO

Static review performed on 2026-07-05.

Scope:

  • Reviewed the repository statically.
  • Focused on first-party Velox code.
  • Did not modify product code.
  • Did not run a Delphi build or runtime tests.
  • Vendor code was not treated as a normal refactoring target.

High Risk​

REST API timeout can free an action while it is still executing​

Files:

  • Classes\Service\vxManagerREST.pas
  • Classes\Service\vxActionThread.pas

Evidence:

  • TvxRESTItem.ExecuteGET starts at Classes\Service\vxManagerREST.pas:134.
  • TvxRESTItem.ExecutePOST starts at Classes\Service\vxManagerREST.pas:194.
  • Both methods wait for FIVE_MINS_MILLI.
  • On timeout they may call FreeAndNil(lThread.Action) while the worker thread can still be running.
  • TRESTActionThread.Execute starts at Classes\Service\vxActionThread.pas:626 and can still be inside FAction.Execute.
  • Local variable lAction is declared but not initialized before Assigned(lAction) checks.

Risk:

  • Use-after-free.
  • Access violations.
  • Corrupted flow/action state.
  • Database or connection cleanup running against objects still in use.

Suggested direction:

  • Replace the timeout cleanup path with cooperative cancellation.
  • Ensure the action lifetime is owned by one component.
  • Initialize locals defensively.
  • Do not free lThread.Action until the thread has actually finished.

REST endpoint dictionary is not consistently thread-safe​

File:

  • Classes\Service\vxManagerREST.pas

Evidence:

  • SyncAction adds to FEndpoints at Classes\Service\vxManagerREST.pas:431.
  • FreeOldRESTItems iterates FEndpoints at Classes\Service\vxManagerREST.pas:611.
  • FreeOldRESTItems removes from FEndpoints during that iteration at Classes\Service\vxManagerREST.pas:618.
  • Request lookup uses locking, but update/removal paths do not appear to use the same locking consistently.

Risk:

  • Race conditions while API requests are active.
  • Collection-modified exceptions.
  • Incorrect REST endpoint routing after sync.

Suggested direction:

  • Use one consistent lock for all reads and writes to FEndpoints.
  • Avoid removing dictionary items while enumerating the same dictionary.
  • Consider building a removal list, then removing after enumeration.

Flow/action cancellation uses TerminateThread​

Files:

  • Classes\Service\vxManagerExecution.pas
  • Classes\vxExecutionMonitor.pas
  • Forms\frmMain.pas
  • Forms\frmLogs.pas

Evidence:

  • TvxExecutionItem.ForcefullyTerminate calls TerminateThread at Classes\Service\vxManagerExecution.pas:477.
  • TvxExecutionMonitor.ForcefullyTerminateThreads calls TerminateThread at Classes\vxExecutionMonitor.pas:130.
  • Designer shutdown calls forceful termination at Forms\frmMain.pas:1688.
  • Log UI can forcefully terminate an execution item at Forms\frmLogs.pas:510.

Risk:

  • Locks may remain held.
  • COM state may not unwind correctly.
  • Database connections may leak or remain in an invalid state.
  • Module/action state may be partially updated.

Suggested direction:

  • Move toward proper cooperative cancellation.
  • Ensure actions and transports periodically check cancellation state.
  • Reserve hard termination only for process shutdown fallback, if it must remain at all.

Core runtime classes and methods are high complexity​

Files:

  • Classes\vxModule.pas
  • Classes\vxActionMan.pas
  • VeloxService\classMain.pas
  • VeloxAPIService\classMain.pas
  • Classes\Service\vxManagerREST.pas
  • Forms\frmSetupActions.pas

Evidence:

  • TvxModule starts at Classes\vxModule.pas:86.
  • TvxActionMan starts at Classes\vxActionMan.pas:299.
  • TvxActionMan.ExecuteOnce starts at Classes\vxActionMan.pas:1994.
  • TVeloxWorker.ServiceExecute starts at VeloxService\classMain.pas:178.
  • TVeloxWorker.OnIPCExecuteRequest starts at VeloxService\classMain.pas:449.
  • TVeloxAPIWorker.ServiceExecute starts at VeloxAPIService\classMain.pas:193.
  • TSetupActions.RefreshActionDetails starts at Forms\frmSetupActions.pas:980.
  • Forms\LinkedAction\frmSetupActions.pas is ignored by repository guidance and is not counted in active runtime/UI complexity.

Risk:

  • Changes are hard to reason about.
  • New runtime behavior can accidentally affect persistence, logging, threading, or UI behavior.
  • Testing burden is high.

Suggested direction:

  • Avoid broad rewrites.
  • When touching these areas, make narrow changes and add focused regression coverage where possible.
  • Extract behavior only when there is a clear ownership boundary.

Files:

  • Forms\frmCodeLibrary.pas

Evidence:

  • Code Library node pointer variables are declared as integer.
  • A node pointer is read with Fields[Ord(cfNodePointer)].AsInteger.
  • That integer value is later cast back to TvxCodeNode.

Risk:

  • This is a direct pointer-to-32-bit-integer-to-pointer round trip.
  • On 64-bit builds, object pointers can be truncated.
  • Failure modes include access violations, incorrect tree navigation, or memory corruption when a selected Code Library item is opened.

Suggested direction:

  • Do not store object pointers through Integer/AsInteger.
  • Prefer a real object reference owner model, a lookup key, or pointer-width-safe storage such as NativeInt where a pointer-sized cast is unavoidable.
  • Add explicit 64-bit Designer coverage for opening, searching, selecting, and refreshing Code Library nodes.

Medium Risk​

Database update version 311 dispatches version 310​

Files:

  • Classes\GUI\vxDBUpdate.pas
  • Classes\GUI\vxDBUpdateVersion.pas

Evidence:

  • TVxDBUpdate.UpdateDatabase starts at Classes\GUI\vxDBUpdate.pas:224.
  • Version 310 dispatches TVxDBUpdateVersion310 at Classes\GUI\vxDBUpdate.pas:243.
  • Version 311 also dispatches TVxDBUpdateVersion310 at Classes\GUI\vxDBUpdate.pas:244.
  • TVxDBUpdateVersion311 exists at Classes\GUI\vxDBUpdateVersion.pas:199.
  • TVxDBUpdateVersion311.Update exists at Classes\GUI\vxDBUpdateVersion.pas:1534.

Risk:

  • Future database upgrades for version 311 may not run.
  • Customers may have schema drift if that version becomes active.

Suggested direction:

  • Confirm whether version 311 should dispatch TVxDBUpdateVersion311.
  • If yes, update the dispatcher when ready.

TvxConnection.GetConfigNum is empty​

File:

  • Classes\vxCon.pas

Evidence:

  • TvxConnection.ConfigNumThreadSafe returns FConfigNum at Classes\vxCon.pas:366.
  • TvxConnection.GetConfigNum starts at Classes\vxCon.pas:452 and has an empty body.

Risk:

  • Calls through the non-thread-safe ConfigNum property may return an undefined or default value.
  • Behavior may differ depending on which accessor is used.

Suggested direction:

  • Confirm intended accessor behavior.
  • If GetConfigNum should return the same field, implement it explicitly.

WinAPI ProcessIdToSessionId declaration is not 64-bit safe​

File:

  • Classes\Tools\vxShell.pas

Evidence:

  • ProcessIdToSessionId is declared with pSessionId: DWORD.
  • GetSessionIdfromProccessId calls it with DWORD(@sessionId).
  • The Windows API expects the second parameter to be a pointer to DWORD, not a 32-bit integer containing a pointer value.

Risk:

  • On 64-bit builds, casting an address to DWORD truncates the pointer.
  • If this helper is used by service/session launching logic, the failure mode can be intermittent and machine-specific.

Suggested direction:

  • Change the declaration to use a pointer-width-safe type such as PDWORD or a var parameter.
  • Regression-test service/user-session helper paths in both 32-bit and 64-bit binaries.

Config template save result can hide earlier failures​

File:

  • Classes\vxConfigTemplate.pas

Evidence:

  • TvxConfigTemplate.SaveAllModules starts at Classes\vxConfigTemplate.pas:592.
  • The method repeatedly assigns FResult := Save....
  • Final Result := FResult occurs at Classes\vxConfigTemplate.pas:692.

Risk:

  • An early failed save can be overwritten by a later successful save.
  • Template/config export may report success when part of the save failed.

Suggested direction:

  • Accumulate failures instead of replacing the result each time.
  • Preserve enough context to identify which save step failed.

TvxRESTManager.DisableAction is an active stub​

File:

  • Classes\Service\vxManagerREST.pas

Evidence:

  • TvxRESTManager.DisableAction starts at Classes\Service\vxManagerREST.pas:705.
  • The implementation body is commented out.
  • There is no active Result assignment.

Risk:

  • If called, the method is effectively a no-op.
  • Boolean result is undefined.

Suggested direction:

  • Either implement it or remove/replace call paths after confirming usage.
  • If retained as intentionally unsupported, return an explicit value and log/raise appropriately.

VeloxService and VeloxAPIService duplicate service infrastructure​

Files:

  • VeloxService\classMain.pas
  • VeloxAPIService\classMain.pas

Evidence:

  • Both services have similar startup, shutdown, IPC, user/password, config key, test mode, and display name logic.
  • VeloxService helpers start around VeloxService\classMain.pas:58.
  • VeloxAPIService helpers start around VeloxAPIService\classMain.pas:55.

Risk:

  • Fixes can be applied to one service and missed in the other.
  • Future SSL/config changes may require duplicated work.

Suggested direction:

  • Keep behavior aligned deliberately.
  • Consider a shared service helper layer if the duplication becomes a source of defects.

Low-level ZIP and IBM MQ integration code needs 64-bit and Unicode regression coverage​

Files:

  • Classes\Tools\vxZip.pas
  • Classes\Transports\vxTransportIBMMQ.pas

Evidence:

  • vxZip.pas is a large hand-maintained ZIP implementation with historical comments about x64 support and compiler workarounds, extensive pointer arithmetic, and many ANSI/PAnsiChar Windows API calls.
  • vxTransportIBMMQ.pas uses fixed-size buffers and IBM MQ API calls; queue-manager, connection-name, and channel-name values are copied into fixed arrays before MQCONNX.

Risk:

  • These areas are close to operating-system or vendor ABI boundaries where 32/64-bit, character encoding, buffer length, and calling convention mistakes can become stability issues.
  • ZIP handling can affect file transport reliability.
  • MQ handling can affect production message delivery.
  • It is unclear from static review alone whether the MQ connection-name buffer is populated from the intended configuration value.

Suggested direction:

  • Build a focused regression set for 32-bit and 64-bit ZIP operations, including large archives and Unicode filenames.
  • Test IBM MQ connections and message send/receive flows with both 32-bit and 64-bit client libraries.
  • Treat changes here as high-risk unless backed by integration tests or a test MQ environment.

Action setup trial fork is ignored​

Files:

  • Forms\frmSetupActions.pas
  • Forms\LinkedAction\frmSetupActions.pas

Evidence:

  • Forms\frmSetupActions.pas defines the active Flow setup form.
  • Forms\LinkedAction\frmSetupActions.pas is in an ignored trial/test/backup folder per repository guidance.
  • Main action form class starts at Forms\frmSetupActions.pas:25.
  • Main RefreshActionDetails starts at Forms\frmSetupActions.pas:980.

Risk:

  • Repository search results can still find the ignored fork.

Suggested direction:

  • Treat Forms\frmSetupActions.pas as the active setup source.
  • Ignore Forms\LinkedAction\ unless a task explicitly targets trial/test/backup code.

Action and SQL connection pools use list scans​

Files:

  • Classes\vxActionPool.pas
  • Classes\vxSQLConPool.pas

Evidence:

  • TvxActionPoolItem has a comment: MAKE TDICTIONARY at Classes\vxActionPool.pas:28.
  • TvxSQLConItem has a comment: MAKE TDICTIONARY at Classes\vxSQLConPool.pas:32.
  • GetAction and related paths scan lists at Classes\vxActionPool.pas:97.
  • SQL connection lookup scans lists at Classes\vxSQLConPool.pas:101 and Classes\vxSQLConPool.pas:283.

Risk:

  • Performance may degrade as action/config/thread counts grow.
  • Lock duration may increase under service load.

Suggested direction:

  • Consider dictionary-backed lookup where identity keys are stable.
  • Keep thread ownership and lifecycle behavior unchanged when changing lookup structure.

Module loading is globally serialized​

File:

  • Classes\vxModule.pas

Evidence:

  • LoadModuleTemplate starts at Classes\vxModule.pas:1483.
  • A global critical section is acquired at Classes\vxModule.pas:1553.
  • The comment indicates this is related to ReportBuilder thread safety.

Risk:

  • Service throughput may be limited when many flows/actions load modules concurrently.
  • Long module load paths block unrelated module loads.

Suggested direction:

  • Treat this as intentional until ReportBuilder/module thread-safety is better understood.
  • If performance becomes a problem, investigate narrower locking around the unsafe component only.

Log database separation is not yet implemented​

Files:

  • Classes\vxCon.pas
  • Classes\dmSystemDB.pas

Evidence:

  • TvxConnection.SQLL returns SQLC at Classes\vxCon.pas:577.
  • Comments state that log connections currently use the active config connection.
  • This matches the known future direction to split config and log databases.

Risk:

  • Future log database work must carefully separate connection ownership, transaction behavior, and configuration.

Suggested direction:

  • Keep this documented as intentional current behavior.
  • When implemented, audit all callers of SQLL, SQLC, and active config/log database access.

Low Risk​

Large utility routines are difficult to change safely​

Files:

  • Classes\Tools\vxZip.pas
  • Classes\Tools\vxDateTools.pas
  • Classes\vxAPI.pas

Evidence:

  • CompressEx starts at Classes\Tools\vxZip.pas:665.
  • UncompressEx starts at Classes\Tools\vxZip.pas:1911.
  • vxStringtoDateTime starts at Classes\Tools\vxDateTools.pas:321.
  • vxStringtoDateTimeValid starts at Classes\Tools\vxDateTools.pas:571.
  • TvxAPI.CreateOpenAPISchema starts at Classes\vxAPI.pas:262.

Risk:

  • Isolated utility code is not necessarily a runtime problem.
  • The risk is mainly regression risk when changing these routines.

Suggested direction:

  • Add targeted tests before changing behavior.
  • Avoid refactoring unless needed for a specific defect or feature.

Ignored dead or experimental folders are documented​

Folders:

  • _BACKUP
  • Classes\_NEW

Evidence:

  • Classes\_NEW\velox_json_new\vxJSONEngine.pas exists.
  • Classes\_NEW\velox_json_new\vxFileEngineJSON.pas exists.
  • Classes\_NEW\vxFileEngineJSON.pas exists.
  • The reviewed references appeared self-contained within the experimental folder.

Risk:

  • Repository search results include inactive code.

Suggested direction:

  • Status is documented in AGENTS.md: _BACKUP, Classes\_NEW, Forms\Action, and Forms\LinkedAction can be ignored unless a task explicitly targets them.

Duplicate Code Areas​

  • Velox service startup/shutdown/helper logic in VeloxService\classMain.pas and VeloxAPIService\classMain.pas.
  • Action setup form behavior in active Forms\frmSetupActions.pas; the Forms\LinkedAction fork is ignored by repository guidance.
  • REST GET/POST execution flow in Classes\Service\vxManagerREST.pas.

Dead Code / Stub Areas​

  • Classes\Service\vxManagerREST.pas: TvxRESTManager.DisableAction.
  • Classes\_NEW: ignored by repository guidance.
  • _BACKUP: ignored by repository guidance.

Thread Safety Concerns​

  • REST endpoint dictionary locking in Classes\Service\vxManagerREST.pas.
  • REST timeout/action ownership in Classes\Service\vxManagerREST.pas and Classes\Service\vxActionThread.pas.
  • Forceful thread termination in Classes\Service\vxManagerExecution.pas and Classes\vxExecutionMonitor.pas.
  • Global module load critical section in Classes\vxModule.pas.

32-bit / 64-bit Concerns​

  • High risk: Forms\frmCodeLibrary.pas stores object pointers through 32-bit integer fields before casting them back to TvxCodeNode.
  • Medium risk: Classes\Tools\vxShell.pas passes a pointer to ProcessIdToSessionId through a DWORD.
  • Medium risk: Classes\Tools\vxZip.pas mixes pointer arithmetic, ANSI APIs, and legacy ZIP code; it needs explicit Win64 and Unicode filename coverage.
  • Medium risk: Classes\Transports\vxTransportIBMMQ.pas depends on external IBM MQ structures and client libraries; verify structure packing, string encoding, and 32/64-bit client compatibility.
  • Low risk: several UI forms store small integer indexes in TListItem.Data; these should use pointer-width-aware casts for clarity, although the reviewed cases do not appear to store real object pointers.

Performance Concerns​

  • List-based lookups in Classes\vxActionPool.pas.
  • List-based lookups in Classes\vxSQLConPool.pas.
  • Five-minute synchronous REST wait path in Classes\Service\vxManagerREST.pas.
  • Global serialized module loading in Classes\vxModule.pas.

Areas Difficult to Maintain​

  • TvxModule and module persistence/loading.
  • TvxActionMan execution orchestration.
  • Velox service startup/shutdown/IPC logic.
  • REST API execution and endpoint synchronization.
  • Designer action setup forms.
  • Config template save/import/export logic.

Open Questions​

  • Should database version 311 dispatch TVxDBUpdateVersion311 now, or is version 311 not yet active?
  • Is TvxConnection.GetConfigNum unused, or should it return FConfigNum like ConfigNumThreadSafe?
  • Which code paths currently call TvxRESTManager.DisableAction?
  • What is the desired cancellation model for long-running actions, transports, scripts, and database operations?
  • Is the Code Library expected to be fully supported in 64-bit Designer builds?
  • Which IBM MQ client bitness and version combinations are supported in production?
  • What ZIP filename encodings must be supported for customer integrations?