fix: report per-resource listing failures

[YoungJinJung]

Summary

• preserve successfully described KMS keys and ACM certificates when a sibling detail lookup fails
• show denied KMS rotation status as unknown and render per-resource failures inline
• keep successful Inspector findings while reporting target-health lookup failures as warnings
• update README and project overviews for partial-permission behavior

Related Issues

Closes #314

Validation

• go test ./internal/services/aws ./internal/inspector ./internal/app
• make test
• make build
• git diff --check

Checklist

• Scope is focused
• Branch name follows docs/branch-naming-harness.md
• Documentation harness reviewed (docs/documentation-harness.md)
• README updated if user-facing behavior changed
• Relevant docs/ pages updated if architecture, auth, config, or workflow changed
• Tests/validation included
• Breaking changes documented (none)

[coderabbitai]

Warning

Review limit reached

Next included review available in 10 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c22ed50d-6d44-4672-b9c3-2f932c0d4266

📥 Commits

Reviewing files that changed from the base of the PR and between 706464f and 6d6ac3c.

📒 Files selected for processing (22)

• README.md
• docs/project-overview.en.md
• docs/project-overview.ko.md
• internal/app/messages.go
• internal/app/screen_acm.go
• internal/app/screen_acm_test.go
• internal/app/screen_inspector.go
• internal/app/screen_inspector_test.go
• internal/app/screen_kms.go
• internal/app/screen_kms_test.go
• internal/app/styles.go
• internal/inspector/inspector.go
• internal/inspector/inspector_rules_acm.go
• internal/inspector/inspector_rules_cost.go
• internal/inspector/inspector_rules_cost_test.go
• internal/inspector/inspector_rules_kms.go
• internal/inspector/inspector_rules_kms_test.go
• internal/inspector/inspector_test.go
• internal/services/aws/acm.go
• internal/services/aws/acm_test.go
• internal/services/aws/kms.go
• internal/services/aws/kms_test.go

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[amazon-q-developer]

The partial-failure handling implementation is well-designed and correctly addresses issue #314. The changes properly preserve successful results while reporting per-resource failures as warnings, improving the user experience when working with limited AWS permissions.

Key improvements:

• ACM and KMS service layers now return warnings alongside successful results
• Inspector scanners correctly propagate warnings without failing the entire scan
• UI displays per-resource errors inline, maintaining usability during partial access
• Added RotationKnown field to distinguish between denied permissions and actual rotation status
• All changes are backed by comprehensive tests

The implementation follows Go best practices for error handling and maintains backward compatibility with existing functionality.

---

You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

[YoungJinJung]

Reviewed head: 99722e0

Findings

1. P2 — Preserve context cancellation as a fatal target-health error (internal/inspector/inspector_rules_cost.go:249-254)

   The new partial-failure branch treats every DescribeTargetHealth error as a resource warning, including context.Canceled and context.DeadlineExceeded. After the user cancels a scan or leaves the screen, this loop therefore keeps visiting every remaining target group and the helper can return a nil fatal error with cancellation buried in warnings. Check ctx.Err() in this error path and return the context error; only ordinary per-resource API failures should be collected. A cancellation-focused test would lock in the distinction.

2. P2 — Bound per-resource warnings to the terminal viewport (internal/app/screen_kms.go:109-120, internal/app/screen_acm.go:128-139, internal/app/screen_inspector.go:840-844)

   Each denied key/certificate is rendered as another line above the list, while visibleLines bottoms out at five; the Inspector path similarly renders the full multiline errors.Join value. In an account where a policy denies dozens of detail lookups, the warnings alone exceed the terminal height and push the successful resources and help bar off-screen, defeating the partial-results behavior this PR adds. Render a bounded warning summary/count (or window the warning lines inside the panel) and cover a small-height model with multiple warnings.

[YoungJinJung]

Applied the review feedback in 6d6ac3c.

• preserve context cancellation and deadline failures as fatal target-health scan errors
• replace unbounded KMS, ACM, and Inspector warning dumps with a fixed count-plus-first-failure summary
• add small-height regressions that keep successful resources and help visible
• add cancellation coverage that verifies remaining target groups are not visited
• make test: passed
• make build: passed
• git diff --check: passed

Docs remain unchanged because this fixes the PR’s existing partial-results behavior.

[YoungJinJung]

/q review

[amazon-q-developer]

Summary

This PR successfully implements partial-failure handling for AWS resource listing operations, preserving successfully retrieved resources when individual detail lookups fail due to permission issues. The implementation is well-designed and correctly handles the behavior described in issue #314.

Key Changes Reviewed

Core Implementation:

• Modified return signatures across KMS and ACM services to return ([]Resource, []error, error) for warnings and fatal errors
• Added RotationKnown field to track when KMS rotation status checks fail due to permissions
• Changed inspector behavior to collect findings before checking for errors, ensuring partial results are preserved
• Updated UI components to display warning summaries for resource lookup failures

Error Handling Pattern:
The PR implements a robust three-tier error handling pattern:

1. Fatal errors - returned as the third return value (connection failures, list operation failures)
2. Per-resource warnings - accumulated and returned as second return value (permission denied on specific resources)
3. Successful results - preserved and returned even when warnings exist

Test Coverage:
Comprehensive test coverage validates:

• Per-resource failure scenarios preserve successful results
• Concurrency limits remain enforced with partial failures
• Warning messages correctly identify failed resources
• The RotationKnown flag properly tracks when rotation status cannot be determined

Verdict

The implementation is production-ready with no blocking defects. The code correctly handles partial permissions, maintains existing functionality, and includes thorough test coverage. All changes follow Go best practices and maintain backward compatibility with existing error handling patterns.

---

You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

[YoungJinJung]

Reviewed head: 6d6ac3c — No actionable findings. The new commit preserves context cancellation as fatal, bounds warning rendering while keeping successful resources visible, and adds focused regressions. Validation passed: make test, make build, and git diff --check; CI is green.