Skip to content

refactor(agent): retire legacy agentpresenter#1379

Merged
zerob13 merged 2 commits intodevfrom
codex/plan-and-archive-legacy-agentpresent
Mar 23, 2026
Merged

refactor(agent): retire legacy agentpresenter#1379
zerob13 merged 2 commits intodevfrom
codex/plan-and-archive-legacy-agentpresent

Conversation

@zerob13
Copy link
Copy Markdown
Collaborator

@zerob13 zerob13 commented Mar 23, 2026

Summary by CodeRabbit

  • Documentation

    • Updated architecture, flows, guides, and onboarding to reflect the legacy AgentPresenter retirement (completed 2026-03-23) and added archived legacy references.
  • Refactor

    • Migrated retained ACP/tool/message-format helpers into their new presenter homes and removed legacy runtime surface and stream-completion APIs.
  • Tests

    • Adjusted tests to reference the reorganized presenter/module layout.
  • Chores

    • Archived legacy runtime code and added retirement/guarding documentation.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 23, 2026

📝 Walkthrough

Walkthrough

This pull request finalizes retirement of the legacy AgentPresenter runtime: it migrates ACP/helpers and agent-tool code into new presenter modules, removes legacy runtime wiring and streaming APIs, archives retired code, updates presenter types, and rewrites architecture/flow documentation to the post‑retirement design.

Changes

Cohort / File(s) Summary
Architecture & Specs
docs/ARCHITECTURE.md, docs/FLOWS.md, docs/README.md, docs/specs/agent-cleanup/*, docs/specs/legacy-agentpresenter-retirement/*
Rewrote architecture and flows to reflect post‑retirement main path (newAgentPresenter → AgentRegistry → deepchatAgentPresenter); added retirement plan/spec/tasks and moved legacy docs to archives.
Archives
docs/archives/legacy-agentpresenter-architecture.md, docs/archives/legacy-agentpresenter-flows.md, archives/code/legacy-agentpresenter-retirement/README.md
Added archival docs capturing original AgentPresenter architecture and flows; added archive README with retirement metadata.
Presenter core & wiring
src/main/presenter/index.ts, src/main/presenter/llmProviderPresenter/index.ts, src/main/presenter/llmProviderPresenter/managers/..., src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
Removed legacy presenter fields and AgentLoop/stream-completion surface; simplified LLMProviderPresenter constructor; centralized ACP re-exports under llmProviderPresenter/acp.
Tool system & agent tools
src/main/presenter/toolPresenter/index.ts, src/main/presenter/toolPresenter/runtimePorts.ts, src/main/presenter/toolPresenter/agentTools/*
Introduced AgentToolRuntimePort, consolidated agentTools with new index exports, moved agent tool helpers to new module paths, and updated imports to shared agentRuntime helpers.
Agent tool handlers (impl)
src/main/presenter/toolPresenter/agentTools/agentFileSystemHandler.ts, .../agentBashHandler.ts, .../agentToolManager.ts
Enhanced filesystem authorization (allowedDirectoryRoots, macOS alias handling), improved error propagation, switched runtime imports to @/lib/agentRuntime/*, and removed optional-fallbacks for runtime port calls.
Session & export presenters
src/main/presenter/sessionPresenter/index.ts, src/main/presenter/exporter/formats/*
Switched message formatter imports to sessionPresenter location, removed legacy runtime status derivation (status now 'idle'), and routed cleanup to unified cleanupConversationRuntimeArtifacts.
Tab & sync presenters
src/main/presenter/tabPresenter.ts, src/main/presenter/syncPresenter/index.ts
Updated to use unified presenter methods: getActiveConversationIdSync, cleanupConversationRuntimeArtifacts, and broadcastConversationThreadListUpdate.
ACP package & helpers
src/main/presenter/llmProviderPresenter/acp/index.ts, .../acpProcessManager.ts, .../providers/acpProvider.ts
Added acp index re-exports; changed internal acp imports to new acp path and to shared shellEnvHelper where applicable.
Types & public surface
src/shared/types/presenters/index.d.ts, .../legacy.presenters.d.ts, .../llmprovider.presenter.d.ts
Removed IAgentPresenter re-export and removed legacy sessionPresenter/agentPresenter fields and ILlmProviderPresenter.startStreamCompletion signature from presenter type surfaces.
Scripts & guard
scripts/agent-cleanup-guard.mjs
Excluded archived agentPresenter path from scan roots and added ENOENT-aware error handling when collecting files.
Tests
test/main/presenter/**, test/main/lib/**, archives/.../test/**
Updated many test import paths to new module locations (llmProviderPresenter/acp, toolPresenter/agentTools, sessionPresenter, @/lib/agentRuntime). Removed stream-completion/stream-management test suites and adjusted LLMProviderPresenter tests to new interface.
Misc presenters & helpers
src/main/presenter/configPresenter/acpInitHelper.ts, src/main/presenter/exporter/formats/*, src/main/presenter/syncPresenter/index.ts
Switched imports to shared agentRuntime helpers and sessionPresenter message formatter; minor call-site updates to new presenter APIs.
Archive tests adjustments
archives/code/legacy-agentpresenter-retirement/test/**, test/.../agentRuntime/*
Moved/updated archive test imports to point at new helper locations where applicable.

Sequence Diagram(s)

sequenceDiagram
  participant Renderer as Renderer
  participant NewAP as newAgentPresenter
  participant Registry as AgentRegistry
  participant Deep as deepchatAgentPresenter
  participant Tool as ToolPresenter
  participant LLM as LLMProviderPresenter
  participant DB as SQLitePresenter

  Renderer->>NewAP: create/bind session, enqueue message
  NewAP->>Registry: register/session routing
  Registry->>Deep: dispatch message -> processMessage
  Deep->>LLM: build context -> request completion
  LLM->>LLM: generateCompletion (no startStreamCompletion)
  Deep->>Tool: pre-check / dispatch tool call
  Tool-->>Deep: tool result (or pause/resume)
  Deep->>DB: persist events/messages
  Deep-->>NewAP: finalize/persist session state
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 The warren cheers—old burrows sealed with care,
New tunnels carved, with paths laid clean and fair.
ACP helpers hop to brighter halls,
While legacy maps rest in archive walls.
~A rabbit's hop to cleaner architecture

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the primary change: retiring the legacy agentpresenter runtime and migrating its functionality to other presenters.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/plan-and-archive-legacy-agentpresent

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/presenter/toolPresenter/agentTools/agentFileSystemHandler.ts (1)

288-325: ⚠️ Potential issue | 🔴 Critical

Do not drop the child Access denied result when the parent resolves.

If fs.realpath(normalizedRequested) hits the “symlink target outside allowed directories” guard, Line 314 still returns normalizedRequested whenever the parent directory is allowed. A symlinked file inside an allowed directory can then be edited or overwritten outside the allowlist. The parent-directory fallback should only run for “path does not exist yet” cases.

🔒 Proposed fix
     } catch (error) {
       pathResolutionError = error
+      if (
+        pathResolutionError instanceof Error &&
+        pathResolutionError.message.startsWith('Access denied')
+      ) {
+        throw pathResolutionError
+      }
+
+      const { code } = pathResolutionError as NodeJS.ErrnoException
+      if (code !== undefined && code !== 'ENOENT' && code !== 'ENOTDIR') {
+        throw pathResolutionError
+      }
       const parentDir = path.dirname(normalizedRequested)
       try {
         const realParentPath = await fs.realpath(parentDir)
         const normalizedParent = this.normalizePath(realParentPath)
         if (enforceAllowed) {
           const isParentAllowed = this.isPathAllowed(normalizedParent)
           if (!isParentAllowed) {
             throw new Error('Access denied - parent directory outside allowed directories')
           }
         }
         return normalizedRequested
       } catch (parentError) {
-        if (
-          pathResolutionError instanceof Error &&
-          pathResolutionError.message.startsWith('Access denied')
-        ) {
-          throw pathResolutionError
-        }
         if (parentError instanceof Error && parentError.message.startsWith('Access denied')) {
           throw parentError
         }
         throw new Error(`Parent directory does not exist: ${parentDir}`)
       }
     }

A regression test with a symlinked file under an allowed root would be worth adding here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/presenter/toolPresenter/agentTools/agentFileSystemHandler.ts` around
lines 288 - 325, When resolving normalizedRequested in the fs.realpath
try/catch, do not swallow an "Access denied - symlink target outside allowed
directories" error from the initial realpath attempt; only fall back to checking
the parent when the original error is a non-existence error (e.g.,
ENOENT/ENOTDIR). Concretely, inside the catch for the first fs.realpath (where
pathResolutionError is set), change the guard before attempting
realpath(parentDir) so that you rethrow pathResolutionError if it is an Error
whose message or errno indicates access-denied (the same condition used later)
and only run the parent-resolution fallback when pathResolutionError indicates
the path does not exist; keep using the existing symbols normalizedRequested,
pathResolutionError, parentDir, realParentPath, normalizedParent, and
isPathAllowed/isParentAllowed and return normalizedRequested only in the allowed
non-existence case.
🧹 Nitpick comments (1)
test/main/presenter/llmProviderPresenter/acpContentMapper.test.ts (1)

3-3: Move this spec to mirror the new acp/ source layout.

The SUT now lives under src/main/presenter/llmProviderPresenter/acp/, but this spec is still in the old flat location. Moving it to test/main/presenter/llmProviderPresenter/acp/acpContentMapper.test.ts will keep the test tree aligned with the source tree.

As per coding guidelines, test/**/*.{test,spec}.ts files should mirror source structure under test/main/** and test/renderer/**.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/main/presenter/llmProviderPresenter/acpContentMapper.test.ts` at line 3,
The test file is located in the old flat test tree and should mirror the source
layout; move the spec for AcpContentMapper to the matching test directory under
test/main/presenter/llmProviderPresenter/acp/ (so the test path mirrors
src/main/presenter/llmProviderPresenter/acp/), update any import paths in the
test that reference AcpContentMapper to the relative path from the new location,
and run the test suite to confirm imports resolve and the spec executes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/archives/legacy-agentpresenter-architecture.md`:
- Around line 312-317: The relative links in
docs/archives/legacy-agentpresenter-architecture.md point to ./architecture/...
and ./FLOWS.md which resolve under docs/archives/ and 404; update those list
entries (the lines referencing "architecture/session-management.md",
"architecture/agent-system.md", "architecture/tool-system.md",
"architecture/event-system.md", "FLOWS.md", and
"architecture/mcp-integration.md") to use the correct relative path up one level
(e.g., ../architecture/... and ../FLOWS.md) so they resolve from docs/ not
docs/archives/.
- Around line 121-130: Update the quick-reference table entries (AgentPresenter,
agentLoopHandler, streamGenerationHandler, loopOrchestrator, toolCallProcessor,
llmEventHandler, permissionHandler, messageBuilder) so their file locations
point to the archived tree or explicitly mark the listed paths as historical
(e.g., prefix with "archives/code/legacy-agentpresenter-retirement/..." or
append "(historical, retired runtime)"); apply the same change to the other
referenced sections at lines noted in the comment (146-153 and 293-303) so
readers are not directed to live src/main/presenter/agentPresenter paths that no
longer exist.

In `@docs/archives/legacy-agentpresenter-flows.md`:
- Line 248: Multiple top-level section headings are misnumbered (duplicate "##
3. 工具调用路由流程" and other "## 3" occurrences), which shifts navigation; locate the
repeated heading text "## 3. 工具调用路由流程" and the other duplicate "## 3" headings
(also the occurrences around the later sections noted) and renumber them to the
correct sequential major section numbers so all major headings increment
consistently (e.g., adjust the repeated "## 3" entries to "## 4", "## 5", "## 6"
as appropriate to restore the intended sequence).

In `@docs/guides/getting-started.md`:
- Line 128: The sentence "补跑相关 Vitest 套件和:" is a dangling conjunction; edit the
line containing that exact text to finish the thought by removing the trailing
"和:" and either list the missing item(s) or rephrase to a complete phrase such
as "补跑相关 Vitest 套件:" or "补跑相关 Vitest 套件以及相关步骤:" so the sentence reads complete
and clear for new contributors.

In `@scripts/agent-cleanup-guard.mjs`:
- Around line 106-111: The collectFiles function currently swallows all
fs.access() failures and returns [] which hides missing/renamed/unreadable
protected roots; change it to stop treating every access error as ignorable by
removing the blanket catch-return and instead let the error propagate (rethrow)
so callers can handle it, or explicitly handle only expected optional cases
(e.g., check err.code === 'ENOENT' if you truly want to treat missing roots as
optional). Update collectFiles (the async function) and adjust its callers such
as scanRoots to decide explicitly whether a missing root is acceptable rather
than collectFiles silently returning an empty array.

In `@src/main/presenter/sessionPresenter/index.ts`:
- Around line 1049-1051: toSession() currently hardcodes status: 'idle', which
hides runtime state for consumers like getSession() and getSessionList(); change
to derive the status instead of flattening it—either return the live status from
the conversation object (e.g. conversation.status or conversation.state) or
query the new runtime owner API (e.g.
runtimeOwner.getSessionStatus(conversation.id)) and use that value, or if the
field is being removed, remove/deprecate Session.status from the shared contract
and update callers accordingly; update the toSession() return to use that
derived value rather than the string literal 'idle'.

In `@src/main/presenter/toolPresenter/runtimePorts.ts`:
- Around line 21-22: Change the port contract so approval hooks are required:
remove the optional markers from getApprovedFilePaths and
consumeSettingsApproval (make them getApprovedFilePaths(conversationId: string):
string[] and consumeSettingsApproval(conversationId: string, toolName: string):
boolean) and update all implementations to provide explicit fail-closed behavior
(return [] for no approved paths and return false for no settings approval) so
callers can rely on non-optional methods; update any implementing classes/types
that mention getApprovedFilePaths or consumeSettingsApproval to match the new
signatures.

---

Outside diff comments:
In `@src/main/presenter/toolPresenter/agentTools/agentFileSystemHandler.ts`:
- Around line 288-325: When resolving normalizedRequested in the fs.realpath
try/catch, do not swallow an "Access denied - symlink target outside allowed
directories" error from the initial realpath attempt; only fall back to checking
the parent when the original error is a non-existence error (e.g.,
ENOENT/ENOTDIR). Concretely, inside the catch for the first fs.realpath (where
pathResolutionError is set), change the guard before attempting
realpath(parentDir) so that you rethrow pathResolutionError if it is an Error
whose message or errno indicates access-denied (the same condition used later)
and only run the parent-resolution fallback when pathResolutionError indicates
the path does not exist; keep using the existing symbols normalizedRequested,
pathResolutionError, parentDir, realParentPath, normalizedParent, and
isPathAllowed/isParentAllowed and return normalizedRequested only in the allowed
non-existence case.

---

Nitpick comments:
In `@test/main/presenter/llmProviderPresenter/acpContentMapper.test.ts`:
- Line 3: The test file is located in the old flat test tree and should mirror
the source layout; move the spec for AcpContentMapper to the matching test
directory under test/main/presenter/llmProviderPresenter/acp/ (so the test path
mirrors src/main/presenter/llmProviderPresenter/acp/), update any import paths
in the test that reference AcpContentMapper to the relative path from the new
location, and run the test suite to confirm imports resolve and the spec
executes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 634dcd31-29d3-450a-813e-0d23558dd77e

📥 Commits

Reviewing files that changed from the base of the PR and between 18c8909 and f6a1872.

📒 Files selected for processing (117)
  • archives/code/legacy-agentpresenter-retirement/README.md
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/acp/backgroundExecSessionManager.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/acp/index.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/acp/shellEnvHelper.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/agent/index.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/index.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/loop/agentLoopHandler.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/loop/errorClassification.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/loop/index.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/loop/loopOrchestrator.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/loop/loopState.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/loop/toolCallHandler.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/loop/toolCallProcessor.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/message/messageBuilder.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/message/messageCompressor.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/message/messageTruncator.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/message/skillsPromptBuilder.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/permission/permissionHandler.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/persistence/index.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/runtimePorts.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/session/sessionContext.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/session/sessionManager.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/session/sessionResolver.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/session/sessionRuntimePort.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/streaming/contentBufferHandler.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/streaming/llmEventHandler.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/streaming/streamGenerationHandler.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/streaming/streamUpdateScheduler.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/streaming/types.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/tool/toolCallCenter.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/tool/toolRegistry.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/tool/toolRouter.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/types.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/types/handlerContext.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/utility/promptEnhancer.ts
  • archives/code/legacy-agentpresenter-retirement/src/main/presenter/agentPresenter/utility/utilityHandler.ts
  • archives/code/legacy-agentpresenter-retirement/src/shared/types/presenters/agent.presenter.d.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/agentPresenter.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/loop/agentLoopHandler.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/loop/toolCallProcessor.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/message/systemEnvPromptBuilder.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/messageBuilder.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/messageCompressor.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/permission/permissionHandler.resume.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/promptBuilder.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/sessionManager.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/skillsPromptBuilder.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/streaming/llmEventHandler.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/agentPresenter/streaming/streamGenerationHandler.test.ts
  • archives/code/legacy-agentpresenter-retirement/test/main/presenter/sessionPresenter/permissionHandler.test.ts
  • docs/ARCHITECTURE.md
  • docs/FLOWS.md
  • docs/README.md
  • docs/architecture/agent-system.md
  • docs/architecture/session-management.md
  • docs/architecture/tool-system.md
  • docs/archives/legacy-agentpresenter-architecture.md
  • docs/archives/legacy-agentpresenter-flows.md
  • docs/guides/code-navigation.md
  • docs/guides/getting-started.md
  • docs/specs/agent-cleanup/plan.md
  • docs/specs/agent-cleanup/spec.md
  • docs/specs/agent-cleanup/tasks.md
  • docs/specs/legacy-agentpresenter-retirement/plan.md
  • docs/specs/legacy-agentpresenter-retirement/spec.md
  • docs/specs/legacy-agentpresenter-retirement/tasks.md
  • scripts/agent-cleanup-guard.mjs
  • src/main/presenter/configPresenter/acpInitHelper.ts
  • src/main/presenter/exporter/formats/conversationExporter.ts
  • src/main/presenter/exporter/formats/nowledgeMemExporter.ts
  • src/main/presenter/index.ts
  • src/main/presenter/llmProviderPresenter/acp/acpCapabilities.ts
  • src/main/presenter/llmProviderPresenter/acp/acpConfigState.ts
  • src/main/presenter/llmProviderPresenter/acp/acpContentMapper.ts
  • src/main/presenter/llmProviderPresenter/acp/acpFsHandler.ts
  • src/main/presenter/llmProviderPresenter/acp/acpMessageFormatter.ts
  • src/main/presenter/llmProviderPresenter/acp/acpProcessManager.ts
  • src/main/presenter/llmProviderPresenter/acp/acpSessionManager.ts
  • src/main/presenter/llmProviderPresenter/acp/acpSessionPersistence.ts
  • src/main/presenter/llmProviderPresenter/acp/acpTerminalManager.ts
  • src/main/presenter/llmProviderPresenter/acp/index.ts
  • src/main/presenter/llmProviderPresenter/acp/mcpConfigConverter.ts
  • src/main/presenter/llmProviderPresenter/acp/mcpTransportFilter.ts
  • src/main/presenter/llmProviderPresenter/acp/types.ts
  • src/main/presenter/llmProviderPresenter/index.ts
  • src/main/presenter/llmProviderPresenter/managers/providerInstanceManager.ts
  • src/main/presenter/llmProviderPresenter/providers/acpProvider.ts
  • src/main/presenter/sessionPresenter/index.ts
  • src/main/presenter/sessionPresenter/messageFormatter.ts
  • src/main/presenter/syncPresenter/index.ts
  • src/main/presenter/tabPresenter.ts
  • src/main/presenter/toolPresenter/agentTools/agentBashHandler.ts
  • src/main/presenter/toolPresenter/agentTools/agentFileSystemHandler.ts
  • src/main/presenter/toolPresenter/agentTools/agentToolManager.ts
  • src/main/presenter/toolPresenter/agentTools/chatSettingsTools.ts
  • src/main/presenter/toolPresenter/agentTools/index.ts
  • src/main/presenter/toolPresenter/index.ts
  • src/main/presenter/toolPresenter/runtimePorts.ts
  • src/shared/types/presenters/index.d.ts
  • src/shared/types/presenters/legacy.presenters.d.ts
  • src/shared/types/presenters/llmprovider.presenter.d.ts
  • test/main/lib/agentRuntime/backgroundExecSessionManager.test.ts
  • test/main/presenter/acpMcpPassthrough.test.ts
  • test/main/presenter/acpProvider.test.ts
  • test/main/presenter/acpSessionManager.test.ts
  • test/main/presenter/llmProviderPresenter.test.ts
  • test/main/presenter/llmProviderPresenter/acp/acpProcessManager.test.ts
  • test/main/presenter/llmProviderPresenter/acpCapabilities.test.ts
  • test/main/presenter/llmProviderPresenter/acpContentMapper.test.ts
  • test/main/presenter/llmProviderPresenter/acpFsHandler.test.ts
  • test/main/presenter/llmProviderPresenter/openAICompatibleProvider.test.ts
  • test/main/presenter/sessionPresenter/messageFormatter.test.ts
  • test/main/presenter/toolPresenter/agentTools/agentBashHandler.test.ts
  • test/main/presenter/toolPresenter/agentTools/agentFileSystemHandler.test.ts
  • test/main/presenter/toolPresenter/agentTools/agentToolManagerRead.test.ts
  • test/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.ts
  • test/main/presenter/toolPresenter/agentTools/chatSettingsTools.test.ts
💤 Files with no reviewable changes (4)
  • test/main/presenter/llmProviderPresenter/openAICompatibleProvider.test.ts
  • src/shared/types/presenters/index.d.ts
  • src/shared/types/presenters/llmprovider.presenter.d.ts
  • src/shared/types/presenters/legacy.presenters.d.ts

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
scripts/agent-cleanup-guard.mjs (1)

223-231: Consider removing dead code now that agentPresenter is excluded from scan roots.

Since LEGACY_AGENT_RUNTIME_DIR (src/main/presenter/agentPresenter) is no longer in scanRoots, no files from that directory will be collected into fileSet. This means the check at lines 223-231 will never match any files and is effectively dead code.

If the legacy runtime is being fully archived, this block can be removed along with LEGACY_AGENT_RUNTIME_DIR (line 42) and LEGACY_AGENT_RUNTIME_GLOBALS (lines 47-61).

♻️ Proposed cleanup
-const LEGACY_AGENT_RUNTIME_DIR = path.join(ROOT, 'src/main/presenter/agentPresenter')
 const PROVIDER_LAYER_DIR = path.join(ROOT, 'src/main/presenter/llmProviderPresenter/providers')
 const SKILL_PRESENTER_DIR = path.join(ROOT, 'src/main/presenter/skillPresenter')
 const MCP_TOOL_MANAGER_FILE = path.join(ROOT, 'src/main/presenter/mcpPresenter/toolManager.ts')

-const LEGACY_AGENT_RUNTIME_GLOBALS = [
-  'sessionManager',
-  'toolPresenter',
-  'mcpPresenter',
-  'configPresenter',
-  'skillPresenter',
-  'filePermissionService',
-  'settingsPermissionService',
-  'newAgentPresenter',
-  'sessionPresenter',
-  'yoBrowserPresenter',
-  'filePresenter',
-  'llmproviderPresenter',
-  'windowPresenter'
-]

And remove lines 223-231:

-    if (isProtectedPath(filePath, [LEGACY_AGENT_RUNTIME_DIR])) {
-      for (const legacyGlobal of LEGACY_AGENT_RUNTIME_GLOBALS) {
-        if (source.includes(`presenter.${legacyGlobal}`)) {
-          violations.push(
-            buildViolation(`agent-global-${legacyGlobal}`, filePath, `presenter.${legacyGlobal}`)
-          )
-        }
-      }
-    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/agent-cleanup-guard.mjs` around lines 223 - 231, The if-block that
checks isProtectedPath(filePath, [LEGACY_AGENT_RUNTIME_DIR]) and iterates
LEGACY_AGENT_RUNTIME_GLOBALS (the lines calling buildViolation for
`presenter.${legacyGlobal}`) is dead now that agentPresenter
(LEGACY_AGENT_RUNTIME_DIR) is excluded from scanRoots; remove that entire
conditional block and, if the legacy runtime is fully archived, also remove the
unused constants LEGACY_AGENT_RUNTIME_DIR and LEGACY_AGENT_RUNTIME_GLOBALS to
avoid dead-code references and unused symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@scripts/agent-cleanup-guard.mjs`:
- Around line 223-231: The if-block that checks isProtectedPath(filePath,
[LEGACY_AGENT_RUNTIME_DIR]) and iterates LEGACY_AGENT_RUNTIME_GLOBALS (the lines
calling buildViolation for `presenter.${legacyGlobal}`) is dead now that
agentPresenter (LEGACY_AGENT_RUNTIME_DIR) is excluded from scanRoots; remove
that entire conditional block and, if the legacy runtime is fully archived, also
remove the unused constants LEGACY_AGENT_RUNTIME_DIR and
LEGACY_AGENT_RUNTIME_GLOBALS to avoid dead-code references and unused symbols.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 841ab371-50af-4d04-a14e-2a6da8c77d7b

📥 Commits

Reviewing files that changed from the base of the PR and between f6a1872 and d81cd83.

📒 Files selected for processing (6)
  • docs/archives/legacy-agentpresenter-architecture.md
  • docs/archives/legacy-agentpresenter-flows.md
  • docs/guides/getting-started.md
  • scripts/agent-cleanup-guard.mjs
  • src/main/presenter/toolPresenter/agentTools/agentToolManager.ts
  • src/main/presenter/toolPresenter/runtimePorts.ts
✅ Files skipped from review due to trivial changes (3)
  • docs/archives/legacy-agentpresenter-architecture.md
  • src/main/presenter/toolPresenter/runtimePorts.ts
  • docs/archives/legacy-agentpresenter-flows.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/presenter/toolPresenter/agentTools/agentToolManager.ts

@zerob13 zerob13 merged commit 98450f5 into dev Mar 23, 2026
3 checks passed
@zerob13 zerob13 deleted the codex/plan-and-archive-legacy-agentpresent branch March 29, 2026 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant