Skip to content

Cowork internal refactors — DRY and hot-path cleanup #922

@sergio-sisternes-epam

Description

@sergio-sisternes-epam

Follow-up to #913. Surfaced by the APM Expert Review Panel (Python Architect specialist). All MEDIUM-severity maintenance items, no user-facing impact. CEO arbitration: acceptable as follow-ups, no release gate.

1. SkillIntegrator inlines dynamic-root routing 3x

File: src/apm_cli/integration/skill_integrator.py:666-667, 778-779, 838-839

Three call sites open-code if target.resolved_deploy_root is not None: ... instead of calling the target.deploy_path() accessor the PR itself introduced. This re-duplicates the very abstraction the target profile is meant to provide.

Fix: route all three sites through target.deploy_path(...).

2. resolve_cowork_skills_dir() called twice per entry in sync_remove_files

File: src/apm_cli/integration/base_integrator.py around lines 410 and 420

Each call hits apm config read + OneDrive glob. In a sync loop this compounds.

Fix: resolve once at the top of the cowork branch in sync_remove_files, reuse the value.

3. unset_temp_dir() / unset_cowork_skills_dir() copy-paste bypassing update_config()

File: src/apm_cli/config.py:129-140, 178-189

Two near-identical unset helpers, both re-implementing the read-modify-write pattern instead of calling update_config(). Maintenance trap for any future config-file format change.

Fix: consolidate into _unset_key(key) that calls update_config(); keep the two public wrappers as one-liners.


Acceptance criteria

  • All three SkillIntegrator sites call target.deploy_path(...).
  • sync_remove_files resolves the cowork root once per sync invocation.
  • Both unset helpers delegate to a single implementation that uses update_config().
  • Existing tests remain green; add a regression test for the update_config code path in both unset helpers.

/cc panel review: #913

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/cliCLI command surface, flags, help text (cross-cutting).area/multi-targetMulti-target deploy spec, target directory creation, agent surface routing.enhancementDeprecated: use type/feature. Kept for issue history; will be removed in milestone 0.10.0.experimentalstatus/acceptedDirection approved, safe to start work.status/triagedInitial agentic triage complete; pending maintainer ratification (silence = approval).type/refactorInternal restructure, no behavior change.

    Type

    No type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions