Skip to content

refactor(core): remove APIs with no production callers #22

Closed

Description

@showxu

Summary

Core contains alternate helper APIs that no production path uses. Some are exercised only by tests; others have no callers, and SourceResolver.resolve(refresh:) exposes a flag that is never read. Consolidating the internal API surface would make the implementation and tests reflect the paths used by the CLI.

Code evidence at 5d4987b:

  • Sources/Core/UpdateSource.swift:3 defines formatting logic whose only external callers are in Tests/CoreTests/UpdateSourceBehaviorTests.swift:7; the current update path starts at Sources/Core/RuntimeService.swift:615.
  • Sources/Core/WatchLedger.swift:278 wraps upsert and has test-only callers; production add calls upsert at Sources/Core/RuntimeService.swift:289.
  • Sources/Core/WatchLedger.swift:241 and Sources/Core/InstallLock.swift:118 / :122 provide test-used path aliases for current URL helpers.
  • Sources/Core/AgentRegistry.swift:147 defines detectedInstalledAgents without callers in Sources or Tests.
  • Sources/Core/ProcessRunner.swift:288 declares refresh, but the resolver never reads it.
  • Package.swift:12 exposes an executable product, not a Core library product.

Use Case

Maintainers should be able to follow the actual source-update and watch flows without a parallel set of test-only production helpers suggesting additional behavior. Tests should exercise the current owner APIs.

Reproduction / current-state verification

Executed at 5d4987b. This was a source/reference inventory, not a runtime failure reproduction.

Run in Bash after cd "<checkout>" && swift build, replacing <checkout> with the absolute path of the checkout at this revision. Use a fresh reproduction directory. The commands isolate HOME, agent configuration, and XDG_*, and remove their fixture root on exit.

set -eu
checkout="<checkout>"
skill_bin="$checkout/.build/debug/skill"
repro_root="$checkout/.build/issue-repro-core-api"
mkdir -p "$repro_root/home" "$repro_root/tmp" "$repro_root/project"
trap 'rm -rf "$repro_root"' EXIT
isolated() {
  env -i PATH="$PATH" HOME="$repro_root/home" \
    CODEX_HOME="$repro_root/home/.codex" \
    CLAUDE_CONFIG_DIR="$repro_root/home/.claude" \
    XDG_CONFIG_HOME="$repro_root/home/.config" \
    XDG_STATE_HOME="$repro_root/home/.local/state" \
    XDG_CACHE_HOME="$repro_root/home/.cache" TMPDIR="$repro_root/tmp" \
    GIT_CONFIG_NOSYSTEM=1 GIT_CONFIG_GLOBAL=/dev/null \
    GIT_AUTHOR_NAME=Fixture [email protected] \
    GIT_COMMITTER_NAME=Fixture [email protected] \
    "$@"
}
skill() { isolated "$skill_bin" "$@"; }
cd "$repro_root/project"


cd "$checkout"
isolated rg -n 'UpdateSource|WatchService\.add\(|detectedInstalledAgents|ledgerURL|projectLockURL|globalLockURL|refresh:' Sources Tests
isolated sed -n '1,44p' Package.swift

Observed findings (summarized from the command output):

Source/reference search found:
- UpdateSource: declaration plus UpdateSourceBehaviorTests only.
- WatchService.add: test callers only; production uses upsert.
- ledgerURL, projectLockURL, globalLockURL: declarations plus test callers only.
- detectedInstalledAgents: declaration only.
- refresh: declaration only; no body read.
Package.swift declares the skill executable product.

Environment: Apple Swift 6.4, macOS 27.0.1 (26A434), arm64; CLI 0.2.1; validation date 2026-10-17.

Proposed Behavior

Maintainer decision: confirm whether any of these public declarations have an intended supported consumer outside the declared package products. Remove unneeded declarations, route meaningful tests through the owning production APIs, and remove tests that only preserve otherwise unused formatting behavior.

Completion checks:

  • Each retained entry point has an identified current caller or explicit supported contract.
  • Remove the unused refresh parameter or define it under the separately accepted refresh contract.
  • Preserve coverage of current watch/path behavior while removing redundant aliases.
  • swift build and swift test pass.

Tradeoffs

This is a maintenance proposal, not proof that every test-only wrapper is harmful. In particular, wrappers with test callers are not literally unused. Keep a declaration if the maintainer identifies a current consumer; lack of in-repository production calls alone does not establish external compatibility requirements.

Compatibility Notes

Do not change persisted schemas, migration behavior, accepted decision records, or functional source aliases as part of this cleanup.

Related: #9 owns the behavioral policy for refresh side effects; removing an unread parameter does not resolve that policy. #5 / PR #12 own resolved-file safety, which must remain intact. No duplicate found.

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

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions