feat: hand MCP work to live coding agents by mariusvniekerk · Pull Request #858 · kenn-io/forge

mariusvniekerk · 2026-08-07T20:45:55Z

Kenn Forge now owns MCP as an optional daemon capability instead of requiring a separate companion process.

Serve stateless Streamable HTTP at /mcp on a dedicated loopback listener configured through [mcp].
Call typed Forge services directly while sharing the daemon's authentication, discovery, readiness, and shutdown lifecycle.
Bind every repository and item reference to the provider-verified stable repository ID.
Keep review candidate output bounded and exclude removed-upstream items from workflow operations.
Create or reuse workspaces, launch coding agents, and deliver one bounded initial instruction with explicit delivery evidence.
Preserve structured failure evidence and full diffs for binary, rename-only, and metadata-only changes.
Bound temporary full-diff storage with a configurable 128 MiB least-recently-used cache.
Remove the standalone MCP command, stdio transport, daemon self-client, cached endpoint credentials, and duplicate workflow HTTP routes.

roborev-ci · 2026-08-07T20:52:59Z

roborev: Combined Review (`9c3a89e`)

High-risk security and prompt-delivery flaws remain, alongside several medium reliability and performance issues.

High

Indirect prompt injection enables local process execution — internal/mcpserver/tools_agent_spawn.go:84
Provider-controlled PR content can influence a model that can create worktrees and launch coding agents with maintainer privileges, without deterministic human approval. Require an out-of-band, human-approved capability bound to the source and target, or separate/disable handoff when provider-reading tools are enabled.
Owner-managed PTY delivery is falsely acknowledged — internal/workspace/localruntime/manager.go:2463
Socket encoding is treated as successful delivery even if the PTY write fails. A permitted 64 KiB payload may also exceed the owner’s framed input limit and be silently discarded. Add an acknowledged operation that verifies the complete write, accounts for framing, and has full-stack coverage.

Medium

Receipt recovery can accept an unrelated delivery — internal/mcpserver/tools_agent_spawn.go:545
Recovery ignores agent, session, and byte-count identity, so a concurrent receipt may falsely confirm delivery. Verify all identities and preserve ambiguous: true for uncertain or pending states.
Wait loops overload the daemon — internal/mcpserver/tools_agent_spawn.go:20
Polling every 10 ms generates 100–200 requests per second during potentially long operations. Use a slower cadence or bounded backoff with a test-configurable interval.
Validation errors violate the stable MCP envelope — internal/mcpserver/tools_read.go:98
Several local failures return plain errors without kind, retryable, or ambiguous. Convert them to typed daemonError values and add a fallback wrapper for unclassified errors.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 13m15s

roborev-ci · 2026-08-08T05:20:05Z

roborev: Combined Review (`8fb095f`)

The PR has three medium-severity correctness issues; no critical or high-severity findings were identified.

Medium

internal/mcpserver/tools_diff.go:153 — Diff-file generation rejects empty patches, but binary and metadata-only changes such as pure renames can legitimately produce them, causing emit_diff_file to fail. Generate complete patch sections for binary, rename-only, and mode-only changes, with full-stack test coverage.
internal/mcpserver/tools_agent_spawn.go:168,586 — Possible writes may be reported as ambiguous:false: recovered pending/uncertain receipts become plain errors, while total-timeout handling can discard a mutation error’s ambiguity. Preserve ambiguity independently of timeout classification and return an ambiguous error for every non-delivered receipt that may represent a write.
internal/mcpserver/tools_candidates.go:135 — Candidate discovery filters item types only after /activity applies its 5,000-row cap, so busy repository-level activity can hide relevant PR or issue activity. Pass the selected item_types=pr and/or item_types=issue filters to /activity.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 13m18s

roborev-ci · 2026-08-08T05:45:47Z

roborev: Combined Review (`f95b481`)

Changes need fixes for two medium-severity reliability issues.

Medium

internal/server/workspaceapi/initial_message.go:171 — Failures occurring before any write, such as AttachSession failures, incorrectly mark the receipt uncertain and permanently block safe retries. Distinguish definitive pre-write failures from possible partial writes; release reservations for pre-write failures and mark only potentially partial writes uncertain.
internal/mcpserver/difftmp.go:24 — Deterministic per-item diff filenames are atomically overwritten, so concurrent snapshot requests can return summaries referencing files that now contain another request’s diff. Use content-addressed or request-unique filenames.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 12m9s

roborev-ci · 2026-08-09T17:44:07Z

roborev: Combined Review (`4e75578`)

High-severity issues prevent safe merge; three additional medium-severity reliability defects need correction.

High

Persistent daemon token can leak to a stale loopback listener
Location: internal/mcpserver/daemon.go:186-225
do indefinitely reuses the cached baseURL and sends the daemon bearer token without revalidating the runtime lock or endpoint identity. If the daemon stops and another local process binds the former port, the next MCP request can disclose the persistent token.
Fix: Revalidate authoritative runtime identity before every credential-bearing request. Use the existing token-derived proof flow, reject stopped or changed runtime records, atomically bind the verified endpoint and token, and disable credential-bearing redirects.
Duplicate migration version prevents database migration and server startup
Location: internal/db/migrations/000047_agent_initial_message_receipts.up.sql:1
Version 000047 is already assigned to 000047_repository_route_generation, making the embedded migration source invalid.
Fix: Renumber the new up/down migration files to the next unused sequential version and update related references.

Medium

Valid no-hunk changes cause diff export failure
Location: internal/mcpserver/tools_diff.go:153
Diff export treats an empty Patch as a daemon error, although binary files, pure renames, and mode-only changes can legitimately have no hunks. This makes emit_diff_file fail for valid handoffs.
Fix: Preserve file metadata while omitting or synthesizing representations for legitimate no-hunk patches, with tests at the real diff-building boundary.
Multiline initial prompts can be silently lost during agent startup
Location: internal/mcpserver/tools_agent_spawn.go:552
Session startup does not guarantee bracketed-paste readiness. A proven no-write rejection is not retried, leaving a live agent and workspace without the requested prompt.
Fix: Expose a stable paste-not-ready error reason and retry only that proven-no-write condition within the existing timeout. Add end-to-end coverage using a multiline prompt and actual terminal readiness.

Reviewers: 2 done | Synthesis: codex, 14s | Total: 19m41s

roborev-ci · 2026-08-13T04:28:06Z

roborev: Combined Review (`4fea1e9`)

Verdict: One high-severity security issue and four medium-severity correctness/reliability issues require attention.

High

Agent launch lacks enforceable human approval — internal/mcpserver/tools_agent.go:97, internal/mcpserver/tools_items.go:213, internal/mcpserver/tools_agent_spawn.go:133, internal/mcpserver/tools_agent_spawn.go:163
Untrusted PR bodies, issue comments, and event text are returned verbatim to the MCP model. Prompt injection could induce it to invoke kenn_forge_spawn_workspace_with_agent, creating an attacker-controlled worktree and launching a local coding agent with a model-selected prompt. Written guidance is not an enforceable security boundary. Require out-of-band, one-time human authorization bound to the source, target, and initial-message digest, or isolate and disable agent-launch tools unless explicitly enabled.

Medium

Full-diff export rejects valid empty patches — internal/mcpserver/tools_diff.go:153
Binary and metadata-only changes can legitimately have empty patches, causing the entire emit_diff_file request to fail. Preserve or synthesize records for these changes and add full-stack coverage using a real binary or mode-only change.
Diff snapshots overwrite one deterministic path — internal/mcpserver/difftmp.go:25
Later or concurrent exports for the same PR can replace a snapshot before its caller reads it, leaving summary metadata inconsistent with the file contents. Use immutable unique or content-addressed filenames and remove all snapshots when the MCP server closes.
Pre-write submission failures are incorrectly marked uncertain — internal/server/workspaceapi/initial_message.go:171
Attachment and agent-kind failures can occur before any terminal write, yet permanently consume the one-shot receipt as an ambiguous delivery. Distinguish pre-write errors and release their pending reservation; reserve uncertain for failures during or after the PTY write.
MCP dependency addition rolls back unrelated dependencies — go.mod:9
Huma, GitLab, Kata, SQLite, OpenTelemetry, the OpenAPI generator, and other direct dependencies are downgraded, potentially restoring previously fixed behavior. Restore the prior versions, add only the required MCP dependencies, and regenerate API artifacts with the current generator.

Reviewers: 2 done | Synthesis: codex, 15s | Total: 19m0s

roborev-ci · 2026-08-13T13:39:27Z

roborev: Combined Review (`9350288`)

The PR has three medium-severity issues affecting diff emission, agent handoff reliability, and dependency stability.

Medium

internal/mcpserver/tools_diff.go:153 — Diff-file emission rejects all empty patches, even though binary files and metadata-only changes such as renames can legitimately have no hunks. One such file causes emit_diff_file to fail for the entire PR. Preserve or synthesize valid unified-diff metadata and add full-stack coverage for binary or metadata-only changes.
internal/mcpserver/tools_agent_spawn.go:163 — Handoff can submit a multiline message before the PTY has processed bracketed-paste activation. ErrBracketedPasteInactive then aborts the handoff and leaves a live partial runtime. Treat inactive bracketed-paste mode as temporary readiness, retry safely within the remaining timeout, and add a real handoff end-to-end test for this timing path.
go.mod:9 — Adding the MCP SDK downgrades unrelated direct dependencies, including Huma, GitLab, Kata, kit, OpenTelemetry, and SQLite, potentially reverting fixes across core functionality. Restore the previous direct dependency versions, add the MCP dependencies without unrelated downgrades, and regenerate affected artifacts.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 21m42s

roborev-ci · 2026-08-13T14:41:49Z

roborev: Combined Review (`919a974`)

High-severity dependency regressions and two medium-severity handoff lifecycle issues must be addressed before merge.

High

go.mod:9 — Existing dependencies were unintentionally downgraded. Adding MCP dependencies downgrades much of the dependency graph, including go.kenn.io/kata from the required v0.14.3 to v0.11.1 while the frontend remains pinned to v0.14.3. This risks regressions and incompatible Kata daemon behavior.
- Fix: Restore the pre-change dependency versions, add only the new MCP requirements, and regenerate API artifacts using the current generator versions.

Medium

internal/mcpserver/tools_agent_spawn.go:163 — Multiline handoff can permanently fail during a transient paste-mode race. The hook session may become visible before bracketed-paste mode 2004 is observed. Although the daemon safely releases the reservation without writing, MCP treats the rejection as permanent, leaving a created workspace/runtime without delivering the prompt.
- Fix: Return a typed “paste mode not ready” response and retry this proven-no-write condition within the handoff timeout. Add a full-stack test in which paste mode activates after the hook report.
internal/server/workspaceapi/initial_message.go:236 — Initial-message retention can grow without bound. Complete prompts of up to 64 KiB remain in initialMessages for the daemon lifetime unless the workspace is deleted. Stopping or forgetting runtime sessions does not evict them, so repeated handoffs to reused workspaces continuously retain memory.
- Fix: Use bounded lifecycle retention, such as compact digests or tombstones for idempotency, and evict attempts when their runtime is retired while preserving a bounded status/retry window.

Reviewers: 2 done | Synthesis: codex, 15s | Total: 16m55s

roborev-ci · 2026-08-14T14:33:35Z

roborev: Combined Review (`c05d80a`)

Code review found four medium-severity issues requiring attention.

Medium

Dependency downgrades and unrelated churn — go.mod:9
Adding the MCP SDK also downgrades established dependencies, including Huma, GitLab, Kata, SQLite, OpenTelemetry, and the OpenAPI generator. This could reintroduce fixed bugs or alter provider and server behavior.
Fix: Restore the previous versions, add only MCP-related requirements, and regenerate API artifacts with the repository’s current generator version.
Candidate filtering occurs after the safety cap — internal/mcpserver/tools_candidates.go:135
The request omits the activity API’s item_types filter, then removes irrelevant events only after the fixed safety cap is applied. Busy repositories may therefore hide eligible pull requests or issues.
Fix: Pass the appropriate item_types=pr and/or item_types=issue filters before fetching activity, and paginate if complete filtered results are required.
Ambiguous delivery recovery may query a different daemon — internal/mcpserver/tools_agent_spawn.go:594
Recovery after an ambiguous initial-message POST uses generic daemon rediscovery. A restart or concurrent handoff could provide status from a different daemon, which does not prove the outcome of the original POST.
Fix: Pin the daemon identity and connection used for the POST, perform recovery without rediscovery, and report an ambiguous outcome if that daemon is unavailable.
Successful handoff prompts accumulate indefinitely — internal/server/workspaceapi/initial_message.go:218
Full normalized prompts of up to 64 KiB remain in initialMessages until workspace deletion. Runtime exit and explicit session stop do not clear them, allowing unbounded memory growth in persistent workspaces.
Fix: Expire or clear attempts after sessions end and a bounded idempotency window passes; retain only a bounded hash or tombstone if duplicate detection must continue.

Reviewers: 2 done | Synthesis: codex, 20s | Total: 17m12s

Current main advanced the server dependencies and API generator after this branch started. Retaining the branch's older module graph would restore superseded behavior and generated a client that no longer matched current API fields. Keep main's selected dependency versions, add only the MCP requirements, and regenerate the clients from the rebased schema. Update affected tests for the current generator's namespaced enum constants and the rebased auto-assignment signature. Resolves the dependency and generated-client finding reported at #858 (comment). Generated with Pi

roborev-ci · 2026-08-18T18:01:16Z

roborev: Combined Review (`7d14fce`)

One-line verdict: Significant concurrency, ambiguity, visibility, retention, and credential-leak risks require fixes before merge.

High

Privileged bearer token can leak through a stale cached daemon endpoint — internal/mcpserver/daemon.go:186, internal/mcpserver/daemon.go:224
After a daemon restart or port change, another local user could bind the old loopback port and capture the persistent token. Re-discover and cryptographically verify the daemon before each credential-bearing connection, snapshot endpoint/token atomically, prohibit cross-origin redirects, and never authorize an unverified cached address.
Ambiguous mutation outcomes are incorrectly reported as safe to retry — internal/mcpserver/tools_agent_spawn.go:141
Successful responses lacking an ID/key, or returning pending/uncertain, may cause retries that create duplicate workspaces or agents. Return a non-retryable daemonError with Ambiguous: true whenever conclusive mutation evidence is absent.

Medium

Concurrent route reassignment can mutate the displaced repository — internal/server/workflow_state_routes.go:321
Capture and guard the repository route generation through item validation and the database write.
Workflow endpoints expose and mutate removed_upstream tombstones — internal/server/workflow_state_routes.go:470, internal/db/queries_workflow.go:332
Use visible-item getters and exclude archived tombstones from both workflow-list query branches.
Proven pre-write runtime failures are permanently marked uncertain — internal/server/workspaceapi/initial_message.go:165
Return a distinct pre-write runtime error and release the reservation for every proven no-write failure so retries remain safe.
Initial prompts have unbounded retention after runtime exit — internal/server/workspaceapi/initial_message.go:218
Add bounded retention and prune exited-runtime attempts after a recovery grace period.
PR diff cache has no aggregate retention ceiling — internal/mcpserver/difftmp.go:24
Enforce total file and byte limits with bounded LRU eviction.

Reviewers: 2 done | Synthesis: codex, 13s | Total: 14m48s

roborev-ci · 2026-08-19T13:36:27Z

roborev: Combined Review (`41fec02`)

High-risk MCP boundary issues—including prompt-triggered privileged execution and lost structured failures—must be addressed before merge.

High

Privileged agent execution via prompt injection — internal/mcpserver/server.go:52, internal/mcpserver/tools_items.go:117
Untrusted provider content is exposed to models alongside the agent-spawning tool, allowing malicious instructions to trigger commands or file changes with daemon-user privileges. Require out-of-band approval tied to the exact source and target, or isolate handoff behind a disabled-by-default privileged MCP surface.
Structured handoff failures are lost — internal/mcpserver/tools_read.go:101
wrapTool converts typed failures into ordinary errors, discarding retryability, ambiguity, recovery IDs, and partial-progress evidence. Return an error CallToolResult with structured content and test failure through an MCP session.

Medium

Valid empty diffs fail — internal/mcpserver/tools_diff.go:130
Binary files and metadata-only renames or copies can legitimately have empty patches. Preserve and serialize their metadata instead of returning an internal error.
Workflow validation differs across transports — internal/mcpserver/tools_workflow.go:69
MCP mutations bypass the HTTP API’s 120-character actor and 500-character reason limits. Enforce these constraints at a shared service boundary.
Prompt attempts can accumulate indefinitely — internal/server/workspaceapi/initial_message.go:263
Completed and indeterminate attempts remain until workspace deletion. Remove or expire them after the recovery window or runtime-session exit.
Terminal deletion failure waits for timeout — internal/mcpserver/tools_agent_spawn.go:385
Readiness polling does not handle stable deletion_failed state. Fail immediately while preserving handoff evidence.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 10m25s

roborev-ci · 2026-08-19T13:47:25Z

roborev: Combined Review (`398aa22`)

Changes need revision: 1 High and 5 Medium findings remain.

High

internal/mcpserver/tools_read.go:101 — Tool failures return ordinary Go errors, dropping structured fields such as ambiguity, retryability, stage, and created workspace/runtime IDs. Clients cannot determine whether retrying a handoff could duplicate resources. Convert typed errors into explicit MCP error results and test serialization through an MCP client session.

Medium

internal/server/workflow_state_routes.go:262 — Workflow list/read/write routes expose items archived as removed_upstream; point lookups at lines 470 and 478 also use unfiltered getters. Set ExcludeRemovedUpstream, use visible-item getters, and test list/get/put behavior for removed items.
internal/mcpserver/tools_diff.go:130 — Full-diff emission rejects empty patches, although binary and metadata-only changes can validly produce them. Preserve or synthesize binary, rename, and mode-change headers instead of failing the entire handoff.
internal/mcpserver/tools_diff.go:206 — Owner and repository names are always lowercased, causing case-sensitive Forgejo/Gitea repository identities to collide. Only case-fold when platform.LowercaseRepoNames permits it.
internal/mcpserver/difftmp.go:24 — The temporary diff store lacks total-size, file-count, TTL, and eviction bounds, allowing disk use to grow until daemon shutdown. Add expiration or bounded LRU eviction.
internal/server/workspaceapi/initial_message.go:274 — Delivered or uncertain prompts remain in memory until workspace deletion or daemon exit, allowing sequential launches to retain unbounded 64 KiB prompts. Expire attempts after runtime exit and the recovery window while preserving active-runtime retry protection.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 13m46s

roborev-ci · 2026-08-19T14:15:44Z

roborev: Combined Review (`8855fa9`)

Code is generally sound, but three medium-severity reliability and resource-management issues should be addressed.

Medium

internal/mcpserver/tools_diff.go:130 — Valid binary, copy, rename, and metadata-only changes may have an empty Patch, causing emit_diff_file to fail. Serialize them using raw or synthesized diff metadata instead of treating them as internal errors.
internal/mcpserver/difftmp.go:24 — Generated diff files accumulate until daemon shutdown without aggregate size, count, or age limits, enabling unbounded temporary-disk usage. Add bounded retention with eviction or expiration.
internal/mcpserver/tools_agent_spawn.go:336 — Pull-request workspace lookup and creation are non-atomic. Concurrent handoffs can cause one request to receive a duplicate conflict instead of reusing the winning workspace. Re-resolve after duplicate conflicts or make creation idempotent.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 11m34s

roborev-ci · 2026-08-19T14:38:43Z

roborev: Combined Review (`cc76005`)

The PR has three medium-severity resource-management and error-propagation issues to address.

Medium

internal/mcpserver/difftmp.go:24 — Exported diffs remain until daemon shutdown, allowing unbounded temporary disk usage. Add a size/count ceiling with TTL or LRU eviction and test eviction behavior.
internal/mcpserver/tools_agent_spawn.go:548 — handoffFailure drops backendErr.Retryable, incorrectly classifying transient failures as non-retryable. Preserve the field and add a focused transient-failure test.
internal/server/workspaceapi/initial_message.go:272 — Initial prompts are retained until workspace deletion, allowing unbounded memory growth in long-lived workspaces. Add bounded or expiring retention while preserving recent-attempt recovery.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 14m50s

roborev-ci · 2026-08-19T15:13:06Z

roborev: Combined Review (`797f354`)

High-severity identity and session-blocking issues must be fixed; two medium-severity lifecycle and concurrency issues also remain.

High

internal/mcpserver/backend.go:32 — MCP repository and item identities omit platform_repo_id. After a rename or route reuse, an earlier MCP reference could resolve to a different repository and access or mutate the wrong numbered item. Carry the stable provider repository ID through all MCP schemas and validate it during route resolution.
internal/server/workspaceapi/initial_message.go:184 — Initial-message submission discards request context and performs a potentially blocking PTY write while holding the session mutex. Backpressure can exceed the handoff timeout and wedge session lifecycle operations. Make the operation context-aware and bounded, and release the session-wide mutex before writing.

Medium

internal/server/workspaceapi/initial_message.go:274 — Full initial prompts remain in memory until workspace deletion, causing unbounded growth in long-lived workspaces and unnecessarily retaining sensitive text. Add a byte-bounded LRU or TTL that preserves retry recovery, and remove entries after runtime exit.
internal/mcpserver/tools_agent_spawn.go:325 — PR workspace reuse uses a check-then-create sequence that races with concurrent creation, producing a conflict even when the desired workspace now exists. On the specific duplicate-workspace conflict, perform an authoritative read and reuse the matching workspace.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m34s

roborev-ci · 2026-08-19T16:09:05Z

roborev: Combined Review (`809b6ca`)

High-risk repository identity race and two medium-severity agent handoff issues require fixes before merge.

High

internal/server/mcp_backend.go:132 — Stable repository identity is validated before a separate route-based service call. Repository reconciliation can reuse the route between operations, causing reads or handoff through a replacement workspace to target another repository. Execute validation and the operation under a reconciliation fence, or pass the resolved stable repository ID through the service layer.

Medium

internal/mcpserver/tools_agent_spawn.go:418 — Handoff selects the first hook session matching only the runtime key. When profiles share a runtime, a Codex launch could claim a Claude session and submit incorrect identity evidence. Require the observed session’s agent to match the launched target and test multiple profiles sharing a runtime key.
internal/mcpserver/tools_agent_spawn.go:390 — Readiness polling handles only ready and error. A reused workspace in deleting, deletion_failed, or another terminal state waits until the five-to-fifteen-minute timeout. Fail immediately for deletion and unknown terminal states, including the recorded workspace error.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 16m25s

roborev-ci · 2026-08-19T16:43:05Z

roborev: Combined Review (`9fd7520`)

Code review identified four medium-severity issues; no high or critical findings.

Medium

internal/server/mcp_backend.go:132 — Repository identity validation and route-based execution are separate, so reconciliation may return or mutate a replacement repository. Perform downstream operations using the resolved repository ID, or hold a route fence across validation and execution.
internal/mcpserver/tools_stack.go:96 — A nonexistent PR is reported as unstacked because both cases use the same message-based notFound response. Verify PR existence and return pullNotFound; use a distinct structured code for unstacked state.
internal/mcpserver/types.go:134 — Notification activity reports "unread" as the referenced PR/issue state instead of the actual SubjectState, such as "merged". Prefer SubjectState when available and add notification-backed MCP coverage.
internal/server/workspaceapi/initial_message.go:274 — Delivered or uncertain prompts, each up to 64 KiB, remain in memory until workspace deletion or daemon restart, allowing unbounded retention across sessions. Add byte and entry limits with bounded retention while preserving evidence for pending or uncertain attempts.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 10m57s

roborev-ci · 2026-08-19T17:11:22Z

roborev: Combined Review (`199a040`)

High-severity repository identity race and medium-severity agent handoff reliability issue require fixes before merge.

High

internal/server/mcp_backend.go:570 — Stable repository identity is validated only during preflight. Subsequent route-based reads and workspace creation can race repository reconciliation and operate on a replacement at the same mutable route. Execute resolution and the operation under one repository-generation fence, or use service methods keyed by the resolved internal repository ID.

Medium

internal/mcpserver/tools_agent_spawn.go:498 — A multiline prompt can be rejected before bracketed-paste mode is observed, but this proven-no-write error immediately aborts the handoff and strands the launched runtime. Assign this condition a typed retryable code and retry submission against the same runtime within the handoff deadline.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 11m1s

roborev-ci · 2026-08-19T17:31:50Z

roborev: Combined Review (`26cb2fc`)

High-risk repository identity race must be resolved; two additional medium-severity lifecycle issues remain.

High

internal/server/mcp_backend.go:438 — Stable repository identity validation is separated from route-based reads and workspace mutations. Concurrent repository reconciliation can rename or reuse the route, causing MCP to read or launch an agent against a different repository.
- Fix: Bind the complete operation to a repository route-generation fence or use stable repository IDs throughout the service boundary. Add a route-reuse race test.

Medium

cmd/kenn-forge/main.go:511 — http.Server.Shutdown does not cancel active MCP requests. After the five-second timeout, primary services and the database can be torn down while a handoff is still using them.
- Fix: Cancel and fully drain active MCP handlers before teardown, force-close timed-out connections, and test shutdown during an active handoff.
internal/mcpserver/tools_agent_spawn.go:409 — waitForWorkspaceReady does not recognize terminal deletion_failed state, causing reuse to wait for the full handoff timeout even though the workspace cannot automatically become ready.
- Fix: Return an immediate workspace failure with the stored error details.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 14m31s

The approved design crosses daemon API, runtime, hook, persistence, and MCP boundaries. Record the executable TDD sequence and the current-brand naming decision before porting the existing review primitives and adding consequential agent spawning. The plan keeps one migration, suppresses provider-side assignment, and makes partial delivery state explicit so implementation can be reviewed in independently testable commits. Validation: context structure, placeholder scan, command review, and eight-task coverage check. Generated with Codex (gpt-5.6-sol) Co-authored-by: Codex <noreply@openai.com>

The repository-wide testify analyzer had begun rejecting every Go commit because several existing tests repeated package-level assertions instead of using local helpers. Normalize only those reported call sites so hook-enforced commits can proceed without changing test behavior.\n\nValidation: testify-helper-check ./...; focused affected Go tests across db, gitclone, github, and server. Generated with Codex (gpt-5.6-sol) Co-authored-by: Codex <noreply@openai.com>

MCP review automation needs a daemon-authoritative way to discover and transition local workflow state without inheriting provider-specific mutation behavior. This adds guarded PR and issue state reads and writes, including host-qualified identities and nested repository paths, so the companion server can remain a thin client.\n\nThe mutation contract requires either an expected state or an explicit force override, preserving conflict visibility for concurrent clients. Generated clients are updated with the new internal API contract.\n\nValidation: focused db/server/apitest workflow, Kanban, and Huma metadata tests; generated API artifacts; repository hooks. Generated with Codex (gpt-5.6-sol) Co-authored-by: Codex <noreply@openai.com>

Local automation currently has to scrape the UI or learn a broad internal REST surface to triage review work. Add a curated MCP companion that discovers the authenticated daemon lazily and exposes only cached repository, activity, item, diff, stack, and local workflow primitives.\n\nThe HTTP transport is loopback-only with independent bearer and origin checks, full diffs use companion-owned temporary files, and the tool surface intentionally performs no provider writes. Current Kenn Forge names are the only supported names.\n\nValidation: go test ./internal/mcpserver -shuffle=on; go mod tidy; context structure; repository hooks. Generated with Codex (gpt-5.6-sol) Co-authored-by: Codex <noreply@openai.com>

The initial companion surface could return inferred workflow state after a concurrent write, hide stack-evidence failures, and expose transport errors without enough information for an automation client to decide whether a mutation was safe to retry. Make committed workflow rows, exact reads, bounded stack enrichment, and structured ambiguity part of the contract so callers can act conservatively. Wire the companion through the public Cobra tree and published docs while preserving the historical July design as an unchanged record. The superseding design and execution plan capture the supported-agent, receipt-recovery, and hook-authority constraints for the remaining handoff work. Validation: focused MCP, CLI, DB, server, generated-client, docs-staging, and lint checks; hook-only MCP timeout reproduced as passing in seven fresh focused processes before retry. Generated with Codex (gpt-5.6-sol) Co-authored-by: Codex <noreply@openai.com>

Submitting text through a live terminal has no transactional acknowledgement, so retrying after a lost response can duplicate consequential agent work. Persist one metadata-only receipt before the first write and recover interrupted pending attempts as uncertain when the daemon restarts. Receipts deliberately outlive runtime-session row cleanup so callers can recover evidence after process exit; workspace deletion remains the ownership boundary that removes them. The schema stores neither prompt text nor a digest. Validation: go test ./internal/db -shuffle=on; golangci-lint ./internal/db; migration history check; context structure; repository hooks. The unrelated workspace-enrichment tmux failure passed six fresh focused reruns before this retry. Generated with Codex (gpt-5.6-sol) Co-authored-by: Codex <noreply@openai.com>

A pull request can contain a binary replacement, a rename-only change, or file metadata changes without a textual hunk. Treating an empty patch as an internal error makes one such file block the complete diff handoff to a coding agent. Write minimal Git-style change headers when text is unavailable and keep ordinary patches unchanged. The exported file now preserves the affected paths and binary or rename evidence without inventing file contents. Generated with Pi

Temporary diff files must follow the provider's repository identity rules. Lowercasing every owner and repository name makes distinct Forgejo or Gitea repositories share one filename and allows a later export to overwrite the earlier diff. Case-fold names only for providers that require lowercase repository identity. Keep host normalization and GitHub aliases canonical while preserving case-sensitive Forgejo, Gitea, and GitLab paths. Generated with Pi

Full diff exports are temporary, but a long-running daemon could retain one file for every pull request an automated client inspected. Per-file limits alone do not prevent those files from accumulating on disk. Add a request-based least-recently-used cache with a 128 MiB default. Persist `[mcp].diff_cache_mb`, require a restart when it changes, evict the oldest requested files before writes, and report when one diff cannot fit even after eviction. Generated with Pi

Repository owner and name are mutable routes. An MCP reference that keeps only that route can point at a different repository after a rename and route reuse, making an old item reference unsafe for reads or workflow mutations. Carry the provider-verified repository ID through every repository and item result. Require clients to echo it, validate it against the current route before service calls, and filter workflow reads by the stable ID so stale routes fail closed instead of selecting a replacement repository. Generated with Pi

A coding agent can stop accepting terminal input while Forge sends its first instruction. The previous write ignored the handoff deadline and held the session lock, so one blocked terminal could outlive the request and stall session lifecycle work. Release the session lock before writing and bound the write by the request deadline or 30 seconds. Preserve successful delivery when the client disconnects during a completed write, while a deadline after writing begins remains uncertain so Forge never sends the instruction twice. Generated with Pi

Two handoffs for the same pull request can both observe no workspace before either creation completes. The loser then receives a duplicate-workspace conflict even though the desired workspace is ready to reuse. Give that exact conflict a stable internal code and perform one authoritative pull read. Reuse the concurrent winner when present, while every other creation error and every ambiguous mutation still fails without retry. Generated with Pi

A coding-agent hook can arrive just before Forge observes the terminal's bracketed-paste mode. The first multiline instruction then sends no bytes, but the handoff previously stopped and left the new runtime without its task. Expose that exact no-write condition as a retryable internal code and poll the same runtime until the existing handoff deadline. No workspace or runtime is recreated, and every possible or confirmed write keeps the existing no-retry behavior. Generated with Pi

roborev-ci · 2026-08-19T18:22:11Z

roborev: Combined Review (`0fdd8ba`)

Review found four medium-severity concurrency and resource-management issues that should be addressed before merge.

Medium

internal/server/mcp_backend.go:47, 132, 438 — Stable repository identity is validated separately from route-only service calls. Concurrent route reuse could return another repository’s data or create a workspace against it. Propagate the expected stable ID or route fence through the operation and validate it within the same reconciliation critical section.
internal/server/mcp_backend.go:336, 361 — Workflow visibility is checked before a separate upsert. A sync could mark the item removed_upstream between these operations, allowing a prohibited mutation. Combine the visibility check and workflow update in a guarded transaction or conditional write.
internal/mcpserver/difftmp.go:48 — Cache eviction counts only payload bytes, so distinct empty diffs can create unlimited zero-byte files and map/LRU entries. Add an entry-count limit or charge a minimum size per entry.
cmd/kenn-forge/main.go:927 — MCP temporary-store cleanup runs synchronously without a shutdown deadline, so a slow RemoveAll could block database cleanup and exceed the daemon’s shutdown budget. Give cleanup an explicit budget and invoke it through runBoundedShutdown.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 17m56s

MCP diff-store removal runs before database cleanup. A stalled filesystem operation could otherwise block the remaining shutdown sequence indefinitely.\n\nBound this phase to one minute and include that allowance in daemon stop timing so later cleanup can still proceed. Generated with Pi

roborev-ci · 2026-08-19T20:16:31Z

roborev: Combined Review (`4592ebe`)

Review verdict: Two high-severity security/identity issues and two medium-severity session and memory issues require attention.

High

Provider-controlled prose can authorize privileged MCP actions — internal/mcpserver/tools_items.go:118, internal/mcpserver/tools_items.go:163
PR bodies, issue comments, and event text controlled by remote users are returned directly to the MCP model. Malicious instructions could induce a client to invoke kenn_forge_spawn_workspace_with_agent, launching a local agent with attacker-chosen instructions and access to the maintainer’s filesystem and network. Fence and label provider prose as untrusted, and require user-originated approval outside MCP-returned content before workspace or agent handoff tools execute.
Route reconciliation can switch repository identity after validation — internal/server/mcp_backend.go:132
Stable repository identity is validated separately from the subsequent route-based lookup. Reconciliation between these operations could cause the request to read or mutate another repository now occupying the same owner/name route. Use the resolved stable repository ID for service operations, or keep validation and execution within one reconciliation-safe boundary.

Medium

Agent prompt can be delivered to the wrong session — internal/mcpserver/tools_agent_spawn.go:425
waitForCodingSession matches only the runtime session key, allowing it to select another agent sharing that runtime. Match both runtime key and requested agent identity, with a focused multiple-agent test.
Initial-message attempts form an unbounded memory cache — internal/server/workspaceapi/initial_message.go:279
Completed and uncertain attempts retain full prompts until workspace deletion, allowing repeated launches in persistent workspaces to grow memory without bound. Add count/byte limits and expiry while retaining enough status or digest evidence for safe retries.

Reviewers: 2 done | Synthesis: codex, 13s | Total: 11m4s

Agent TUIs can treat pasted text and an adjacent Enter as one input burst. The instruction then remains in the input box while Forge reports it as delivered. Wait for bracketed-paste mode, paste the prompt, and require its terminal echo before sending Enter. This keeps handoff on the same runtime session without timing delays or duplicate attempts. Validation: launched Claude through the ephemeral MCP server and observed a single-line prompt submit and receive the requested reply without manual input. Generated with Pi

The bracketed-paste check ran before the terminal write with no ordering between them. A terminal that disabled paste mode in that window still received the multiline prompt, and the embedded newlines could execute as separate commands instead of one pasted instruction. Subscribe to live session output before validating paste mode. Broadcast updates mode state and delivers to subscribers under one lock, so every disable after the check is observable in the subscription. The echo wait and a non-blocking drain before Enter detect the disable sequence and abort without submitting. A disable seen after the framed write must report uncertain delivery, not a safe-to-retry rejection: the message bytes already reached the terminal, and a retry could deliver the prompt twice. Generated with Claude Code (claude-fable-5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The initial handoff test used a non-fatal assertion for an error condition. The repository's testify lint requires error assertions to stop the test before later state checks run. Use the existing require helper for the negative error match. Generated with Pi

The diff cache deleted the existing same-name file and evicted other entries before it wrote the replacement. Any failure while writing the replacement destroyed a valid cached diff without producing a new one, and concurrent readers could observe the published path missing. Stage the replacement in a temporary file first, evict other entries only after staging succeeds, and swap the published path with one atomic rename. A same-name entry is never evicted; its budget is reused by the replacement, so a failed write leaves every published diff and the cache accounting untouched. Generated with Claude Code (claude-fable-5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

When a file has no text patch, the exported diff synthesizes Git-style evidence. An empty added or deleted file produced only a bare header, so a reader could not tell what happened to it. A binary rename or copy suppressed the binary marker, hiding that the content also changed during the move. Emit /dev/null markers for empty added and deleted text files, and emit the binary-difference line for binary renames and copies alongside their rename and copy headers. Generated with Claude Code (claude-fable-5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

MCP validated the stable repository ID up front but then called services by the mutable owner/name route. If repository reconciliation reassigned that route between the check and the call, a read could return data from the replacement repository, and a workspace could be created for it. Reads now capture the route-ownership fence at validation and re-verify it after the service call; the fence generation changes on every ownership change, including route reuse, so a match proves the read saw only the validated repository. Workspace creation binds the fence to the request context so guarded database writes reject a mid-request reassignment before any row is persisted. Both paths fail closed with the existing identity-mismatch error. The duplicate pull-workspace conflict also gets a dedicated workspaceAlreadyExists problem code. The MCP reuse-recovery path matched the human-readable detail string, so a harmless rewording would have silently disabled concurrent workspace reuse. Generated with Claude Code (claude-fable-5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

roborev-ci · 2026-08-19T21:55:35Z

roborev: Combined Review (`b3ca902`)

The PR has two medium-severity correctness issues; no critical or high-severity findings were reported.

Medium

internal/workspace/localruntime/manager.go:2884 — If terminal output contains the prompt marker followed by the bracketed-paste disable sequence in the same chunk, the function consumes both and returns success. The subsequent drain misses the disable sequence, so Enter may be sent after paste mode is disabled. Treat any observed disable sequence as an uncertain failure before accepting the marker, and add coverage for both sequences arriving together.
internal/server/mcp_backend.go:438 — Stable repository identity is validated, but the resolved repository is discarded and downstream services resolve the mutable route again. A rename or route reuse between these operations could target a different repository. Pass the resolved internal identity into service calls, or hold the reconciliation read lock through validation and execution.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 14m28s

MCP handoffs can run for minutes, while daemon shutdown waits only a short grace period for the MCP HTTP server. http.Server.Shutdown does not cancel active handlers, so cleanup could close the MCP temp store and database beneath an in-flight handoff, producing partial operations with inconsistent failure evidence. MCP handlers now inherit a cancelable base context. Shutdown stops admission, waits the grace period, then cancels remaining handler contexts and force-closes their connections before later cleanup runs. The shutdown test now proves the database close still runs while an MCP cleanup stays blocked past its budget, and a new end-to-end test starts a real daemon with MCP enabled, connects through the discovered listener address, performs a tool call, and verifies both listeners are released on shutdown. Generated with Claude Code (claude-fable-5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The only client-boundary test for structured tool errors used a list operation with no partial output, so it never verified the core promise of failure evidence: a client can locate the workspace that a failed handoff already created. Exercise the spawn tool through a real client session, fail the runtime launch after workspace creation, and assert the structured error metadata and the partial handoff output both survive the wire. Generated with Claude Code (claude-fable-5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…acts Users enabling MCP with API authentication had no documented way to find or send the bearer token, so the likely outcomes were 401 responses or disabling authentication. Configurations written for the removed standalone stdio command would stop launching with no stated migration. Document where the daemon mints the token, how to reference it from a client configuration without pasting it into shared files, and that the old command and stdio transport are removed without a compatibility shim. Record the durable MCP invariants that landed with the recent fixes in the context topic doc: route-fence identity discipline, atomic diff-cache replacement and path lifetime, and the shutdown ordering contract. Generated with Claude Code (claude-fable-5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The workspace activity HTTP fixture launched a silent shell that never enabled bracketed-paste mode. Initial-message delivery now correctly waits for that terminal contract, so the fixture rejected its own handoff before testing activity state. Use a paste-aware, echoing test agent so the end-to-end route exercises successful delivery under the same protocol as real agent terminals. Generated with Pi

The MCP handoff retries multiline input only when the workspace service reports that the agent terminal's paste mode is not ready. Existing tests stubbed both sides of that boundary, so a regression in the translation from the real service signal to the retryable MCP error could silently drop multiline handoffs again. Drive a real runtime manager whose PTY never enables bracketed paste through mcpBackend.SubmitInitialMessage and assert the wire contract: initialMessageInputModeNotReady, retryable, and not ambiguous. Generated with Claude Code (claude-fable-5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The activity-route fixture only needs a terminal that advertises bracketed-paste mode and echoes the submitted prompt. Running that synthetic shell behind tmux made its terminal-mode escape dependent on tmux client rendering, leaving the test intermittently unable to observe paste readiness. Disable tmux agent sessions for this fixture so it exercises the HTTP and runtime handoff contract directly and deterministically. Generated with Pi

roborev-ci · 2026-08-19T22:34:20Z

roborev: Combined Review (`73d5797`)

Code review found two medium-severity runtime delivery issues; no high or critical findings.

Medium

internal/workspace/localruntime/manager.go:2955 — context.WithoutCancel allows terminal delivery to continue after MCP cancellation or daemon shutdown, potentially sending Enter during teardown. Derive the bounded write context from ctx while preserving completed-write evidence.
internal/workspace/localruntime/manager.go:2884 — A paste-disable sequence occurring after the echo marker in the same output chunk is discarded, so Enter may be sent after paste mode becomes inactive. Treat any disable observed after delivery begins as uncertain and add coverage for marker-plus-disable in one chunk.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 13m17s

Claude does not render bracketed-paste input before it receives Enter. The previous handoff waited for prompt text to appear in terminal output before sending Enter, so a real Claude session left both sides waiting until the write deadline expired. Send the complete bracketed-paste frame and Enter in one bounded terminal write after paste mode is observed. This matches the input sequence accepted by Claude and avoids coupling delivery to TUI rendering behavior. Validation: Spawned an authenticated Claude session through localruntime, submitted the initial message through SubmitInitialMessage, and observed the expected live model response. Generated with Pi

roborev-ci · 2026-08-20T00:18:35Z

roborev: Combined Review (`8dc895e`)

Changes requested: five medium-severity lifecycle and reliability issues remain.

Medium

internal/mcpserver/tools_agent_spawn.go:589 — handoffFailure drops backendErr.Retryable, causing recoverable launch failures to become permanently non-retryable. Copy the flag and assert it in the existing client-session failure test.
internal/mcpserver/difftmp.go:97 — Cache entries are deleted before the replacement rename succeeds, so a failed write can destroy previously published diffs. Make eviction rollback-safe or publish before committing evictions, with failure-injection coverage.
cmd/kenn-forge/main.go:887 — Shutdown waits up to five seconds for notification loops before stopping MCP admission, allowing new handoffs after shutdown begins. Stop the MCP HTTP listener immediately after stopping signals.
internal/workspace/localruntime/manager.go:2864 — context.WithoutCancel ignores client disconnect and shutdown cancellation after a terminal write starts, potentially delivering a prompt up to 30 seconds after cancellation. Observe the original context and classify cancellation after write start as uncertain.
internal/server/workspaceapi/initial_message.go:279 — Completed attempts retain full prompts indefinitely after runtime exit, causing unbounded daemon-memory growth across repeated launches. Bound or expire attempts while preserving a short recovery window.

Reviewers: 2 done | Synthesis: codex, 7s | Total: 16m40s

mariusvniekerk self-assigned this Aug 9, 2026

mariusvniekerk force-pushed the t3code/4c6152bf branch from f95b481 to 4e75578 Compare August 9, 2026 17:33

mariusvniekerk force-pushed the t3code/4c6152bf branch from 4e75578 to 4fea1e9 Compare August 13, 2026 04:14

mariusvniekerk force-pushed the t3code/4c6152bf branch from 919a974 to c05d80a Compare August 14, 2026 14:23

mariusvniekerk force-pushed the t3code/4c6152bf branch from c05d80a to 7d14fce Compare August 18, 2026 17:53

mariusvniekerk and others added 6 commits August 19, 2026 13:53

mariusvniekerk added 7 commits August 19, 2026 13:54

mariusvniekerk force-pushed the t3code/4c6152bf branch from 26cb2fc to 0fdd8ba Compare August 19, 2026 18:08

mariusvniekerk and others added 6 commits August 19, 2026 17:33

mariusvniekerk and others added 6 commits August 19, 2026 17:55

mariusvniekerk marked this pull request as ready for review August 20, 2026 01:35

mariusvniekerk merged commit c9da678 into main Aug 20, 2026
17 checks passed

mariusvniekerk deleted the t3code/4c6152bf branch August 20, 2026 01:35

Uh oh!

Conversation

mariusvniekerk commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roborev-ci Bot commented Aug 7, 2026

roborev: Combined Review (9c3a89e)

High

Medium

Uh oh!

roborev-ci Bot commented Aug 8, 2026

roborev: Combined Review (8fb095f)

Medium

Uh oh!

roborev-ci Bot commented Aug 8, 2026

roborev: Combined Review (f95b481)

Medium

Uh oh!

roborev-ci Bot commented Aug 9, 2026

roborev: Combined Review (4e75578)

High

Medium

Uh oh!

roborev-ci Bot commented Aug 13, 2026

roborev: Combined Review (4fea1e9)

High

Medium

Uh oh!

roborev-ci Bot commented Aug 13, 2026

roborev: Combined Review (9350288)

Medium

Uh oh!

roborev-ci Bot commented Aug 13, 2026

roborev: Combined Review (919a974)

High

Medium

Uh oh!

roborev-ci Bot commented Aug 14, 2026

roborev: Combined Review (c05d80a)

Medium

Uh oh!

roborev-ci Bot commented Aug 18, 2026

roborev: Combined Review (7d14fce)

High

Medium

Uh oh!

roborev-ci Bot commented Aug 19, 2026

roborev: Combined Review (41fec02)

High

Medium

Uh oh!

roborev-ci Bot commented Aug 19, 2026

roborev: Combined Review (398aa22)

High

Medium

Uh oh!

roborev-ci Bot commented Aug 19, 2026

roborev: Combined Review (8855fa9)

Medium

Uh oh!

roborev-ci Bot commented Aug 19, 2026

roborev: Combined Review (cc76005)

Medium

Uh oh!

roborev-ci Bot commented Aug 19, 2026

roborev: Combined Review (797f354)

High

Medium

Uh oh!

roborev-ci Bot commented Aug 19, 2026

roborev: Combined Review (809b6ca)

High

Medium

Uh oh!

roborev-ci Bot commented Aug 19, 2026

roborev: Combined Review (9fd7520)

Medium

Uh oh!

roborev-ci Bot commented Aug 19, 2026

roborev: Combined Review (199a040)

mariusvniekerk commented Aug 7, 2026 •

edited

Loading

roborev: Combined Review (`9c3a89e`)

roborev: Combined Review (`8fb095f`)

roborev: Combined Review (`f95b481`)

roborev: Combined Review (`4e75578`)

roborev: Combined Review (`4fea1e9`)

roborev: Combined Review (`9350288`)

roborev: Combined Review (`919a974`)

roborev: Combined Review (`c05d80a`)

roborev: Combined Review (`7d14fce`)

roborev: Combined Review (`41fec02`)

roborev: Combined Review (`398aa22`)

roborev: Combined Review (`8855fa9`)

roborev: Combined Review (`cc76005`)

roborev: Combined Review (`797f354`)

roborev: Combined Review (`809b6ca`)

roborev: Combined Review (`9fd7520`)

roborev: Combined Review (`199a040`)

roborev: Combined Review (`26cb2fc`)

roborev: Combined Review (`0fdd8ba`)

roborev: Combined Review (`4592ebe`)

roborev: Combined Review (`b3ca902`)

roborev: Combined Review (`73d5797`)

roborev: Combined Review (`8dc895e`)