PR 1: DI refactor (ssmAPI interface + cmd App holder)#30
Merged
Conversation
…or pattern, update tests
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR 1 of 2 implementing the DI and tests spec at
docs/superpowers/specs/2026-05-08-di-and-tests-design.md.This PR is purely structural — no behavior changes on the happy path. It introduces the dependency-injection seams that PR 2 will exploit for unit tests.
chore: fix stale pinpt/esp imports in tests) — restore green test suite by fixing stalepinpt/espimports left over from the rebrand (PR Rebrand: pinpt/esp → AbsolutOD/esp + LICENSE/README refresh #29).refactor(ssm,client): introduce ssmAPI interface and collapse Service constructor) — extract unexportedssmAPIinterface insideinternal/ssm;Serviceholdsapi ssmAPI; collapseService.New+Initinto one constructor;client.Newadapts to the new return signature. The spec listed these as two commits but the plan combined them — splitting would break the build between commits.refactor(cmd): introduce App holder, convert subcommands to constructor pattern, update tests) — introducecmd.Appholder; convert all 8 subcommands tonewXCmd(*App)constructor pattern with extractedrunXfunctions; remove the package-levelesp,c, andverboseglobals; update existing tests for new pure-helper signatures.docs: update CLAUDE.md for App holder pattern and DI seams) — refresh architecture, "Adding a subcommand", and testing notes sections to reflect the App-holder pattern (caught during code review — was misdirecting against the new structure).PR 2 will add unit tests against the new seams.
Test plan
go build ./...cleango vet ./...cleango test ./...green (existing tests preserved through signature updates)esp --helpworks without AWS env vars setesp versionwith missingAWS_DEFAULT_REGIONreturns exit 1 with the env-var errorespinvocations behave identically to pre-refactor on the happy path against real AWS (manual smoke test)Notes for review
ssmAPIinterface is structurally a superset of the SDK'sawsssm.GetParametersByPathAPIClient, so the sames.apivalue is passed toawsssm.NewGetParametersByPathPaginator.runXextraction is uniform across all subcommands for a uniform test contract, even where the body is 1-3 lines (copy,move,delete,version).Service.Regionfield is now write-only — kept exported for the moment to avoid expanding scope. Worth unexporting in a future cleanup.