Skip to content

feat: add browser XGo executor#3344

Draft
nighca wants to merge 5 commits into
goplus:devfrom
nighca:issue-3341
Draft

feat: add browser XGo executor#3344
nighca wants to merge 5 commits into
goplus:devfrom
nighca:issue-3341

Conversation

@nighca

@nighca nighca commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Closes #3341

Summary

  • add a browser-side XGo executor backed by ixgo and xgobuild
  • isolate every executor in its own Web Worker/WASM runtime so separate executors can run concurrently
  • add a JSON request/response capability bridge with synchronous and asynchronous frontend handlers
  • add a tutorial class framework and keep its implementation separate from its xgoexec binding
  • add a debug page covering plain XGo, lifecycle events, and tutorial UI capabilities

Tutorial capability exploration

  • showMessage: output with a UI side effect
  • readCode: asynchronous input read from the UI
  • waitForSubmit: waits for a user submission event
  • setProgress: structured output used to update UI state

Validation

  • go test ./... in tools/xgoexec
  • pnpm run lint
  • pnpm run type-check
  • pnpm run test -- --run (717 tests)
  • pnpm run build
  • browser validation for concurrent executors, asynchronous capability results, submission events, immediate completion, and stopping while a capability is pending

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a generic XGo executor (xgoexec) compiled to WebAssembly to build and run XGo/Go+ programs (including a tutorial framework) in the browser. It adds a build script, Go files for the executor and tutorial packages, a TypeScript wrapper XGoExecutor to interface with the WASM module, and a debug page with routing to test the executor. The review feedback highlights several critical issues, including swallowed asynchronous runtime errors, a compilation/interpretation error due to a missing MainEntry method on the Course struct, potential race conditions when waiting for WASM initialization, and data races caused by not cancelling previous executions during subsequent build or run operations.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread spx-gui/src/utils/xgo-executor.ts Outdated
Comment on lines +24 to +25
const errors: string[] = []
this.target.xbuilder_xgoexec_error = (phase, message) => errors.push(`${phase}: ${message}`)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Asynchronous runtime errors (such as panics or interpreter errors) are reported via xbuilder_xgoexec_error after XGoExecutor.run() has already resolved. Currently, these errors are pushed to a local errors array inside run(), which is no longer accessible. This causes all runtime errors to be silently swallowed, making debugging extremely difficult.

We should at least log these errors to console.error or provide an onError callback to handle them properly.

Suggested change
const errors: string[] = []
this.target.xbuilder_xgoexec_error = (phase, message) => errors.push(`${phase}: ${message}`)
const errors: string[] = []
this.target.xbuilder_xgoexec_error = (phase, message) => {
if (phase === 'runtime') {
console.error('[XGo Runtime Error] ' + message)
} else {
errors.push(phase + ': ' + message)
}
}

Comment thread tools/xgoexec/main.go Outdated
Comment on lines +77 to +79
if state.framework == "tutorial" {
source = []byte(strings.Replace(string(source), "new(Course).Main()", "new(Course).MainEntry()", 1))
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

In tools/xgoexec/tutorial/tutorial.go, the Course struct has a Main() method but does NOT have a MainEntry() method. Replacing new(Course).Main() with new(Course).MainEntry() here will result in a compilation/interpretation error because MainEntry is undefined on Course.

If the intention was to invoke the base class's entry point, MainEntry must be defined in tutorial.go. Otherwise, if Main is the correct entry point, this replacement should be removed.

Comment thread spx-gui/src/utils/xgo-executor.ts Outdated
Comment on lines +31 to +32
await new Promise((resolve) => window.setTimeout(resolve, 0))
this.target.xbuilder_xgoexec_configure(this.framework)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The window.setTimeout(resolve, 0) call is used to yield control to the event loop, hoping that the Go WASM runtime completes its initialization and registers the global functions. However, this is highly prone to race conditions. If the browser is under load or the WASM initialization takes longer, xbuilder_xgoexec_configure might still be undefined when called, leading to runtime crashes.

A more robust approach is to poll for the existence of the global function with a timeout.

    const start = Date.now()
    while (typeof this.target.xbuilder_xgoexec_configure !== 'function') {
      if (Date.now() - start > 5000) {
        throw new Error('Timeout waiting for XGo executor to initialize')
      }
      await new Promise((resolve) => window.setTimeout(resolve, 10))
    }
    this.target.xbuilder_xgoexec_configure(this.framework)

Comment thread tools/xgoexec/main.go
Comment on lines +92 to +110
func run(this js.Value, args []js.Value) any {
state.Lock()
if state.ctx == nil || state.interp == nil {
state.Unlock()
return jsError("not built")
}
ctx, interp := state.ctx, state.interp
runCtx, cancel := context.WithCancel(context.Background())
state.cancel = cancel
state.Unlock()
go func() {
ctx.RunContext = runCtx
_, err := ctx.RunInterp(interp, "main.go", nil)
if err != nil && runCtx.Err() == nil {
js.Global().Call("xbuilder_xgoexec_error", "runtime", err.Error())
}
}()
return nil
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If run is called while a previous execution is still running, the previous execution is not cancelled or stopped. Instead, state.cancel is simply overwritten with the new cancel function. This leaves the previous interpreter running in the background, leading to concurrent executions on the same shared ixgo.Context, which is not thread-safe and will cause data races, undefined behavior, or panics.

We should ensure that any active execution is cancelled before starting a new one, and set ctx.RunContext inside the lock to avoid data races.

func run(this js.Value, args []js.Value) any {
	state.Lock()
	if state.ctx == nil || state.interp == nil {
		state.Unlock()
		return jsError("not built")
	}
	if state.cancel != nil {
		state.cancel()
	}
	ctx, interp := state.ctx, state.interp
	runCtx, cancel := context.WithCancel(context.Background())
	state.cancel = cancel
	ctx.RunContext = runCtx
	state.Unlock()
	go func() {
		_, err := ctx.RunInterp(interp, "main.go", nil)
		if err != nil && runCtx.Err() == nil {
			js.Global().Call("xbuilder_xgoexec_error", "runtime", err.Error())
		}
	}()
	return nil
}

Comment thread tools/xgoexec/main.go Outdated
Comment on lines +68 to +72
state.Lock()
defer state.Unlock()
if state.ctx == nil {
return jsError("not configured")
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

When building a new program, we should also cancel any currently running execution to prevent concurrent build and run operations on the same shared ixgo.Context.

Suggested change
state.Lock()
defer state.Unlock()
if state.ctx == nil {
return jsError("not configured")
}
state.Lock()
defer state.Unlock()
if state.ctx == nil {
return jsError("not configured")
}
if state.cancel != nil {
state.cancel()
state.cancel = nil
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a frontend module for running XGo code in the browser

1 participant