From 4bcfc64e25d69c2a0f6f53da099ede92618ede1d Mon Sep 17 00:00:00 2001 From: rabbitblood Date: Sun, 26 Apr 2026 11:07:00 -0700 Subject: [PATCH] chore(simplify): drop verbose comments + introduce DefaultMaxConcurrentTasks const Simplify pass on top of the wire-up commit: - New const models.DefaultMaxConcurrentTasks = 1; handlers and tests reference the symbol so the schema-default mirror lives in one place. - Strip 5 multi-line comments that narrated what the code does. - Drop the duplicate field-rationale on OrgWorkspace; the one on CreateWorkspacePayload is canonical. - Drop test-side positional comments that would silently lie if columns get reordered. Pure cleanup; no behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/handlers/handlers_additional_test.go | 12 ++++-------- workspace-server/internal/handlers/handlers_test.go | 4 +--- workspace-server/internal/handlers/org.go | 8 +------- workspace-server/internal/handlers/org_import.go | 5 +---- workspace-server/internal/handlers/workspace.go | 5 +---- workspace-server/internal/models/workspace.go | 13 ++++++++----- 6 files changed, 16 insertions(+), 31 deletions(-) diff --git a/workspace-server/internal/handlers/handlers_additional_test.go b/workspace-server/internal/handlers/handlers_additional_test.go index aa31930a..ca3df0de 100644 --- a/workspace-server/internal/handlers/handlers_additional_test.go +++ b/workspace-server/internal/handlers/handlers_additional_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/DATA-DOG/go-sqlmock" + "github.com/Molecule-AI/molecule-monorepo/platform/internal/models" "github.com/gin-gonic/gin" ) @@ -31,7 +32,7 @@ func TestWorkspaceCreate_WithParentID(t *testing.T) { mock.ExpectBegin() // Default tier is 3 (Privileged) — see workspace.go create-handler comment. mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Child Agent", nil, 3, "langgraph", sqlmock.AnyArg(), &parentID, nil, "none", (*int64)(nil), 1). + WithArgs(sqlmock.AnyArg(), "Child Agent", nil, 3, "langgraph", sqlmock.AnyArg(), &parentID, nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO canvas_layouts"). @@ -66,7 +67,7 @@ func TestWorkspaceCreate_ExplicitClaudeCodeRuntime(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "CC Agent", nil, 2, "claude-code", sqlmock.AnyArg(), (*string)(nil), nil, "none", (*int64)(nil), 1). + WithArgs(sqlmock.AnyArg(), "CC Agent", nil, 2, "claude-code", sqlmock.AnyArg(), (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO canvas_layouts"). @@ -277,12 +278,8 @@ func TestWorkspaceList_WithData(t *testing.T) { } } -// ---------- workspace.go: Create with explicit max_concurrent_tasks (#1408) ---------- +// ---------- workspace.go: Create with explicit max_concurrent_tasks ---------- -// Locks the wire-up that lets a leader workspace's template config.yaml -// (or a direct API caller) set max_concurrent_tasks > 1 so an in-flight -// cron doesn't reject incoming A2A delegations. Schema default is 1; a -// non-zero payload value must reach the INSERT verbatim. func TestWorkspaceCreate_MaxConcurrentTasksOverride(t *testing.T) { mock := setupTestDB(t) setupTestRedis(t) @@ -290,7 +287,6 @@ func TestWorkspaceCreate_MaxConcurrentTasksOverride(t *testing.T) { handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir()) mock.ExpectBegin() - // 11th arg = max_concurrent_tasks; payload says 3, must propagate. mock.ExpectExec("INSERT INTO workspaces"). WithArgs(sqlmock.AnyArg(), "Leader Agent", nil, 3, "claude-code", sqlmock.AnyArg(), (*string)(nil), nil, "none", (*int64)(nil), 3). WillReturnResult(sqlmock.NewResult(0, 1)) diff --git a/workspace-server/internal/handlers/handlers_test.go b/workspace-server/internal/handlers/handlers_test.go index 83a91e6c..be1fd1c8 100644 --- a/workspace-server/internal/handlers/handlers_test.go +++ b/workspace-server/internal/handlers/handlers_test.go @@ -290,10 +290,8 @@ func TestWorkspaceCreate(t *testing.T) { // Expect workspace INSERT (uuid is dynamic, use AnyArg for id, runtime, awareness_namespace). // Default tier is 3 (Privileged) — see workspace.go create-handler comment. - // 11th arg: max_concurrent_tasks defaults to 1 when payload omits it - // (see workspace.go Create handler — schema default mirror, #1408). mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Test Agent", nil, 3, "langgraph", sqlmock.AnyArg(), (*string)(nil), nil, "none", (*int64)(nil), 1). + WithArgs(sqlmock.AnyArg(), "Test Agent", nil, 3, "langgraph", sqlmock.AnyArg(), (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks). WillReturnResult(sqlmock.NewResult(0, 1)) // Expect transaction commit (no secrets in this payload) diff --git a/workspace-server/internal/handlers/org.go b/workspace-server/internal/handlers/org.go index 79d8a016..ed216bfe 100644 --- a/workspace-server/internal/handlers/org.go +++ b/workspace-server/internal/handlers/org.go @@ -287,13 +287,7 @@ type OrgWorkspace struct { // InitialMemories are memories seeded into this workspace at creation // time. If empty, defaults.initial_memories are used. Issue #1050. InitialMemories []models.MemorySeed `yaml:"initial_memories" json:"initial_memories"` - // MaxConcurrentTasks lets a leader workspace handle multiple A2A - // messages + cron fires concurrently (#1408). Default 1 keeps the - // classic worker-style serialised execution; bump to 3 for leaders - // (PM, leads with 5-min orchestrator pulses) so an in-flight cron - // doesn't reject incoming A2A delegations. Wired into the workspaces - // table at template-import time + read by the scheduler's capacity - // check (workspace-server/internal/scheduler/scheduler.go). + // MaxConcurrentTasks: see models.CreateWorkspacePayload. MaxConcurrentTasks int `yaml:"max_concurrent_tasks" json:"max_concurrent_tasks"` Schedules []OrgSchedule `yaml:"schedules" json:"schedules"` Channels []OrgChannel `yaml:"channels" json:"channels"` diff --git a/workspace-server/internal/handlers/org_import.go b/workspace-server/internal/handlers/org_import.go index 8bcb5777..c91b9294 100644 --- a/workspace-server/internal/handlers/org_import.go +++ b/workspace-server/internal/handlers/org_import.go @@ -103,12 +103,9 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX // (see canvas-topology.ts), so imports don't spray the viewport. initialCollapsed := false - // max_concurrent_tasks from org template (#1408). 0/omitted → schema - // default of 1. Leaders typically set 3 in their org.yaml so an - // in-flight cron doesn't reject incoming A2A delegations. maxConcurrent := ws.MaxConcurrentTasks if maxConcurrent <= 0 { - maxConcurrent = 1 + maxConcurrent = models.DefaultMaxConcurrentTasks } _, err := db.DB.ExecContext(ctx, ` INSERT INTO workspaces (id, name, role, tier, runtime, awareness_namespace, status, parent_id, workspace_dir, workspace_access, max_concurrent_tasks) diff --git a/workspace-server/internal/handlers/workspace.go b/workspace-server/internal/handlers/workspace.go index 9d041316..87676b55 100644 --- a/workspace-server/internal/handlers/workspace.go +++ b/workspace-server/internal/handlers/workspace.go @@ -210,12 +210,9 @@ func (h *WorkspaceHandler) Create(c *gin.Context) { return } - // max_concurrent_tasks: payload-supplied → write as-is; 0/omitted → - // fall back to the schema default (1). Single INSERT shape regardless, - // avoids a forked column list. (#1408) maxConcurrent := payload.MaxConcurrentTasks if maxConcurrent <= 0 { - maxConcurrent = 1 + maxConcurrent = models.DefaultMaxConcurrentTasks } // Insert workspace with runtime persisted in DB (inside transaction) _, err := tx.ExecContext(ctx, ` diff --git a/workspace-server/internal/models/workspace.go b/workspace-server/internal/models/workspace.go index da71db48..5d1d1f5f 100644 --- a/workspace-server/internal/models/workspace.go +++ b/workspace-server/internal/models/workspace.go @@ -6,6 +6,12 @@ import ( "time" ) +// DefaultMaxConcurrentTasks mirrors the workspaces.max_concurrent_tasks +// schema default. Handlers that resolve a 0/omitted payload value write +// this constant so the read-side (scheduler capacity check) sees a +// guaranteed non-zero column on every row. +const DefaultMaxConcurrentTasks = 1 + type Workspace struct { ID string `json:"id" db:"id"` Name string `json:"name" db:"name"` @@ -85,11 +91,8 @@ type CreateWorkspacePayload struct { // workspace secrets at creation time. Stored encrypted (same path as // POST /workspaces/:id/secrets). Nil/empty map is a no-op. Secrets map[string]string `json:"secrets"` - // MaxConcurrentTasks caps how many A2A messages + cron fires the - // scheduler will dispatch in parallel for this workspace (#1408). - // 0 = use the schema default of 1 (serialised, worker-style). - // Set to 3 for leaders with frequent orchestrator pulses so an - // in-flight cron doesn't reject incoming A2A delegations. + // MaxConcurrentTasks caps parallel A2A + cron dispatch. 0 means use + // DefaultMaxConcurrentTasks. Leaders typically set 3. MaxConcurrentTasks int `json:"max_concurrent_tasks"` Canvas struct { X float64 `json:"x"`