Skip to content

ACP: Cleanup dead code, dropped protocol messages, and stale comments #363

Description

@julianromli

Problem or Opportunity

A maturity audit of the ACP subsystem surfaced several pieces of dead code, dropped protocol messages, and stale documentation that should be cleaned up before the feature ships. None are individually critical, but together they add confusion and mask real behavior.

1. Dead code: acpListSessions / ListSessionsResponse never called

src/renderer/lib/acp-api.ts:215-217:

export interface ListSessionsResponse {
  [k: string]: unknown
}

acpListSessions is defined (line ~383) and wired into the acpApi facade, but it is never called from any component or store action. The opaque index signature also signals the result is never decoded. Either wire it into session discovery (e.g. the history tab) or remove it.

2. Dropped protocol message: SessionInfoUpdate

src-tauri/src/acp/client.rs:296-303:

// SessionInfoUpdate and any future (non_exhaustive) variants have no
// dedicated P0 event; ignore them — but log so a silently-dropped
// update can be diagnosed instead of vanishing.
ref other => {
    log::debug!("[acp] agent {agent_id} sent an unhandled session/update variant: {other:?}");
}

SessionInfoUpdate (session metadata from the agent) is logged at debug — off by default — and dropped. The renderer never sees session metadata changes. Either surface it as an acp:* event or document explicitly why it's intentionally ignored.

3. Error-swallowing catch {} in closeSession

src/renderer/stores/acp-store.ts:605-629:

closeSession: async (sessionId) => {
  const session = get().sessions[sessionId]
  if (session && session.status !== 'closed') {
    try {
      await acpApi.closeSession(session.agentId, sessionId)
    } catch {
      // close may fail if the agent lacks the capability; mark closed locally regardless
    }
  }
  ...

A bare catch {} swallows all errors (network/transport included), not just the capability-mismatch the comment justifies. At minimum, log the error so transport failures are diagnosable; ideally distinguish capability errors from transport errors.

4. Stale doc: "terminal stubs"

src-tauri/src/acp/client.rs:3:

//! handling (permission, filesystem), session-update fan-out, and terminal
//! stubs.

The terminal handlers are fully implemented (src-tauri/src/acp/manager.rs:898-1022, src-tauri/src/acp/terminal.rs), not stubs. The module doc comment is stale.

5. Stale phase comments

Multiple comments label code by delivery phase ("P0", "P1", "deferred to P1+") even though the UI for P1–P6 now exists:

These add noise without current meaning.

Proposed Solution

  1. acpListSessions: wire into session discovery or remove the command + type.
  2. SessionInfoUpdate: surface as an acp:* event (e.g. acp:session_info) or add an explicit // intentionally ignored: … comment explaining why.
  3. closeSession catch: log the swallowed error (console.warn or the log adapter) so transport failures aren't invisible; keep the local-close behavior.
  4. client.rs:3: update the doc comment to reflect that terminal handlers are implemented (remove "stubs").
  5. Phase comments: strip or rewrite the stale phase labels now that the phases have shipped.

Success Criteria

  • acpListSessions is either used or removed (no dead facade entry).
  • SessionInfoUpdate is either surfaced or explicitly justified-as-ignored.
  • closeSession logs swallowed errors instead of silently dropping them.
  • client.rs module doc no longer claims terminal handlers are stubs.
  • Stale "P0/P1/deferred" phase labels are removed or rewritten.

Additional Context

  • These are small, independent edits — suitable as a single cleanup PR or folded into the larger ACP hardening effort.
  • bun run ci + cargo clippy -- -D warnings + cargo test must stay green.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions