refactor: migrate executor handlers to free functions with ExecContext

[retran]

Summary

Migrates ~450 Executor receiver methods to free functions receiving *ExecContext, completing the handler migration phase of the executor refactoring.

Why free functions?

Receiver methods on *Executor have implicit access to all 14 struct fields (reader, writer, cache, etc.), making the dependency surface invisible and preventing backend substitution. Free functions can only access what *ExecContext provides — this makes dependencies explicit, enables MockBackend-based testing without a .mpr file, and is a prerequisite for the next MR where handlers switch from e.reader.ListModules() to ctx.Backend.ListModules() through domain-specific interfaces. The migration itself is mechanical (no behavioral changes) but necessary to decouple handlers from the concrete Executor struct.

Changes

• Convert all handler methods from (e *Executor) receivers to func name(ctx *ExecContext, ...) free functions
• Update all 115 handler registrations in register_stubs.go to call free functions directly (zero ctx.executor. indirection)
• Remove 233 unused Executor wrapper methods; 62 remain (lifecycle API, exported wrappers for external callers, test-called wrappers)
• Add TODO for ExecContext.Fork() in captureDescribe (parallel describe operations)

Stacked on

• feat: dispatch registry, backend interfaces, and MockBackend #224 (dispatch registry + backend interfaces) — stacked on feat: add typed error system for executor #222 (typed errors)

Structure

15 incremental commits, each compiles independently:

1. Signature change (StmtHandler → *ExecContext)
   2-10. Domain-by-domain handler migration (connection, module, enum/constant, entity, microflow, page, security, nav/settings/image/workflow, services, mapping/java)
2. Cross-cutting handlers (move/rename, styling, diff, structure, mermaid, contract, context, search, lint, SQL, import, fragments, misc)
3. Core utilities (helpers, format, hierarchy, autocomplete, validate, catalog, query)
4. Cleanup (remove 233 wrappers, fix remaining e.method() calls)
5. TODO comment for ExecContext.Fork()
6. Review feedback fixes

Testing

• make build — clean
• make test — all pass
• make lint — Go lint passes
• No behavioral changes — pure mechanical refactoring

[github-actions]

AI Code Review

What Looks Good

• Clean refactor execution: The PR successfully migrates ~450 Executor receiver methods to free functions taking *ExecContext as specified, completing the handler migration phase.
• Incremental approach: Broken into 15 commits that each compile independently, making review and bisection easier.
• Backward compatibility preserved: Maintains 62 Executor wrapper methods for external API compatibility while removing 233 unused ones.
• Consistent application: The refactor pattern is applied uniformly - converting methods to free functions, updating call sites to use direct free function calls or ctx.executor. for not-yet-migrated methods.
• Test integrity: All tests pass (make test), build succeeds, and lint passes with no behavioral changes reported.
• Proper encapsulation: Access to executor state is properly mediated through ctx.executor in the converted functions.
• Utility function alignment: Companion functions like getHierarchy() and writeResult() are appropriately updated to take *ExecContext where needed.
• Future-proofing: Added TODO for ExecContext.Fork() in captureDescribe shows consideration for parallel operations.

Recommendation

Approve - This is a well-executed, pure mechanical refactor that successfully completes the handler migration phase without changing behavior. The approach is consistent, thoroughly tested, and maintains all necessary backward compatibility while improving internal architecture. No issues were found against the review checklist.

---

Automated review via OpenRouter (Nemotron Super 120B) — workflow source

[Copilot]

Pull request overview

This PR completes the “handler migration” phase of the executor refactor by converting executor statement handlers from (*Executor) receiver methods to package-level functions that accept *ExecContext, and wiring dispatch/registrations to use the new handler signature.

Changes:

• Update StmtHandler and registry dispatch to pass *ExecContext instead of *Executor.
• Introduce Executor.newExecContext(ctx) and migrate many handlers/utilities to use ctx.Output, ctx.Format, ctx.Cache, etc.
• Add transitional Executor wrapper methods to keep unmigrated callers working during the incremental migration.

Reviewed changes

Copilot reviewed 86 out of 86 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
mdl/executor/registry_test.go	Update tests to use the new StmtHandler(ctx *ExecContext, ...) signature.
mdl/executor/registry.go	Change handler type + dispatch to accept *ExecContext.
mdl/executor/oql_type_inference.go	Convert OQL inference/validation helpers to *ExecContext-based free functions.
mdl/executor/hierarchy.go	Move hierarchy caching/invalidation to ExecContext cache; add temporary wrappers.
mdl/executor/format.go	Route output/format through ExecContext; add compatibility wrappers.
mdl/executor/executor_dispatch.go	Construct ExecContext per dispatch and call registry with it.
mdl/executor/executor_connect.go	Migrate connect/disconnect/status handlers to *ExecContext.
mdl/executor/executor.go	Pass the execution context.Context into executeInner.
mdl/executor/exec_context.go	Add a temporary back-reference from ExecContext to Executor during migration.
mdl/executor/cmd_workflows_write.go	Migrate workflow write handlers and related helpers to *ExecContext.
mdl/executor/cmd_workflows.go	Migrate workflow show/describe helpers to *ExecContext.
mdl/executor/cmd_snippets.go	Migrate snippet listing output to *ExecContext.
mdl/executor/cmd_settings.go	Migrate settings show/describe/alter helpers to *ExecContext.
mdl/executor/cmd_search.go	Migrate catalog-backed search/show commands to *ExecContext.
mdl/executor/cmd_rest_clients.go	Migrate REST client show/describe/create/drop helpers to *ExecContext.
mdl/executor/cmd_rename.go	Migrate rename entrypoint + helpers to *ExecContext and context output.
mdl/executor/cmd_published_rest.go	Migrate published REST show/describe/create/alter/drop to *ExecContext.
mdl/executor/cmd_pages_show.go	Migrate page listing to *ExecContext.
mdl/executor/cmd_pages_create_v3.go	Migrate V3 page/snippet creation to *ExecContext.
mdl/executor/cmd_pages_builder.go	Convert module ID/name helpers and drop handlers to *ExecContext + wrappers.
mdl/executor/cmd_page_wireframe.go	Migrate wireframe JSON generation to *ExecContext + wrappers.
mdl/executor/cmd_oql_plan.go	Migrate OQL plan ELK output to *ExecContext.
mdl/executor/cmd_navigation.go	Migrate navigation show/describe/alter output to *ExecContext.
mdl/executor/cmd_module_overview.go	Migrate module overview graph generation to *ExecContext + wrapper.
mdl/executor/cmd_misc.go	Migrate misc commands (update/help/version/exit/script) to *ExecContext.
mdl/executor/cmd_microflows_format_action.go	Migrate microflow formatting helpers to *ExecContext + wrappers.
mdl/executor/cmd_microflows_drop.go	Migrate microflow drop handler output/cache invalidation to *ExecContext.
mdl/executor/cmd_microflows_create.go	Migrate microflow create handler and helpers to *ExecContext.
mdl/executor/cmd_microflow_elk.go	Migrate microflow ELK generation to *ExecContext + wrapper.
mdl/executor/cmd_mermaid.go	Migrate Mermaid describe generation to *ExecContext + external wrapper.
mdl/executor/cmd_lint.go	Migrate lint command to *ExecContext and avoid name collision with lint context.
mdl/executor/cmd_layouts.go	Migrate layout listing output to *ExecContext.
mdl/executor/cmd_languages.go	Migrate language listing output to *ExecContext.
mdl/executor/cmd_jsonstructures.go	Migrate JSON structure show/describe/create/drop to *ExecContext.
mdl/executor/cmd_javascript_actions.go	Migrate JS action show/describe and refactor source reader to pure function.
mdl/executor/cmd_javaactions.go	Migrate Java action show/describe/create/drop and refactor source reader to pure function.
mdl/executor/cmd_import_mappings.go	Migrate import mapping show/describe/create/drop; make element printer io.Writer-based.
mdl/executor/cmd_import.go	Migrate import handler and link resolution to *ExecContext; thread DB context into helpers.
mdl/executor/cmd_imagecollections.go	Migrate image collection show/describe/create/drop/find to *ExecContext.
mdl/executor/cmd_fragments.go	Migrate fragment define/show/describe/describe-from to *ExecContext with shared session map.
mdl/executor/cmd_folders.go	Migrate folder drop/move and folder lookup to *ExecContext.
mdl/executor/cmd_features.go	Migrate feature gating + show features output to *ExecContext.
mdl/executor/cmd_export_mappings.go	Migrate export mapping show/describe/create/drop; make element printer io.Writer-based.
mdl/executor/cmd_enumerations.go	Migrate enumeration show/describe/create/drop/find to *ExecContext; adjust validation tables.
mdl/executor/cmd_entities_describe.go	Migrate entity show/describe helpers and related lookups to *ExecContext.
mdl/executor/cmd_entities_access.go	Migrate entity access grant output/formatting to *ExecContext.
mdl/executor/cmd_domainmodel_elk.go	Migrate domain model ELK generation to *ExecContext + wrapper.
mdl/executor/cmd_diff_output.go	Migrate diff renderers to *ExecContext output.
mdl/executor/cmd_dbconnection.go	Migrate DB connection show/describe/create and constant default resolution to *ExecContext.
mdl/executor/cmd_datatransformer.go	Migrate data transformer list/describe/create/drop to *ExecContext.
mdl/executor/cmd_constants.go	Migrate constants show/describe/create/drop and MDL output to *ExecContext + wrapper.
mdl/executor/cmd_businessevents.go	Migrate business events show/describe/create/drop to *ExecContext.
mdl/executor/cmd_alter_workflow.go	Migrate workflow alter entrypoint to *ExecContext; convert helper methods to free funcs.
mdl/executor/cmd_alter_page.go	Migrate page/snippet alter entrypoint + widget build helpers to *ExecContext.
mdl/executor/cmd_agenteditor_models.go	Migrate agent editor model show/describe/find to *ExecContext.
mdl/executor/cmd_agenteditor_mcpservices.go	Migrate agent editor MCP service show/describe/find to *ExecContext.
mdl/executor/cmd_agenteditor_kbs.go	Migrate agent editor knowledge base show/describe/find to *ExecContext.
mdl/executor/cmd_agenteditor_agents.go	Migrate agent editor agent show/describe/find and emitters to *ExecContext.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 86 out of 86 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

mdl/executor/cmd_oql_plan.go:479

• The file ends with a "Executor method wrapper for backward compatibility" marker, but no wrapper method is actually implemented. Since OqlQueryPlanELK was previously an Executor receiver method, this silently removes that API; either add the missing (*Executor) OqlQueryPlanELK(...) wrapper delegating to the new free function, or remove the compatibility comment and confirm the API break is intentional.
  mdl/executor/cmd_languages.go:53
• There is a trailing "Executor method wrapper for backward compatibility" section marker but no wrapper implementation following it. If no wrapper is needed, please remove this comment to avoid confusion about expected backwards-compatibility surface.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ako]

Review

Overall: mechanically correct, CI green, but this is the third PR in a stack with zero proof that the end-state architecture works. The migration itself is clean — 300 functions transformed with the same pattern, 15 commits that each compile independently. No behavioral changes. But no handler actually uses ctx.Backend yet.

What I like

• No go fmt noise. First PR in this series without unrelated whitespace changes. Thank you.
• ExecContext is well-designed. Embeds context.Context, carries Backend, Output, Format, Cache, Catalog, Fragments, Logger. Clean dependency bag. The executor back-reference is honestly documented as temporary.
• 15 incremental commits that each compile. Domain-by-domain migration makes bisection easy and review tractable despite the 3800-line diff.
• Remaining 41 Executor methods are justified. Public API for external callers (GetModuleNames, Search, DiffProgram), lifecycle (executeInner, newExecContext), formatters (microflow graph traversal). These genuinely need the Executor struct.
• Handler registrations are direct calls — execCreateEntity(ctx, stmt.(*ast.CreateEntityStmt)) with no intermediate indirection through ctx.executor.execCreateEntity(). That's the right pattern.

Concerns

1. 300× e := ctx.executor is the entire PR. Every migrated handler does:

   func execCreateEntity(ctx *ExecContext, s *ast.CreateEntityStmt) error {
       e := ctx.executor
       // ... body unchanged, still uses e.reader/e.writer ...
   }

   This is structurally correct as a phased migration, but means the PR delivers zero decoupling benefit on its own. The value materializes only when the NEXT PR replaces e.reader.ListModules() with ctx.Backend.ListModules().

2. Three PRs deep, zero ctx.Backend usage. feat: add typed error system for executor #222 (typed errors) → feat: dispatch registry, backend interfaces, and MockBackend #224 (backend interfaces + registry) → refactor: migrate executor handlers to free functions with ExecContext #225 (ExecContext migration). Across all three, ctx.Backend is defined but never read by any handler. MockBackend exists but is never instantiated in a test. The full investment is ~10,000 lines of infrastructure with no proof-of-concept that the target architecture actually works for one real handler.

   Strong request: the next PR in this stack must wire ctx.Backend into at least 5-10 handlers from different domains (e.g., one entity handler, one microflow handler, one security handler) AND have a test that uses MockBackend. That would validate: (a) the 22 sub-interfaces are the right granularity, (b) the e := ctx.executor pattern can actually be removed, (c) MockBackend is usable. Without that, the stack remains faith-based.

3. StmtHandler signature changed between PRs. In feat: dispatch registry, backend interfaces, and MockBackend #224: func(e *Executor, stmt ast.Statement) error. In refactor: migrate executor handlers to free functions with ExecContext #225: func(ctx *ExecContext, stmt ast.Statement) error. If someone builds on feat: dispatch registry, backend interfaces, and MockBackend #224's API before refactor: migrate executor handlers to free functions with ExecContext #225 lands, their code breaks. This is fine for a stacked PR workflow, but worth noting that feat: dispatch registry, backend interfaces, and MockBackend #224 and refactor: migrate executor handlers to free functions with ExecContext #225 should land together or feat: dispatch registry, backend interfaces, and MockBackend #224's StmtHandler should use *ExecContext from the start.

4. captureDescribe concurrency note. The code has a comment "NOT safe for concurrent use — use captureDescribeParallel instead" and a TODO: ExecContext.Fork(). Worth tracking — if Fork() deep-copies the context, the executor back-reference becomes a shared-state problem. Once executor is removed (after Backend migration), this resolves naturally.

Minor

• Quiet field on ExecContext is only used for connection messages. Could be folded into Output (use io.Discard when quiet). Not urgent.
• Some helper functions like writeResult, writeDescribeJSON, getHierarchy remain as Executor methods but are called by free-function handlers via ctx.executor.writeResult(...). These should eventually migrate to free functions too, or become ExecContext methods.

Verdict

Approve. The migration is mechanical and correct, CI is green, and the incremental commit structure is exemplary. But this PR + #224 + #222 form a three-PR stack of infrastructure prep. The next PR in the stack must include concrete Backend usage — without it, the architecture is untested and the 22-interface design is a guess. I won't approve a fourth infrastructure-only PR in this stack.