Skip to content

refactor: migrate all services to per-package structure under internal/service/#33

Merged
dannysteenman merged 18 commits intomainfrom
feat/cli
Feb 20, 2026
Merged

refactor: migrate all services to per-package structure under internal/service/#33
dannysteenman merged 18 commits intomainfrom
feat/cli

Conversation

@dannysteenman
Copy link
Copy Markdown
Member

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Docstrings or comments have been reviewed and added / updated if needed
  • The change has been tested and confirmed working (Tested on? OS, AWS CLI Version)

Pull request type

Please check the type of change your PR introduces:

  • Bugfix (if it's an open issue, please add the issue number):
  • Feature e.g. new script (please describe):
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Other information

dannysteenman and others added 13 commits February 19, 2026 17:01
Extract shared CLI infrastructure (runtime, destructive actions,
pointers, service group, tag parsing, test utilities) into a
dedicated package for use by per-service packages.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…l/service/

Move all 13 AWS service commands from the monolithic internal/cli/ package
into individual packages under internal/service/<name>/. Each service
package exports NewCommand() and contains its own tests and mocks.

Shared runtime infrastructure (GlobalOptions, CommandRuntime,
NewServiceRuntime, destructive action helpers, pointer helpers, tag
parsing) was previously extracted to internal/cliutil/ and is now the
sole source of these utilities.

The internal/cli/ package is now slim: root command wiring, help
defaults, completion, and version only.

Coverage gate updated from internal/cli-only to all internal/ packages
at >=80% (current: 88.1%).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0378cac0cf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/service/s3/commands.go Outdated
Comment thread internal/service/org/accounts.go Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa7ec9c95d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/service/s3/helpers.go
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa5356ebd8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +106 to +110
key := objectKey(object)
relativeKey := strings.TrimPrefix(key, prefix)
relativeKey = strings.TrimPrefix(relativeKey, "/")
if relativeKey == "" {
relativeKey = key
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Ignore folder marker objects when downloading S3 prefixes

S3 prefixes commonly include zero-byte "folder marker" keys that end with / (for example logs/). This loop downloads every listed object without filtering those markers, so it creates a file at logs and then later fails to download real children like logs/app.log because the parent path is now a file, causing partial/failed downloads for otherwise valid buckets. Skip marker keys (or treat them as directories) before resolving targetPath and calling downloadObject.

Useful? React with 👍 / 👎.

Comment on lines +67 to +69
normalizedKey := strings.ReplaceAll(relativeKey, "\\", "/")
normalizedKey = strings.TrimPrefix(normalizedKey, "/")
normalizedKey = path.Clean(normalizedKey)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject normalized-key collisions in download target paths

resolveDownloadTargetPath canonicalizes object keys with path.Clean, which collapses distinct S3 keys such as a//b.txt and a/b.txt onto the same local file path. During download-bucket, both rows can appear successful while the later object silently overwrites the earlier one, corrupting exports from buckets that contain intentionally distinct keys. Detect and fail on post-normalization path collisions instead of mapping multiple keys to one file.

Useful? React with 👍 / 👎.

@dannysteenman dannysteenman merged commit 1ab0f63 into main Feb 20, 2026
3 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

Version tag created successfully.

  • New version: v0.0.1
  • Previous version: v0.0.0
  • Bump type: patch

Release workflow should now run for v0.0.1.

@dannysteenman dannysteenman deleted the feat/cli branch March 19, 2026 13:57
dannysteenman added a commit that referenced this pull request Mar 19, 2026
* refactor(rules): adopt discovery dataset dependencies

* refactor(sdk): drive aws discovery from dataset registry

* refactor(rules): organize discovery dataset exports

* refactor(sdk): finalize discovery registry cleanup

* test(sdk): assert live resource bag contents

* fix(sdk): reject inherited discovery dataset keys
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant