Skip to content

refactor(acp): split oversized service.rs into focused modules #65

Open

Description

@yan-ad

Problem

src/acp/service.rs has grown beyond 3,000 lines of production code. On current upstream main, the file is 4,986 lines, with the first test module beginning at line 3,365.

The file combines session lifecycle and persistence, prompt execution, cancellation, compaction, usage/cost accounting, permissions, tool notifications, attachments, model/config selection, slash commands, and extensive tests. This makes related behavior harder to find, review, and test independently, and increases conflicts between ACP changes.

The automatic-compaction work in #64 is a recent example: context accounting, prompt preparation, client notifications, and billing changes all touch this same file.

Proposed refactor

Keep service.rs as a thin ACP orchestration layer and incrementally extract cohesive responsibilities, for example:

  • Session lifecycle: attachment state, active-prompt ownership, cancellation, and persistence finalization.
  • Prompt/stream handling: prompt preparation, stream event processing, and turn completion.
  • Compaction: threshold decisions, summary generation, transcript replacement, and compaction notifications.
  • Usage accounting: provider context measurements, cumulative billing, and ACP usage updates.
  • Protocol helpers: permission/elicitation handling and tool/content notification conversion, split further where useful.
  • Input and configuration: attachment processing, model/config options, and slash-command handling.

Move tests alongside the extracted responsibilities while keeping end-to-end stdio tests in tests/acp_stdio.rs. These are suggested boundaries, not a requirement to create every listed module.

Acceptance criteria

  • service.rs primarily coordinates ACP operations rather than implementing every subsystem.
  • Extraction is incremental and behavior-preserving; avoid combining it with unrelated feature or protocol changes.
  • Preserve prompt serialization, cancellation/close/delete behavior, lock ownership, and persisted transcript/status finalization.
  • Preserve compaction thresholds, context-versus-billing semantics, and ACP client notifications, including the regressions covered by fix(acp): trigger automatic compaction from provider context usage #64.
  • Keep internal interfaces narrow; do not broaden visibility just to move code.
  • Existing ACP unit tests, compaction tests, and stdio integration tests remain green; formatting checks pass.

The goal is clearer ownership and safer changes, not merely meeting an arbitrary line-count target.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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