WIP Add Critical Carl review report

[aivong-openhands]

HUMAN:

• A human has tested these changes.

AGENT:

Documentation-only change. Applies the Critical Carl subtract-first lens to the agent-canvas repo, mirroring OpenHands/integrations-hub#129 but targeted at this codebase. Every finding cites a real file/line and was verified against the working tree (see commands in How to Test).

---

Why

The repo has strong behavior/regression coverage and docs, but no subtract-first review of structural complexity. Carl's rule — delete → simplify → reuse → add — surfaces a few oversized control-plane files, vendor trivia in generic layers, a junk-drawer utils.ts, and a large legacy/compatibility surface that documentation explains but nothing schedules for deletion.

Summary

• Add CRITICAL_CARL_REVIEW.md with a repository-wide Critical Carl review pass and concrete file:line evidence.
• Compare the findings against the existing specs/ tracking set and call out what is untracked.
• Record the strongest untracked gaps in AGENTS.md for future work.

Issue Number

How to Test

Documentation-only; no runtime behavior changes. The findings are reproducible from the repo root:

# Finding 1 — websocket god object size + store coupling
wc -l src/contexts/conversation-websocket-context.tsx
grep -cE 'use[A-Z][a-zA-Z]*Store|useQueryClient|usePostHog' src/contexts/conversation-websocket-context.tsx

# Finding 2/3 — adapter and utils sizes
wc -l src/api/agent-server-adapter.ts src/utils/utils.ts
grep -cE '^export ' src/utils/utils.ts

# Finding 4 — vendor-specific entry.id patches in the generic MCP catalog
grep -nE 'function patch|entry.id|getDeploymentMode' src/utils/mcp-marketplace-utils.ts

# Finding 5 — legacy/compatibility surface count
grep -rniE 'legacy|deprecat|obsolete|backward[- ]?compat' src --include='*.ts' --include='*.tsx' | wc -l

Each command's output matches the numbers cited in CRITICAL_CARL_REVIEW.md.

Video/Screenshots

N/A — no UI change.

Type

• Bug fix
• Feature
• Refactor
• Breaking change
• Docs / chore

Notes

No code paths, dependencies, or tests are touched — only two Markdown files (CRITICAL_CARL_REVIEW.md new, AGENTS.md one note appended). The review proposes deletion-oriented follow-ups but performs none of them in this PR.

---

This PR was created by an AI agent (OpenHands) on behalf of the user.

@aivong-openhands can click here to continue refining the PR

---

🐳 Docker images for this PR

• GHCR package: https://github.com/OpenHands/agent-canvas/pkgs/container/agent-canvas

Component	Value
Image	ghcr.io/openhands/agent-canvas
Architectures	amd64, arm64
Agent Server	ghcr.io/openhands/agent-server:1.28.1-python
Automation	openhands-automation==1.0.0a9
Commit	a5b361085eaad76da342e80d9203c656f1755d32

Pull (multi-arch manifest)

# Multi-arch manifest — Docker automatically pulls the correct architecture
docker pull ghcr.io/openhands/agent-canvas:sha-a5b3610

Run

docker run -it --rm \
  -p 8000:8000 \
  ghcr.io/openhands/agent-canvas:sha-a5b3610

All tags pushed for this build

ghcr.io/openhands/agent-canvas:sha-a5b3610-amd64
ghcr.io/openhands/agent-canvas:carl-critical-carl-review-pass-amd64
ghcr.io/openhands/agent-canvas:pr-1334-amd64
ghcr.io/openhands/agent-canvas:sha-a5b3610-arm64
ghcr.io/openhands/agent-canvas:carl-critical-carl-review-pass-arm64
ghcr.io/openhands/agent-canvas:pr-1334-arm64
ghcr.io/openhands/agent-canvas:sha-a5b3610
ghcr.io/openhands/agent-canvas:carl-critical-carl-review-pass
ghcr.io/openhands/agent-canvas:pr-1334

About Multi-Architecture Support

• Each tag (e.g., sha-a5b3610) is a multi-arch manifest supporting both amd64 and arm64
• Docker automatically pulls the correct architecture for your platform
• Individual architecture tags (e.g., sha-a5b3610-amd64) are also available if needed

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agent-canvas	Ready	Preview, Comment	Jun 12, 2026 6:25pm

[github-actions]

✅ Mock-LLM E2E Tests

53/53 passed

Commit: a5b36108 · Workflow run · Test artifacts

Status	Test	Duration
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 1: configure ACP agent via Settings → Agent UI	13.7s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 2: reload and verify ACP settings are persisted in UI	5.6s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 3: start ACP conversation and verify agent reply	6.4s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 4: resume ACP conversation from sidebar after navigating away	5.7s
✅	mock-llm-auth-modes.spec.ts › auth mode: fresh install with runtime-injected key › reaches the onboarding modal without pre-seeded localStorage	1.4s
✅	mock-llm-auth-modes.spec.ts › auth mode: non-public key rotation › recovers when localStorage has a stale session API key	5.3s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configured	1.2s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › rejects an incorrect key with an inline error	1.4s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › allows access after pasting the correct key	1.7s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › skips auth screen for returning user with valid stored key	773ms
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › re-prompts when the server rotates its key (stale localStorage)	1.5s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 1: setup LLM profile and register automation trajectory	7.2s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 2: create automation and dispatch run via the UI	30.5s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 3: verify automation and run on the automations page	6.3s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 1: create an LLM profile pointing at the mock LLM server	6.2s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 2: activate the mock-llm profile and verify settings API	6.2s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 3: run a conversation with the mock LLM	6.6s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 4: resume conversation from sidebar after navigating away	5.8s
✅	mock-llm-cross-connect.spec.ts › cross-connect: frontend-only → backend-only › frontend-only connects to a separate backend-only instance	15.9s
✅	mock-llm-cross-connect.spec.ts › cross-connect: frontend-only → multiple backends › connects to two separate backends and switches between them	19.7s
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 1: ensure mock LLM profile is configured	169ms
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 2: start conversation and attach workspace metadata	11.6s
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 3: git control bar shows workspace pill and git actions	25.4s
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 4: files tab defaults to diff view for attached workspace	6.0s
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 5: browser tab shows empty state	6.4s
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 6: files tab defaults to file-tree view without attached workspace	7.5s
✅	mock-llm-folder-workspace.spec.ts › mock-LLM folder browser → workspace → conversation › step 1: browse to a folder, add it as a workspace, and launch a conversation with the correct working_dir	7.8s
✅	mock-llm-image-upload.spec.ts › mock-LLM image upload › attaching an image embeds it as base64 in the LLM completion call	13.4s
✅	mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 1: GitHub card is visible on the MCP marketplace page	5.6s
✅	mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 2: clicking GitHub card opens the install modal with correct fields	5.7s
✅	mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 3: full install flow — fill PAT, submit, verify installed	12.8s
✅	mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 4: installed GitHub server can be deleted	5.8s
✅	mock-llm-model-switch.spec.ts › mock-LLM /model slash command › step 1: configure LLM, create switch-target profile, register trajectory	13.1s
✅	mock-llm-model-switch.spec.ts › mock-LLM /model slash command › step 2: start conversation, switch profile via /model, verify switch	6.9s
✅	mock-llm-onboarding-happy-path.spec.ts › onboarding happy path › completes the full onboarding flow and launches a conversation	4.6s
✅	mock-llm-onboarding-regressions.spec.ts › onboarding recent regressions › keeps the modal open on backdrop click and Escape	1.4s
✅	mock-llm-onboarding-regressions.spec.ts › onboarding recent regressions › defaults the LLM setup step to OpenAI GPT-5.5	1.7s
✅	mock-llm-partial-stack.spec.ts › partial stack: --frontend-only › serves the frontend but returns 503 for backend routes	7.4s
✅	mock-llm-partial-stack.spec.ts › partial stack: --backend-only › serves backend APIs but returns 503 for the frontend root	13.1s
✅	mock-llm-partial-stack.spec.ts › partial stack: port conflict › fails with a clear error when the ingress port is occupied	104ms
✅	mock-llm-partial-stack.spec.ts › partial stack: port conflict › starts successfully on a free port after a conflict	6.0s
✅	mock-llm-preset-automation.spec.ts › preset automation → slash command conversation › automation card sends the correct slash command to a conversation	17.6s
✅	mock-llm-preset-automation.spec.ts › preset automation → slash command conversation › direct slash command from home page triggers skill activation	13.6s
✅	mock-llm-profile-management.spec.ts › active profile deletion + reconciliation › active profile is deletable and reconciliation activates another profile	8.5s
✅	mock-llm-profile-management.spec.ts › same-model profile identity › chat header shows the correct profile when two profiles share the same model	15.2s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › project skill in workspace/.agents/skills/ triggers on matching keyword	13.7s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › user skill in ~/.openhands/skills/ triggers on matching keyword	13.5s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › deleting a user skill removes it from subsequent conversations	13.6s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › scopes standalone styles to the agent-server-ui shell	1.3s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › renders critic results on agent messages and finish actions	1.5s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › loads older events when scrolling up	1.7s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › selected workspace persists after navigating away and returning	2.1s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › cleared sessionStorage yields empty workspace selection	995ms

_{Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses)}

[github-actions]

🔶 Mock-LLM Docker E2E Test Results

48/53 passed · 5 skipped

Commit: a5b36108 · Workflow run · Test artifacts

Status	Test	Duration
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 1: configure ACP agent via Settings → Agent UI	13.9s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 2: reload and verify ACP settings are persisted in UI	5.5s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 3: start ACP conversation and verify agent reply	6.7s
✅	mock-llm-acp-agent.spec.ts › mock-LLM ACP agent conversation › step 4: resume ACP conversation from sidebar after navigating away	5.7s
✅	mock-llm-auth-modes.spec.ts › auth mode: fresh install with runtime-injected key › reaches the onboarding modal without pre-seeded localStorage	1.3s
✅	mock-llm-auth-modes.spec.ts › auth mode: non-public key rotation › recovers when localStorage has a stale session API key	5.3s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configured	1.2s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › rejects an incorrect key with an inline error	1.3s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › allows access after pasting the correct key	1.7s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › skips auth screen for returning user with valid stored key	1.2s
✅	mock-llm-auth-modes.spec.ts › auth mode: public gate › re-prompts when the server rotates its key (stale localStorage)	1.5s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 1: setup LLM profile and register automation trajectory	7.2s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 2: create automation and dispatch run via the UI	34.5s
✅	mock-llm-automation.spec.ts › mock-LLM automation lifecycle › step 3: verify automation and run on the automations page	6.2s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 1: create an LLM profile pointing at the mock LLM server	6.3s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 2: activate the mock-llm profile and verify settings API	6.2s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 3: run a conversation with the mock LLM	6.4s
✅	mock-llm-conversation.spec.ts › mock-LLM agent-server conversation › step 4: resume conversation from sidebar after navigating away	5.7s
⏭️	mock-llm-cross-connect.spec.ts › cross-connect: frontend-only → backend-only › frontend-only connects to a separate backend-only instance	198ms
⏭️	mock-llm-cross-connect.spec.ts › cross-connect: frontend-only → multiple backends › connects to two separate backends and switches between them	189ms
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 1: ensure mock LLM profile is configured	200ms
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 2: start conversation and attach workspace metadata	11.5s
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 3: git control bar shows workspace pill and git actions	25.3s
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 4: files tab defaults to diff view for attached workspace	5.9s
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 5: browser tab shows empty state	6.3s
✅	mock-llm-files-and-git.spec.ts › files tab, git control bar, and browser tab › step 6: files tab defaults to file-tree view without attached workspace	7.2s
✅	mock-llm-folder-workspace.spec.ts › mock-LLM folder browser → workspace → conversation › step 1: browse to a folder, add it as a workspace, and launch a conversation with the correct working_dir	7.3s
✅	mock-llm-image-upload.spec.ts › mock-LLM image upload › attaching an image embeds it as base64 in the LLM completion call	13.3s
✅	mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 1: GitHub card is visible on the MCP marketplace page	5.5s
✅	mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 2: clicking GitHub card opens the install modal with correct fields	5.7s
✅	mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 3: full install flow — fill PAT, submit, verify installed	12.9s
✅	mock-llm-mcp-github.spec.ts › MCP GitHub server install flow › step 4: installed GitHub server can be deleted	5.8s
✅	mock-llm-model-switch.spec.ts › mock-LLM /model slash command › step 1: configure LLM, create switch-target profile, register trajectory	13.2s
✅	mock-llm-model-switch.spec.ts › mock-LLM /model slash command › step 2: start conversation, switch profile via /model, verify switch	6.7s
✅	mock-llm-onboarding-happy-path.spec.ts › onboarding happy path › completes the full onboarding flow and launches a conversation	3.6s
✅	mock-llm-onboarding-regressions.spec.ts › onboarding recent regressions › keeps the modal open on backdrop click and Escape	1.4s
✅	mock-llm-onboarding-regressions.spec.ts › onboarding recent regressions › defaults the LLM setup step to OpenAI GPT-5.5	1.8s
⏭️	mock-llm-partial-stack.spec.ts › partial stack: --frontend-only › serves the frontend but returns 503 for backend routes	218ms
✅	mock-llm-partial-stack.spec.ts › partial stack: --backend-only › serves backend APIs but returns 503 for the frontend root	26.2s
⏭️	mock-llm-partial-stack.spec.ts › partial stack: port conflict › fails with a clear error when the ingress port is occupied	0ms
⏭️	mock-llm-partial-stack.spec.ts › partial stack: port conflict › starts successfully on a free port after a conflict	2ms
✅	mock-llm-preset-automation.spec.ts › preset automation → slash command conversation › automation card sends the correct slash command to a conversation	16.4s
✅	mock-llm-preset-automation.spec.ts › preset automation → slash command conversation › direct slash command from home page triggers skill activation	13.3s
✅	mock-llm-profile-management.spec.ts › active profile deletion + reconciliation › active profile is deletable and reconciliation activates another profile	8.6s
✅	mock-llm-profile-management.spec.ts › same-model profile identity › chat header shows the correct profile when two profiles share the same model	15.0s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › project skill in workspace/.agents/skills/ triggers on matching keyword	13.5s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › user skill in ~/.openhands/skills/ triggers on matching keyword	13.2s
✅	mock-llm-skills.spec.ts › skill loading: project, user, and deletion › deleting a user skill removes it from subsequent conversations	13.2s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › scopes standalone styles to the agent-server-ui shell	1.3s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › renders critic results on agent messages and finish actions	1.4s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › loads older events when scrolling up	1.6s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › selected workspace persists after navigating away and returning	2.0s
✅	mock-llm-ui-regressions.spec.ts › UI regressions › cleared sessionStorage yields empty workspace selection	1.3s

_{Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses)}

[aivong-openhands]

How the 3 follow-up issues were selected

These follow-up issues were derived by reverse-engineering the methodology that turned OpenHands/integrations-hub#127's Critical Carl review into exactly 3 issues from 6 findings (#134 / #135 / #136), then applying the same filter here.

The selection filter

A review finding becomes a new issue only if all three hold:

1. Untracked — a duplicate search against the existing issue set finds no equivalent. (Each created issue documents this "Duplicate check".)
2. Bounded subtraction — a crisp remove X / narrow Y / delete Z surface / dedup task with a concrete target, not an open-ended "decompose this 1,700-line god object" rewrite.
3. Not meta — not a prioritization/process observation.

Why integrations-hub got 3, not 6

integrations-hub finding	Outcome	Reason
1. ContextLayerService god object	❌ no issue	open-ended decomposition, already in broad refactor #125
2. dashboard controller	❌ no issue	open-ended, already in #125
3. HTTP adapter DSL	✅ #135	bounded + untracked
4. provider transport leak	✅ #134	bounded + untracked
5. built-in connector compat	✅ #136	bounded + untracked
6. "issue set leans to analysis"	❌ no issue	meta / priority observation

The big open-ended refactors were left to the existing broad-refactor bucket; the meta finding produced no work item. Only the bounded, untracked subtractions became issues.

Applying the same filter to this review (CRITICAL_CARL_REVIEW.md)

Finding in this PR	Outcome	Reason
1. ConversationWebSocketProvider god object (1086 LOC)	❌ skip	open-ended decomposition (mirrors ih #1/#2)
2. agent-server-adapter.ts doing 4 jobs (988 LOC)	❌ skip	open-ended split
3. utils.ts junk drawer (859 LOC)	❌ skip	open-ended reorg, net-zero deletion
4. vendor entry.id MCP catalog patches	✅ #1336	bounded subtraction (direct analog of ih #134)
5. legacy/compatibility surface (~83 refs)	✅ #1337	bounded subtraction (direct analog of ih #136)
6. controllers + duplicated form state	✅ #1338	only the bounded dedup slice; the controller decomposition is excluded

Landing on 3 is a result of the filter, not a forced count. Notably there is no agent-canvas analog of ih #135's "hidden interpolation DSL", so a third issue of that kind doesn't exist — but the duplicated BackendForm/ManualConnectionColumn state block is a clean bounded dedup that qualifies on its own.

The three open-ended decompositions (findings 1–3) intentionally stay documented as structural debt in CRITICAL_CARL_REVIEW.md rather than becoming vague "rewrite this big file" tickets — that's the same choice integrations-hub made for its god-object findings.

Resulting issues

• Remove vendor-specific entry.id transport patches from the generic MCP catalog #1336 — Remove vendor-specific entry.id transport patches from the generic MCP catalog
• Schedule deletion of the legacy/compatibility surface (drain-once-then-delete) #1337 — Schedule deletion of the legacy/compatibility surface (drain-once-then-delete)
• Delete duplicated backend-form state; reuse a shared form hook #1338 — Delete duplicated backend-form state; reuse a shared form hook

Each ran the required duplicate check (all empty), references this review, and uses existing repo labels.

One deviation from the integrations-hub flow: their follow-ups referenced a parent "run Carl review" issue (#127). Here I referenced this review PR instead of creating a parent tracker, since the request itself played that role. Happy to add a parent tracker issue and link the three under it if that's preferred.

---

This comment was created by an AI agent (OpenHands) on behalf of the user.

[enyst]

😅