refactor(edge): 提取 runcontrol 统一 REST/MCP 的 run 创建规则 - #1669
Merged
Conversation
POST /v1/runs 与 MCP agenthub_start_run 此前各自实现同一套 run 创建 序列（目标校验、活跃 run 守卫、CreateRun、run.queued、执行器启动与 失败状态机），规则已开始漂移。提取 internal/runcontrol 作为单一事实源： - Create(store, executor, bus, params) 拥有完整不变量序列，返回 *errcode.Error 供各传输层映射到自己的响应形状（HTTP 状态 vs JSON-RPC 错误文本）。 - 传输层只保留协议差异：REST 注入 profile 默认值、双 token 认证、 timeline 发布（prompt + queued 标记 item）与富上下文构建器；MCP 注入单条 user_message item 与最小上下文。两者经 CreateParams 回调 （Timeline / BuildContext / AgentExists）表达，核心不感知协议。 - 进程级 runCreationMu 串行化 check-then-create 段：修复了 MCP 并发 start_run 可绕过活跃 run 守卫的竞态（REST 原本有 per-Handler 锁， MCP 无锁），且跨协议共享同一把锁。 行为统一（有意为之，PR 内记录）： - MCP 的活跃 run 判定纳入 cancelling（与 REST isActiveRunStatus 对齐） - MCP 错误从 fmt.Errorf 升级为结构化 errcode（workdir_required / active_run_exists / executor_start_failed 等） - REST 空 allowlist 错误码从 workspace_not_allowed 精确为 workspace_allowlist_not_configured（与 MCP 已有行为一致） 新增 runcontrol 直接单测 11 个（含 8 并发串行化对拍，race 干净）； api 22 个 PostRuns 测试与 mcp 33 个 server 测试全部保持通过。 验证：go build/vet/staticcheck 全绿；go test ./... -short -race 通过； verify-orchestrator-deps、verify-test-sleep-ratchet、git diff --check 通过； runcontrol 覆盖率 75%。 Co-authored-by: Cursor <cursor@vectorcontrol.tech>
|
Warning Review limit reached
Next review available in: 102 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds a shared ChangesRun creation flow
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🟡 Moderate · up to MCP start_run can currently accept unknown agent IDs and enqueue runs that fail later, while its message.created event may be dropped because required identifiers are missing; cleanup under the global creation lock can also delay new runs. Merge should wait for these bounded correctness and availability issues to be fixed or explicitly accepted. Sequence Diagram(s)sequenceDiagram
participant REST_or_MCP
participant runcontrol.Create
participant Repository
participant EventBus
participant RunExecutor
REST_or_MCP->>runcontrol.Create: Submit CreateParams
runcontrol.Create->>Repository: Validate target, workspace, permissions, and active runs
runcontrol.Create->>Repository: Create run
runcontrol.Create->>EventBus: Publish run timeline events
runcontrol.Create->>RunExecutor: Start run context
RunExecutor-->>runcontrol.Create: Return start result
runcontrol.Create-->>REST_or_MCP: Return run or mapped error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
edge-server/internal/api/handlers_runs.go (1)
185-187: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBoth transports keep tombstone comments for functions that moved into
runcontrol. The shared root cause is documentation of deleted symbols; git history already records the move, and these comments will go stale.
edge-server/internal/api/handlers_runs.go#L185-L187: delete the comment aboutvalidateRunCreateState.edge-server/internal/mcp/tools_handlers.go#L143-L145: delete the comment aboutvalidateStartRunWorkDiranderrIfActiveRunExists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@edge-server/internal/api/handlers_runs.go` around lines 185 - 187, Remove the stale tombstone comments documenting moved functions: delete the validateRunCreateState comment in edge-server/internal/api/handlers_runs.go at lines 185-187, and delete the comments about validateStartRunWorkDir and errIfActiveRunExists in edge-server/internal/mcp/tools_handlers.go at lines 143-145. No other code changes are needed.edge-server/internal/runcontrol/runcontrol.go (3)
98-100: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider moving
cleanupRunsout of the process-wide critical section.
cleanupRunsscans terminal runs for every thread and can delete many records. It runs whilerunCreationMuis held, so it blocks run creation for all threads and both transports. Cleanup is housekeeping and does not need the check-then-create guarantee.Also applies to: 247-256
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@edge-server/internal/runcontrol/runcontrol.go` around lines 98 - 100, Move cleanupRuns(repository) outside the runCreationMu critical section in the run-control flow, while preserving the existing Cleanup condition and check-then-create synchronization. Ensure the mutex is released before cleanup begins so lengthy terminal-run deletion does not block run creation across threads or transports.
97-134: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReplace the repeated manual
Unlockcalls with one scoped critical section.Every early return must unlock manually. Seven exit points share this obligation. One future added return statement will deadlock the whole process, because the mutex is package-level.
Extract the check-then-create section into a helper that uses
defer.♻️ Proposed refactor
- runCreationMu.Lock() - if params.Cleanup { - cleanupRuns(repository) - } - if err := validateTarget(repository, params.ProjectID, params.ThreadID); err != nil { - runCreationMu.Unlock() - return store.Run{}, err - } - ... - run, err := repository.CreateRun(generateRunID(), params.ProjectID, params.ThreadID) - runCreationMu.Unlock() + run, err := createLocked(repository, executor, params)// createLocked performs the serialized check-then-create section. func createLocked(repository store.Repository, executor lifecycle.RunExecutor, params CreateParams) (store.Run, error) { runCreationMu.Lock() defer runCreationMu.Unlock() if params.Cleanup { cleanupRuns(repository) } if err := validateTarget(repository, params.ProjectID, params.ThreadID); err != nil { return store.Run{}, err } // ... remaining validations ... return repository.CreateRun(generateRunID(), params.ProjectID, params.ThreadID) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@edge-server/internal/runcontrol/runcontrol.go` around lines 97 - 134, Extract the serialized validation and run-creation flow from the caller into a helper such as createLocked, using runCreationMu.Lock with deferred Unlock. Move cleanupRuns, all validation and active-run/executor/agent checks, and repository.CreateRun into that helper, removing every manual unlock and preserving the existing error mappings and return behavior; have the caller handle post-creation errors as before.
49-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove or document the
CreateParamsfields thatCreatenever reads.
CreatereadsProjectID,ThreadID,WorkDir,WorkspaceAllowlist,PermissionMode,AgentID,Cleanup,Timeline, andBuildContext. It never readsPrompt,Model,SessionID, orContinueLast. Both transports fill these fields, and both also pass the same values again insideBuildContext. A reader can assume the core forwards them to the executor context, which it does not.Either drop the four unused fields, or state in the doc comment that they are informational only.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@edge-server/internal/runcontrol/runcontrol.go` around lines 49 - 79, Remove the unused CreateParams fields Prompt, Model, SessionID, and ContinueLast, since Create does not read them and the values are already supplied through BuildContext. Update both transport call sites to stop populating these fields while preserving the corresponding executor context values.edge-server/internal/runcontrol/runcontrol_test.go (1)
77-206: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd cases for the remaining
Createbranches.The suite covers validation, executor failure, and serialization. Four branches stay untested:
repository == nilreturningErrStoreNotConfigured,Cleanup: trueinvokingcleanupRuns, nilTimelineand nilBuildContext(executor start skipped), and thestore.ErrNotFoundmapping fromCreateRun. The stated coverage is 75%, so these additions would close most of the gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@edge-server/internal/runcontrol/runcontrol_test.go` around lines 77 - 206, Extend the Create tests with cases for nil repository returning ErrStoreNotConfigured, Cleanup: true invoking cleanupRuns, nil Timeline and nil BuildContext skipping executor startup, and CreateRun returning store.ErrNotFound mapping to the expected error. Anchor the additions near TestCreateRequiresExecutor and reuse existing test doubles and assertions to verify each branch’s observable behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@edge-server/internal/mcp/tools_handlers.go`:
- Around line 184-208: Update the Timeline callback’s message.created
publication to include the reducer-required messageId, threadId, and role fields
alongside content, using the created item’s identifiers and user role;
alternatively publish item.created with the complete item record.
- Around line 174-183: Update the runcontrol.CreateParams construction in the
MCP handler to pass the adapter-existence resolver via AgentExists and set
Cleanup to true. Reuse the existing resolver used by other run-creation paths so
unknown agent IDs return ErrInvalidAgentID during creation while terminal MCP
runs are cleaned up.
---
Nitpick comments:
In `@edge-server/internal/api/handlers_runs.go`:
- Around line 185-187: Remove the stale tombstone comments documenting moved
functions: delete the validateRunCreateState comment in
edge-server/internal/api/handlers_runs.go at lines 185-187, and delete the
comments about validateStartRunWorkDir and errIfActiveRunExists in
edge-server/internal/mcp/tools_handlers.go at lines 143-145. No other code
changes are needed.
In `@edge-server/internal/runcontrol/runcontrol_test.go`:
- Around line 77-206: Extend the Create tests with cases for nil repository
returning ErrStoreNotConfigured, Cleanup: true invoking cleanupRuns, nil
Timeline and nil BuildContext skipping executor startup, and CreateRun returning
store.ErrNotFound mapping to the expected error. Anchor the additions near
TestCreateRequiresExecutor and reuse existing test doubles and assertions to
verify each branch’s observable behavior.
In `@edge-server/internal/runcontrol/runcontrol.go`:
- Around line 98-100: Move cleanupRuns(repository) outside the runCreationMu
critical section in the run-control flow, while preserving the existing Cleanup
condition and check-then-create synchronization. Ensure the mutex is released
before cleanup begins so lengthy terminal-run deletion does not block run
creation across threads or transports.
- Around line 97-134: Extract the serialized validation and run-creation flow
from the caller into a helper such as createLocked, using runCreationMu.Lock
with deferred Unlock. Move cleanupRuns, all validation and
active-run/executor/agent checks, and repository.CreateRun into that helper,
removing every manual unlock and preserving the existing error mappings and
return behavior; have the caller handle post-creation errors as before.
- Around line 49-79: Remove the unused CreateParams fields Prompt, Model,
SessionID, and ContinueLast, since Create does not read them and the values are
already supplied through BuildContext. Update both transport call sites to stop
populating these fields while preserving the corresponding executor context
values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c6e9592-b162-42f7-80b7-4b7164b49e51
📒 Files selected for processing (7)
edge-server/internal/api/handlers.goedge-server/internal/api/handlers_events.goedge-server/internal/api/handlers_runs.goedge-server/internal/api/handlers_test.goedge-server/internal/mcp/tools_handlers.goedge-server/internal/runcontrol/runcontrol.goedge-server/internal/runcontrol/runcontrol_test.go
💤 Files with no reviewable changes (2)
- edge-server/internal/api/handlers_events.go
- edge-server/internal/api/handlers.go
Comment on lines
+174
to
+183
| run, err := runcontrol.Create(s.store, s.executor, s.bus, runcontrol.CreateParams{ | ||
| ProjectID: params.ProjectID, | ||
| ThreadID: params.ThreadID, | ||
| Prompt: params.Prompt, | ||
| AgentID: params.AgentID, | ||
| Model: params.Model, | ||
| WorkDir: params.WorkDir, | ||
| WorkspaceAllowlist: s.workspaceAllowlist, | ||
| SessionID: "mcp_" + params.ThreadID, | ||
| ContinueLast: true, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the MCP Server holds an adapter registry or equivalent resolver.
set -euo pipefail
ast-grep outline edge-server/internal/mcp --items all --type struct,function
rg -nP --type=go -C4 '\b(adapterRegistry|AdapterRegistry|workspaceAllowlist)\b' edge-server/internal/mcpRepository: TokenDanceLab/AgentHub
Length of output: 5713
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- MCP server construction and start_run ---'
sed -n '90,180p' edge-server/internal/mcp/server.go
sed -n '130,230p' edge-server/internal/mcp/tools_handlers.go
printf '%s\n' '--- CreateParams and cleanup/agent validation ---'
rg -nP --type=go -C5 '\bCreateParams\b|\bAgentExists\b|\bCleanup\b|ErrInvalidAgentID|func Create\b' edge-server/internal edge-server/cmd
printf '%s\n' '--- REST run creation and MCP wiring ---'
rg -nP --type=go -C8 'NewServer\(|SetWorkspaceAllowlist|PostRuns|CreateParams' edge-server
printf '%s\n' '--- Adapter registry definitions and uses ---'
rg -nP --type=go -C6 'type .*Adapter|AdapterRegistry|adapter registry|Resolve.*Agent|AgentExists|agent.*exists' edge-serverRepository: TokenDanceLab/AgentHub
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Relevant files ---'
git ls-files '*runcontrol*' '*tools_handlers.go' '*server.go' '*handlers.go' | head -n 120
printf '%s\n' '--- MCP server and handler excerpts ---'
sed -n '108,175p' edge-server/internal/mcp/server.go
sed -n '145,225p' edge-server/internal/mcp/tools_handlers.go
printf '%s\n' '--- Narrow CreateParams references ---'
rg -l --type=go 'CreateParams|AgentExists|Cleanup|ErrInvalidAgentID' edge-server | sortRepository: TokenDanceLab/AgentHub
Length of output: 8381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- runcontrol structure ---'
ast-grep outline edge-server/internal/runcontrol/runcontrol.go --items all
sed -n '1,280p' edge-server/internal/runcontrol/runcontrol.go
printf '%s\n' '--- REST PostRuns construction ---'
rg -n -C12 'runcontrol.Create|AgentExists|Cleanup|ErrInvalidAgentID' edge-server/internal/api/handlers_runs.go
printf '%s\n' '--- MCP construction call sites ---'
rg -n -C10 'mcp.NewServer|SetWorkspaceAllowlist' edge-server --glob '*.go' --glob '!**/*_test.go'Repository: TokenDanceLab/AgentHub
Length of output: 21596
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Agent selection and fallback behavior ---'
rg -n -C8 --type=go 'AgentID|agentId|default adapter|DefaultAdapter|AdapterRegistry.Get|registry.Get' \
edge-server/internal/lifecycle edge-server/internal/adapters edge-server/internal/api edge-server/internal/httpserver \
--glob '!**/*_test.go'
printf '%s\n' '--- Cleanup implementation and store support ---'
rg -n -C10 --type=go 'CleanupRuns|RunCleaner|RunCleanupOptions|TerminalTTL|MaxTerminalRunsPerThread' edge-server/internal/storeRepository: TokenDanceLab/AgentHub
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Registry implementation ---'
rg -l --type=go 'func .*Resolve|type Registry struct|func .*Get' edge-server/internal/adapters | sort
rg -n -C12 --type=go 'func \(.*\) Resolve|type Registry struct|func \(.*\) Get' edge-server/internal/adapters
printf '%s\n' '--- Exact executor resolution path ---'
sed -n '1,58p' edge-server/internal/lifecycle/process_executor_run.go
printf '%s\n' '--- Exact cleanup contract and implementation ---'
rg -n -C12 --type=go 'type RunCleaner|type RunCleanupOptions|func .*CleanupRuns' edge-server/internal/storeRepository: TokenDanceLab/AgentHub
Length of output: 32059
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
mcp = Path("edge-server/internal/mcp/tools_handlers.go").read_text()
server = Path("edge-server/internal/mcp/server.go").read_text()
httpserver = Path("edge-server/internal/httpserver/server.go").read_text()
rest = Path("edge-server/internal/api/handlers_runs.go").read_text()
runcontrol = Path("edge-server/internal/runcontrol/runcontrol.go").read_text()
registry = Path("edge-server/internal/adapters/registry.go").read_text()
executor = Path("edge-server/internal/lifecycle/process_executor_run.go").read_text()
def require(text, fragment, label):
if fragment not in text:
raise SystemExit(f"missing {label}: {fragment!r}")
require(mcp, "ContinueLast: true,", "MCP CreateParams")
if "AgentExists:" in mcp or "Cleanup:" in mcp:
raise SystemExit("MCP unexpectedly wires AgentExists or Cleanup")
require(rest, "AgentExists: func(agentID string) bool", "REST AgentExists")
require(rest, "Cleanup: true,", "REST Cleanup")
require(httpserver, "mcp.NewServer(handler.Store, handler.Executor, handler.Bus, handler.PermissionRegistry)", "MCP construction")
if "handler.AdapterRegistry" in httpserver.split("mcp.NewServer", 1)[1].split("slog.Info", 1)[0]:
raise SystemExit("MCP construction already passes adapter registry")
require(runcontrol, "if params.AgentID != \"\" && params.AgentExists != nil && !params.AgentExists(params.AgentID)", "conditional agent validation")
require(runcontrol, "if params.Cleanup {", "conditional cleanup")
require(registry, 'if agentID != "" {', "explicit adapter resolution")
require(registry, 'return nil, fmt.Errorf("agent adapter %q not found", agentID)', "unknown adapter error")
require(executor, "resolved, err := e.adapterReg.Resolve(runCtx.AgentID)", "executor adapter resolution")
print("MCP: AgentExists absent; Cleanup absent")
print("REST: AgentExists wired; Cleanup enabled")
print("Unknown non-empty agentID: runcontrol does not reject it; registry resolution returns an error rather than selecting the default")
print("Adapter registry exists in httpserver.Handler but is not passed to mcp.NewServer")
PYRepository: TokenDanceLab/AgentHub
Length of output: 451
Wire MCP agent validation and terminal-run cleanup
MCP does not pass AgentExists or set Cleanup: true. An unknown agentId bypasses creation-time validation, creates a queued run, and fails later during adapter resolution instead of returning ErrInvalidAgentID. Pass the adapter-existence resolver to MCP and enable cleanup.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@edge-server/internal/mcp/tools_handlers.go` around lines 174 - 183, Update
the runcontrol.CreateParams construction in the MCP handler to pass the
adapter-existence resolver via AgentExists and set Cleanup to true. Reuse the
existing resolver used by other run-creation paths so unknown agent IDs return
ErrInvalidAgentID during creation while terminal MCP runs are cleaned up.
Comment on lines
+184
to
+208
| Timeline: func(run store.Run) { | ||
| // MCP publishes a single user_message item (REST additionally | ||
| // publishes a queued marker item). | ||
| item, createErr := s.store.CreateItem(store.Item{ | ||
| ID: generateID("item_"), | ||
| ProjectID: run.ProjectID, | ||
| ThreadID: run.ThreadID, | ||
| RunID: run.ID, | ||
| Type: "user_message", | ||
| Role: "user", | ||
| Status: "created", | ||
| Content: params.Prompt, | ||
| }) | ||
| if createErr != nil || s.bus == nil { | ||
| return | ||
| } | ||
| s.bus.Publish("message.created", map[string]any{ | ||
| "projectId": item.ProjectID, | ||
| "threadId": item.ThreadID, | ||
| "runId": item.RunID, | ||
| "itemId": item.ID, | ||
| }, map[string]any{ | ||
| "content": params.Prompt, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Start the executor | ||
| sessionID := "mcp_" + run.ThreadID | ||
| runCtx := lifecycle.RunProcessContext{ | ||
| Run: run, | ||
| Prompt: params.Prompt, | ||
| AgentID: params.AgentID, | ||
| Model: params.Model, | ||
| SessionID: sessionID, | ||
| ContinueLast: true, | ||
| WorkDir: params.WorkDir, | ||
| } | ||
|
|
||
| if err := s.executor.Start(run, runCtx); err != nil { | ||
| slog.Error("mcp run start failed", "runId", run.ID, "error", err) | ||
| // Mark run as failed | ||
| if failed, ok := s.store.SetRunStatusIf(run.ID, "failed", "queued"); ok { | ||
| if s.bus != nil { | ||
| s.bus.Publish("run.failed", scope, map[string]any{ | ||
| "runId": failed.ID, | ||
| "status": failed.Status, | ||
| "error": "run execution failed", | ||
| }) | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Trace subscribers of message.created and item.created and their payload expectations.
set -euo pipefail
rg -n --type=go -C5 '"message\.created"|"item\.created"' edge-server
rg -rn --iglob '*.ts' --iglob '*.tsx' -C4 'message\.created|item\.created' . || trueRepository: TokenDanceLab/AgentHub
Length of output: 25291
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- MCP timeline and related publishing ---'
sed -n '150,225p' edge-server/internal/mcp/tools_handlers.go
sed -n '180,225p' edge-server/internal/api/handlers_runs.go
sed -n '420,450p' edge-server/internal/mcp/tools_handlers.go
printf '%s\n' '--- Desktop event bridge handlers ---'
rg -n -C8 'onMessageCreated|onItemCreated|message\.created|item\.created' app/desktop/src/stores/edgeEventBridge.ts app/shared/src
printf '%s\n' '--- Event types and reducers ---'
sed -n '1,180p' app/shared/src/events.ts
rg -n -C12 "case 'message.created'|case 'item.created'|case 'message.created'|case 'item.created'" app/shared/src app/desktop/src app/web/src
printf '%s\n' '--- Tests covering event payloads ---'
rg -n -C10 'message\.created|item\.created|content:.*Prompt|content:.*prompt' edge-server app/shared app/desktop app/web -g '*_test.go' -g '*.test.ts' -g '*.test.tsx'Repository: TokenDanceLab/AgentHub
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Message event conversion ---'
rg -n -C12 'func .*Publish|type EventEnvelope|Payload|Scope' edge-server/internal/events edge-server/internal -g '*.go' | head -n 240
printf '%s\n' '--- Message reducer input requirements ---'
rg -n -C18 'function threadItemFromMessage|const threadItemFromMessage|threadItemFromMessage' app/shared/src
sed -n '1,145p' app/shared/src/workbenchStateApplyEvent.ts
printf '%s\n' '--- All MCP start-run tests and event consumers ---'
rg -n -C12 'toolStartRun|start_run|message\.created|item\.created' edge-server/internal/mcp app -g '*.go' -g '*.ts' -g '*.tsx' | head -n 320
printf '%s\n' '--- Static contract verifier ---'
python3 - <<'PY'
from pathlib import Path
mcp = Path("edge-server/internal/mcp/tools_handlers.go").read_text()
reducer = Path("app/shared/src/workbenchStateApplyEvent.ts").read_text()
timeline = mcp[mcp.index("Timeline: func(run store.Run)"):mcp.index("BuildContext:", mcp.index("Timeline: func(run store.Run)"))]
required = ["messageId", "threadId", "role", "content"]
missing = [name for name in required if f'"{name}"' not in timeline]
print("MCP timeline contains required message payload keys:", not missing)
print("Missing keys:", missing)
print("MCP timeline publishes item.created:", '"item.created"' in timeline)
print("Reducer handles message.created:", "case 'message.created'" in reducer)
PYRepository: TokenDanceLab/AgentHub
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
mcp = Path("edge-server/internal/mcp/tools_handlers.go").read_text()
bus = Path("edge-server/internal/events/bus.go").read_text()
helpers = Path("app/shared/src/workbenchStateHelpers.ts").read_text()
reducer = Path("app/shared/src/workbenchStateApplyEvent.ts").read_text()
start = mcp.index("Timeline: func(run store.Run)")
end = mcp.index("BuildContext:", start)
timeline = mcp[start:end]
print("MCP publishes item.created:", '"item.created"' in timeline)
print("MCP payload has content:", '"content": params.Prompt' in timeline)
for key in ("messageId", "threadId", "role"):
print(f"MCP payload has {key}:", f'"{key}"' in timeline)
print("Bus stores scope separately from payload:",
"Scope: scope" in bus and "Payload: payload" in bus)
required = {
"messageId": 'const id = text(payload.messageId)',
"threadId": 'const threadId = text(payload.threadId)',
"role": 'const itemRole = role(payload.role)',
}
for key, source in required.items():
print(f"Reducer requires payload {key}:", source in helpers)
print("Reducer drops invalid message.created:",
"if (!item) return withSeq(state, nextSeq);" in reducer)
PYRepository: TokenDanceLab/AgentHub
Length of output: 524
Fix the MCP message.created payload.
The shared reducer requires messageId, threadId, and role in the payload. MCP sends only content, so the reducer discards the event. Publish the complete payload or publish item.created with the item record.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@edge-server/internal/mcp/tools_handlers.go` around lines 184 - 208, Update
the Timeline callback’s message.created publication to include the
reducer-required messageId, threadId, and role fields alongside content, using
the created item’s identifiers and user role; alternatively publish item.created
with the complete item record.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
POST /v1/runs 与 MCP
agenthub_start_run此前各自实现同一套 run 创建序列（目标校验、活跃 run 守卫、CreateRun、run.queued、执行器启动与失败状态机），规则已开始漂移（探查报告中 edge-server Top 1 项）。提取internal/runcontrol作为单一事实源。核心设计：
runcontrol.Create(store, executor, bus, params)拥有完整不变量序列，始终返回*errcode.Error，两个传输层各自映射到自己的响应形状（HTTP 状态 vs JSON-RPC 错误文本）。传输层差异经CreateParams回调注入：mcp_<thread>，ContinueLast=true）修复的缺陷：进程级
runCreationMu串行化 check-then-create 段——修复 MCP 并发 start_run 可绕过活跃 run 守卫的竞态（REST 原本有 per-Handler 锁，MCP 无锁），且跨协议共享同一把锁。有意为之的行为统一（详见 commit body）：
cancelling（与 RESTisActiveRunStatus对齐）fmt.Errorf升级为结构化 errcodeworkspace_not_allowed精确为workspace_allowlist_not_configured（与 MCP 已有行为一致）Test plan
go build/go vet/ staticcheck 全绿go test ./... -count=1 -short -race -coverprofile通过（CI 同款）internal/runcontrol直接单测 11 个：happy path / 目标校验 / workDir 三态 / permissionMode / active-run / unknown-agent / executor-nil / executor 失败状态机 / 429 映射 / 8 并发串行化对拍（race 干净）verify-orchestrator-deps.py、verify-test-sleep-ratchet.py、git diff --check通过后续队列
ProcessExecutor12 张并行 map → runState 收敛DecisionLoop孤儿代码接线或删除（需产品确认 maxSteps 语义）Summary by CodeRabbit
New Features
Bug Fixes