Skip to content

fix: handle missing calendar report summaries - #150

Merged
hughgrigg merged 1 commit into
mainfrom
fix/calendar-report-missing-summary
Sep 2, 2026
Merged

fix: handle missing calendar report summaries#150
hughgrigg merged 1 commit into
mainfrom
fix/calendar-report-missing-summary

Conversation

@hughgrigg

@hughgrigg hughgrigg commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Calendar report Lambda receives s3:ListBucket on the summaries bucket. S3 can now return 404 for a missing source summary, and the report writes an incomplete-source section. Access failures remain fatal. The public summary-reader grant remains s3:GetObject only. The policy regression covers both grants, and the deployed calendar-report test covers the incomplete report.

Resolves #148

  • Conventional commit message, used as the title

  • Conventional branch name, like feat/concise-description

  • Full check with pnpm run check passed

  • Rebased off latest main

  • User-facing behaviour is documented in docs/

Summary by CodeRabbit

  • Bug Fixes
    • Improved report source access handling so missing source files are detected accurately.
    • Report generation now distinguishes unavailable source files from permission-related errors, providing more reliable results when source data is missing.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 1a8b2602-5c9b-4042-8cbf-88a58e86b1e7

📥 Commits

Reviewing files that changed from the base of the PR and between 018ed3e and 91a9a9e.

📒 Files selected for processing (3)
  • src/cdk/report-function.ts
  • src/cdk/summary-permissions.test.ts
  • src/cdk/summary-permissions.ts

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.


📝 Walkthrough

Walkthrough

The report Lambda now uses report-source permissions that allow s3:GetObject on summary objects and s3:ListBucket on the summaries bucket. Tests verify the separate object and bucket ARN resources.

Changes

Report source permission flow

Layer / File(s) Summary
Permission contract and Lambda wiring
src/cdk/summary-permissions.ts, src/cdk/summary-permissions.test.ts, src/cdk/report-function.ts
Added reportSourceReadStatements with object-read and bucket-list permissions. Added coverage for both resource scopes. Updated the report Lambda role to use the new helper.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 91a9a

The PR adds handling for missing calendar report summaries and updates the related permissions and tests; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: handling missing calendar report summaries.
Description check ✅ Passed The description includes the change summary, linked issue, and all required checklist items. It matches the repository template.
Linked Issues check ✅ Passed The changes satisfy issue #148 by granting the calendar-report Lambda s3:ListBucket, preserving the public GetObject-only grant, covering the 403/404 distinction, and supporting incomplete-source repo…
Out of Scope Changes check ✅ Passed All changes are directly related to missing calendar report summaries, S3 permissions, and regression coverage. No unrelated code changes are identified.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files.
Full details: Linked Issues check

Explanation

The changes satisfy issue #148 by granting the calendar-report Lambda s3:ListBucket, preserving the public GetObject-only grant, covering the 403/404 distinction, and supporting incomplete-source report behavior while retaining fatal access failures.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/calendar-report-missing-summary

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

@hughgrigg
hughgrigg merged commit 2bac581 into main Sep 2, 2026
8 checks passed
@hughgrigg
hughgrigg deleted the fix/calendar-report-missing-summary branch September 2, 2026 11:52
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.

Calendar reports fail when a source summary object is missing

1 participant