fix(server): implement multi-workflow mode API wiring by intel352 · Pull Request #131 · GoCodeAlone/workflow

intel352 · 2026-02-23T11:37:22Z

Summary

Replace dead TODO block in cmd/server/main.go with working multi-workflow mode implementation
The TODO listed 7 steps for PostgreSQL-backed multi-tenant mode — all underlying code already existed in store/, api/, and engine_manager.go packages
Wires up: PG connection, migrations, stores, admin bootstrap, WorkflowEngineManager, API router, and service registration
Prevents single-config path from also executing when --database-dsn is provided

Test plan

go build ./cmd/server/ compiles cleanly
go test ./cmd/server/ passes all tests
golangci-lint run passes with 0 issues
Only modifies cmd/server/main.go (1 file, 99 insertions, 29 deletions)

Closes #71

🤖 Generated with Claude Code

Replace the dead TODO block in cmd/server/main.go with a working implementation that connects to PostgreSQL via store.NewPGStore, runs database migrations, bootstraps an optional admin user on first run, starts the api.NewRouter on a dedicated port (--multi-workflow-addr, default :8090), and shuts down cleanly alongside the single-config engine. New flags: --multi-workflow-addr New imports: apihandler (api package), bcrypt, time Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Implements the previously TODO multi-workflow (PostgreSQL-backed) startup path in cmd/server/main.go, wiring database initialization and starting the REST API server when --database-dsn is provided.

Changes:

Adds multi-workflow mode wiring: PG store creation, migrations, and optional bootstrap admin creation.
Starts the multi-workflow REST API server on a dedicated address/port (--multi-workflow-addr).
Introduces WorkflowEngineManager construction for multi-workflow mode and adds related imports/flags.

Copilot · 2026-02-23T11:41:16Z

cmd/server/main.go

+		if *adminEmail != "" && *adminPassword != "" {
+			_, lookupErr := pgStore.Users().GetByEmail(context.Background(), *adminEmail)
+			if lookupErr != nil {
+				hash, hashErr := bcrypt.GenerateFromPassword([]byte(*adminPassword), bcrypt.DefaultCost)


Admin bootstrap treats any error from GetByEmail as “user not found” and attempts creation. This can mask real DB/query errors and lead to misleading logs. Please only bootstrap when errors.Is(lookupErr, evstore.ErrNotFound); otherwise handle/return the unexpected error.

cmd/server/main.go

Copilot · 2026-02-23T11:41:16Z

cmd/server/main.go

+		go func() {
+			logger.Info("multi-workflow API listening", "addr", *multiWorkflowAddr)
+			if sErr := apiServer.ListenAndServe(); sErr != nil && sErr != http.ErrServerClosed {
+				logger.Error("multi-workflow API server error", "error", sErr)
+			}
+		}()


ListenAndServe runs in a goroutine and errors are only logged. If the multi-workflow API port is already in use, startup will continue in single-config mode even though multi-workflow mode was requested. Consider failing fast on serve/bind errors (e.g., net.Listen first and exit on failure, or propagate the error via a channel).

cmd/server/main.go

Copilot · 2026-02-23T11:41:17Z

cmd/server/main.go

+		// Multi-workflow mode: connect to PostgreSQL, run migrations, start the
+		// REST API router on a dedicated port alongside the single-config engine.
+		logger.Info("Starting in multi-workflow mode",


PR description mentions mounting the API router at /api/v1 alongside existing routes, but the implementation starts a separate HTTP server on --multi-workflow-addr (dedicated port). Please align the description with the actual behavior, or adjust the implementation to mount the router into the existing server if that’s the intended UX.

Copilot · 2026-02-23T11:41:17Z

cmd/server/main.go

+		logger.Info("Starting in multi-workflow mode",
+			"database_dsn_set", true,
 			"jwt_secret_set", *jwtSecret != "",
 			"admin_email_set", *adminEmail != "",
+			"api_addr", *multiWorkflowAddr,
 		)


Multi-workflow mode starts the REST API even when --jwt-secret is empty. In this codebase the API signs/validates JWTs with the configured HMAC secret; an empty secret makes tokens forgeable. Please fail fast when *jwtSecret is empty (or generate a secure random secret explicitly for dev-only) before starting the API server.

cmd/server/main.go

intel352 · 2026-02-23T14:11:11Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2026-02-23T14:11:20Z

@intel352 I've opened a new pull request, #143, to work on those changes. Once the pull request is ready, I'll request review from you.

* Initial plan * fix(server): address review comments on multi-workflow mode wiring Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

Copilot

Pull request overview

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

Comments suppressed due to low confidence (2)

api/workflow_handler.go:286

Deploy calls h.engine.DeployWorkflow(...) and then independently updates the workflow status in the store. If the injected engine is workflow.WorkflowEngineManager, it already updates the workflow’s DB status internally, so this causes duplicate updates/version bumps and can create inconsistencies if one update succeeds and the other fails. Prefer a single source of truth: either let the engine runner own the status update and re-fetch/return the record, or remove the engine manager’s internal store mutation and keep the handler update.

	if h.engine != nil {
		if err := h.engine.DeployWorkflow(r.Context(), id); err != nil {
			WriteError(w, http.StatusInternalServerError, "failed to deploy workflow engine")
			return
		}
	}
	wf.Status = store.WorkflowStatusActive
	wf.UpdatedAt = time.Now()
	if err := h.workflows.Update(r.Context(), wf); err != nil {
		WriteError(w, http.StatusInternalServerError, "internal error")

api/workflow_handler.go:317

Stop has the same double-write issue as Deploy: calling the engine runner and then updating the workflow status in the handler can lead to duplicate DB updates/version bumps and mismatched state if one step fails. Consolidate status persistence into either the engine runner or the handler (and return the store’s canonical record).

	if h.engine != nil {
		if err := h.engine.StopWorkflow(r.Context(), id); err != nil {
			WriteError(w, http.StatusInternalServerError, "failed to stop workflow engine")
			return
		}
	}
	wf.Status = store.WorkflowStatusStopped
	wf.UpdatedAt = time.Now()
	if err := h.workflows.Update(r.Context(), wf); err != nil {
		WriteError(w, http.StatusInternalServerError, "internal error")

Copilot · 2026-02-23T16:27:18Z

cmd/server/main.go

 		secret = "dev-secret-change-me"
 		logger.Error("No JWT secret configured — using insecure default; set JWT_SECRET env var or -jwt-secret flag")


runMultiWorkflow still exists and is no longer referenced from main(), but it’s being kept partially updated (e.g., switching to apihandler.*). This risks the inlined multi-workflow startup logic and runMultiWorkflow drifting further apart (and runMultiWorkflow currently allows an insecure default JWT secret). Consider deleting runMultiWorkflow or refactoring main() to call it so there’s a single implementation to maintain.

Suggested change

secret = "dev-secret-change-me"

logger.Error("No JWT secret configured — using insecure default; set JWT_SECRET env var or -jwt-secret flag")

log.Fatal("No JWT secret configured — refusing to start; set JWT_SECRET env var or -jwt-secret flag")

Copilot · 2026-02-23T16:27:19Z

cmd/server/main.go

 	"encoding/json"
+	"errors"
 	"flag"
 	"errors"


The import block includes errors twice, which will make this file fail to compile (imported and not used/duplicate import). Remove the duplicate and keep a single errors import.

Suggested change

"errors"

Copilot · 2026-02-23T16:27:19Z

cmd/server/main.go

+		if *adminEmail != "" && *adminPassword != "" {
+			_, lookupErr := pgStore.Users().GetByEmail(context.Background(), *adminEmail)
+			switch {
+			case errors.Is(lookupErr, evstore.ErrNotFound):


Admin bootstrap looks up the user with context.Background() and uses the raw --admin-email value. Consider reusing the existing timed context (dbCtx) and normalizing the email (trim + lower-case) to match the auth handlers’ behavior, otherwise the bootstrap user may not be able to log in if the stored email casing differs.

Copilot · 2026-02-23T16:27:19Z

cmd/server/main.go

+				if createErr := pgStore.Users().Create(context.Background(), adminUser); createErr != nil {
+					logger.Warn("multi-workflow mode: failed to create admin user (may already exist)", "error", createErr)


Admin bootstrap creates the user with context.Background(), so these DB operations ignore the connection/migration timeout and any shutdown cancellation. Use the same bounded context used for startup (or a derived context) so a hung DB call can’t block shutdown indefinitely.

Copilot · 2026-02-23T16:27:20Z

api/router.go


 	// OAuth providers keyed by provider name (e.g. "google", "okta").
 	OAuthProviders map[string]*OAuthProviderConfig
+
+	// Engine is an optional engine lifecycle manager used by the workflow
+	// deploy/stop endpoints to actually start and stop workflow engines.
+	Engine EngineRunner
 }


The PR description says only cmd/server/main.go is modified, but this PR also changes the api/ package (router + workflow handler). Please update the PR description/test plan to reflect the additional files/behavior being changed, since it impacts review scope and release notes.

Copilot · 2026-02-23T16:27:20Z

api/workflow_handler.go

+	if h.engine != nil {
+		if err := h.engine.DeployWorkflow(r.Context(), id); err != nil {
+			WriteError(w, http.StatusInternalServerError, "failed to deploy workflow engine")
+			return
+		}
+	}


New behavior when h.engine != nil (calling DeployWorkflow/StopWorkflow and handling errors) isn’t covered by existing tests in api/workflow_handler_test.go (which only exercises the nil-engine path). Add tests for the engine-runner path, including success and failure cases, to prevent regressions and to validate HTTP status mapping.

Copilot AI review requested due to automatic review settings February 23, 2026 11:37

Copilot started reviewing on behalf of intel352 February 23, 2026 11:37 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 23, 2026

fix(server): harden multi-workflow mode startup and wiring #143

Merged

Copilot AI and others added 2 commits February 23, 2026 09:33

Merge branch 'main' into fix/issue-71-dead-api-wiring

20e5069

Copilot AI review requested due to automatic review settings February 23, 2026 16:21

intel352 merged commit 23b07f8 into main Feb 23, 2026
5 checks passed

intel352 deleted the fix/issue-71-dead-api-wiring branch February 23, 2026 16:22

Copilot started reviewing on behalf of intel352 February 23, 2026 16:22 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

		secret = "dev-secret-change-me"
		logger.Error("No JWT secret configured — using insecure default; set JWT_SECRET env var or -jwt-secret flag")

	secret = "dev-secret-change-me"
	logger.Error("No JWT secret configured — using insecure default; set JWT_SECRET env var or -jwt-secret flag")
	log.Fatal("No JWT secret configured — refusing to start; set JWT_SECRET env var or -jwt-secret flag")

		if createErr := pgStore.Users().Create(context.Background(), adminUser); createErr != nil {
		logger.Warn("multi-workflow mode: failed to create admin user (may already exist)", "error", createErr)

Conversation

intel352 commented Feb 23, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

intel352 commented Feb 23, 2026

Uh oh!

Copilot AI commented Feb 23, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants