Skip to content

plan(deploy): full-saga demo deployment plan + STATUS pointer#42

Merged
emeraldleaf merged 5 commits into
mainfrom
plan/full-saga-deployment
May 28, 2026
Merged

plan(deploy): full-saga demo deployment plan + STATUS pointer#42
emeraldleaf merged 5 commits into
mainfrom
plan/full-saga-deployment

Conversation

@emeraldleaf

@emeraldleaf emeraldleaf commented May 28, 2026

Copy link
Copy Markdown
Owner

Summary

Cross-session tracking artifact for the multi-PR effort to stand up NextAurora as a portfolio-grade demo deployment running the full Order → Payment → Shipping → Notification saga over real cloud infrastructure with the Stripe gateway stubbed.

What's in this PR

  • docs/full-saga-deployment-plan.md — the planning doc itself. Three phases (Order saga visible → full saga → polish), four open decisions (D1–D4), cost ledger structure, scope in/out, prerequisites checklist.
  • docs/STATUS.md — "Next" section now points at the plan so future sessions pick up coherently.

Why this is its own PR

This is the IDSD-style intent + expectations + context for a multi-PR effort. Per the principle encoded yesterday: write the plan as a tracked artifact BEFORE implementation, so the next session opens with the plan, not "what was I doing?"

The plan has zero code changes — just docs. Merging quickly is fine; the actual deployment work happens in subsequent PRs that reference this doc.

Open decisions that block Phase 1

Need to resolve before any infrastructure is provisioned:

  • D1. SQL Server hosting — Azure SQL serverless, Postgres-only-for-demo, or defer
  • D2. Identity provider — Keycloak self-hosted, Auth0, Azure AD B2C, or static JWKS
  • D3. Messaging transport — Azure Service Bus Basic, NATS/RabbitMQ on Fly, or AWS SQS+SNS
  • D4. Cost ceiling — suggested $15/mo Phase 1, $30/mo Phase 2

The plan doc captures the trade-offs for each decision. Will resolve in conversation + commit follow-ups to the doc before Phase 1's first infrastructure PR opens.

Test plan

  • Doc renders cleanly on GitHub (markdown, no broken cross-references)
  • STATUS.md "Next" section still reads cleanly with the new pointer
  • First session resuming after this lands can navigate to the plan and pick up phase work — verify in next session

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added a project status entry tracking the "Full-saga demo deployment" multi-PR initiative.
    • Added a comprehensive deployment plan describing scope, exclusions, three-phase rollout (visibility, end-to-end, ops polish), implementation order, resolved hosting/identity/messaging/cost decisions, prerequisites, cost guidance, and planned telemetry/“try the live demo” and ops items.

Review Change Stack

Cross-session tracking artifact for the multi-PR, multi-week effort to
stand up NextAurora as a portfolio-grade demo deployment running the
full Order → Payment → Shipping → Notification saga over real cloud
infrastructure with the Stripe gateway stubbed.

Structure:
- Three phases, each independently shippable:
  1. Order saga visible (Catalog + Order + minimal Storefront). Order
     persists, OrderPlacedEvent stages, saga stalls because PaymentService
     isn't deployed yet — itself a teaching demo.
  2. Full saga (Payment + Shipping + Notification). End-to-end flow over
     real infrastructure with stubbed Stripe.
  3. Polish (observability + ops + minimal UX) — demoable to humans.

- Four open decisions (D1-D4) called out explicitly: SQL Server hosting,
  identity provider, messaging transport, cost ceiling. All block Phase 1.

- Out-of-scope explicitly listed: real Stripe SDK (stub retirement),
  PaymentRecoveryJob retry-with-key (gated on stub retirement),
  production-grade DR/SLAs. Honest framing of what this is and isn't.

- Cost ledger structure ready for first entries once provisioning begins.

STATUS.md "Next" section now points at the plan so future sessions
can pick up coherently.

Following the IDSD/intent-driven discipline encoded in the recent
articles: explicit intent + expectations + context as a tracked
artifact BEFORE implementation, not after. The plan is the "ICE"
artifact for this multi-PR effort.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a new end-to-end, multi-phase deployment plan document for the NextAurora Order→Payment→Shipping→Notification saga (Stripe stubbed) and updates STATUS.md to list the effort as an active multi-PR initiative with four decision points blocking Phase 1.

Changes

Full-Saga Demo Deployment Planning

Layer / File(s) Summary
Plan introduction, rationale, and scope definition
docs/full-saga-deployment-plan.md
Document header, purpose, motivation for cross-session tracking, and in/out-of-scope boundaries for the saga demo.
Three-phase execution roadmap
docs/full-saga-deployment-plan.md
Phase 1 (Order visible, payment intentionally stalled), Phase 2 (complete saga with Stripe stub), Phase 3 (demo polish and external readiness); each phase lists deliverables, risks, and done criteria; includes Phase 1 detail.
Decision points and Phase 1 implementation ordering
docs/full-saga-deployment-plan.md
Records resolved decisions D1–D4 (DB provider, identity, messaging, cost ceiling) and details Phase 1A/1B/1C implementation order (Postgres provider swap, Keycloak deploy, OrderService + Storefront deploy).
Cost ledger, prerequisites, related docs, and STATUS integration
docs/full-saga-deployment-plan.md, docs/STATUS.md
Adds cost ledger stub, prerequisites checklist, related-doc links, and updates docs/STATUS.md with an “Active multi-PR effort” bullet referencing the full-saga plan and its blocking decisions.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: adding a full-saga deployment plan document and updating the STATUS pointer.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch plan/full-saga-deployment

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

@codecov

codecov Bot commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/STATUS.md (1)

5-5: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Update Last updated to today’s date.

docs/STATUS.md changed in this PR (Line 103-Line 104), but Last updated is still 2026-05-26. It should be updated to 2026-05-28 (or current session timestamp format used in this file).

As per coding guidelines, "docs/STATUS.md: STATUS.md is the cross-session entry point. When this changes, the 'Last updated:' date should be today. Flag if the date is stale relative to the diff."

Also applies to: 103-104

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/STATUS.md` at line 5, The STATUS.md header's "Last updated:" value is
stale; update the "Last updated:" date string in docs/STATUS.md from 2026-05-26
to 2026-05-28 (or the repository's current date format) so it matches the PR
changes (the file was modified around the block at lines referenced in the
review), ensuring the exact same formatting and punctuation around "Last
updated:" is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/full-saga-deployment-plan.md`:
- Around line 238-239: The STATUS.md pointer in
docs/full-saga-deployment-plan.md is wrong: it says the one-line pointer is
under "Open issues" but the actual entry in docs/STATUS.md is under the "Next"
section; update the sentence at the referenced diff to say "Next" instead of
"Open issues" (or reword to point to the "Next" section in docs/STATUS.md) so
the cross-reference matches the actual section header in docs/STATUS.md.

---

Outside diff comments:
In `@docs/STATUS.md`:
- Line 5: The STATUS.md header's "Last updated:" value is stale; update the
"Last updated:" date string in docs/STATUS.md from 2026-05-26 to 2026-05-28 (or
the repository's current date format) so it matches the PR changes (the file was
modified around the block at lines referenced in the review), ensuring the exact
same formatting and punctuation around "Last updated:" is preserved.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 18aa7a79-f4ef-433c-a905-7fde9344bf51

📥 Commits

Reviewing files that changed from the base of the PR and between 7f8da83 and 3dc68ab.

📒 Files selected for processing (2)
  • docs/STATUS.md
  • docs/full-saga-deployment-plan.md

Comment thread docs/full-saga-deployment-plan.md Outdated
User-resolved decisions captured in the plan doc:

- D1 (SQL Server hosting): Postgres-only-for-demo with provider swap.
  Dev environment keeps two-engine split (the architectural story is
  still genuine for dev/learning). Deployed environment uses Postgres
  for all four services with state. README footnote will explain the
  demo-deployment exception.

- D2 (Identity): Auth0 free tier. Hosted, simplest setup. Keycloak
  stays for local dev. ServiceDefaults JWT config is already
  config-driven — points at Auth0 issuer in deployed environment.

- D3 (Messaging): AWS SQS+SNS free tier. Wolverine reconfig from
  UseAzureServiceBus to UseAmazonSqs. 1M req/mo free covers demo
  volume comfortably.

- D4 (Cost ceiling): $30/mo target, $50/mo hard ceiling. Phase 1 ≤$15,
  Phase 2 ≤$30. Fly.io spend cap to be set at $50 before any
  provisioning.

Phase 1 now splits naturally into two sub-PRs given D1:

- Phase 1A: Postgres provider swap (code only). Add `DatabaseProvider`
  config setting, branch Order + Payment Program.cs on it, regenerate
  Postgres migrations, branch concurrency-token config (RowVersion vs
  xmin). Integration tests cover both paths. README footnote.

- Phase 1B: Deploy Order + minimal Storefront. Original Phase 1 goal,
  now sitting on top of 1A's plumbing.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/full-saga-deployment-plan.md (1)

268-268: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Prerequisite contradicts "Resolved decisions" heading.

Line 268 lists "All four open decisions (D1–D4) resolved" as a prerequisite, but line 155 has a heading "Resolved decisions (2026-05-27)" indicating they're already done. This creates confusion about whether the decisions are blocking work or already complete.

Either remove this prerequisite (if truly resolved) or change the heading to indicate they remain open.

📝 Suggested fix if decisions are truly resolved
 ## Prerequisites before any phase starts
 
-- [ ] All four open decisions (D1–D4) resolved and captured in this doc
 - [ ] Fly.io account billing set up with a hard spend cap matching D4
 - [ ] Identity provider account created per D2
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/full-saga-deployment-plan.md` at line 268, The prerequisite "All four
open decisions (D1–D4) resolved and captured in this doc" conflicts with the
heading "Resolved decisions (2026-05-27)"; update the document to remove the
contradiction by either deleting the prerequisite bullet about D1–D4 if those
decisions are indeed resolved, or else change the heading "Resolved decisions
(2026-05-27)" to an explicit "Open decisions" (or similar) and update the
date/notes to reflect they remain unresolved; ensure you edit the exact strings
"All four open decisions (D1–D4) resolved and captured in this doc" and
"Resolved decisions (2026-05-27)" so readers are no longer confused.
♻️ Duplicate comments (1)
docs/full-saga-deployment-plan.md (1)

284-284: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix incorrect STATUS section reference.

This says the pointer is under "Open issues", but the actual entry in docs/STATUS.md is under "Next" (as shown in the context snippet at lines 101-104). Please update this line to reference the correct section.

📝 Proposed fix
-- [docs/STATUS.md](STATUS.md) — Cross-session entry point. Has a one-line
-  pointer to this doc under "Open issues."
+- [docs/STATUS.md](STATUS.md) — Cross-session entry point. Has a one-line
+  pointer to this doc under "Next."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/full-saga-deployment-plan.md` at line 284, Update the pointer text in
docs/full-saga-deployment-plan.md that currently says the STATUS entry is under
"Open issues" so it instead references the correct section "Next" in
docs/STATUS.md; locate the sentence mentioning "Open issues" and replace that
section name with "Next" (ensure the link or reference text matches the "Next"
heading in docs/STATUS.md).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/full-saga-deployment-plan.md`:
- Around line 155-219: STATUS.md still claims “four open decisions (D1–D4) block
Phase 1” while full-saga-deployment-plan.md shows those decisions under the
header “Resolved decisions (2026-05-27)”; open STATUS.md, locate the sentence
containing “four open decisions (D1–D4) block Phase 1” and update it to reflect
that D1–D4 are resolved (or explicitly mark them as tentative) and add a short
pointer to the resolved section by quoting the header “Resolved decisions
(2026-05-27)”; ensure the wording is consistent with the deployment plan and
bump the STATUS.md update date or status tag accordingly so both documents are
aligned.

---

Outside diff comments:
In `@docs/full-saga-deployment-plan.md`:
- Line 268: The prerequisite "All four open decisions (D1–D4) resolved and
captured in this doc" conflicts with the heading "Resolved decisions
(2026-05-27)"; update the document to remove the contradiction by either
deleting the prerequisite bullet about D1–D4 if those decisions are indeed
resolved, or else change the heading "Resolved decisions (2026-05-27)" to an
explicit "Open decisions" (or similar) and update the date/notes to reflect they
remain unresolved; ensure you edit the exact strings "All four open decisions
(D1–D4) resolved and captured in this doc" and "Resolved decisions (2026-05-27)"
so readers are no longer confused.

---

Duplicate comments:
In `@docs/full-saga-deployment-plan.md`:
- Line 284: Update the pointer text in docs/full-saga-deployment-plan.md that
currently says the STATUS entry is under "Open issues" so it instead references
the correct section "Next" in docs/STATUS.md; locate the sentence mentioning
"Open issues" and replace that section name with "Next" (ensure the link or
reference text matches the "Next" heading in docs/STATUS.md).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9513e7cd-5431-4764-8727-514ac82beb62

📥 Commits

Reviewing files that changed from the base of the PR and between 3dc68ab and fa4e1db.

📒 Files selected for processing (1)
  • docs/full-saga-deployment-plan.md

Comment thread docs/full-saga-deployment-plan.md
… + Phase 3 rate-limit audit

User reconsidered D2 after seeing an article on Keycloak's value as an
identity provider. Switched from Auth0 to Keycloak self-hosted on Fly.

Rationale:
- Same IdP in dev and prod. NextAurora already runs Keycloak in local
  Aspire dev with the realm at realms/nextaurora-realm.json. Deployed
  Keycloak imports the same realm export on boot — one IdP, one realm,
  two environments. Removes "works in dev, breaks in prod" risk around
  realm shape, claim names, test users.
- Stronger portfolio story than vendor-managed Auth0.
- Cost is roughly comparable (~$5-10/mo Fly Machine vs Auth0 free
  vendor-locked).

Trade-off: real ops surface (Postgres DB for Keycloak state, persistent
volume, realm-import-on-boot, two-stage readiness probe, ~30-60s cold
start for Java + realm import).

Phase 1 now splits into three sub-PRs:
- 1A: Postgres provider swap (code only) — unchanged
- 1B: Deploy Keycloak self-hosted (NEW infrastructure-only sub-phase)
- 1C: Deploy Order + minimal Storefront against deployed Keycloak

Phase 3 deliverables grown by one item: rate-limit audit + Redis-backed
swap if any service runs 2+ Fly Machines. The in-memory ASP.NET Core
limiter on Catalog search + Payment process is correct for single-
instance Phase 2 but silently weakens under scale-out. The rule
encoding lands in a separate PR (CLAUDE.md / .coderabbit.yaml /
architecture-reviewer.md).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/full-saga-deployment-plan.md (1)

232-233: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix D4 cost text to match the resolved D2 provider choice.

Line 232 still says “Auth0 free tier,” but D2 is resolved to Keycloak self-hosted on Fly (Line 185). This creates planning/cost ambiguity in the same document.

Suggested minimal doc fix
-- Phase 1 target: ≤ $15/mo (Postgres free with Fly + Auth0 free tier + SQS
+- Phase 1 target: ≤ $15/mo (Postgres free with Fly + Keycloak self-hosted on Fly + SQS
   free tier + small Fly Machines)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/full-saga-deployment-plan.md` around lines 232 - 233, Update the Phase 1
target cost line to reflect the resolved D2 identity provider choice: replace
the reference to "Auth0 free tier" with "Keycloak self-hosted on Fly" (or
similar phrasing) so the Phase 1 target cost statement and the D2 decision are
consistent; ensure the sentence that currently reads "Phase 1 target: ≤ $15/mo
(Postgres free with Fly + Auth0 free tier + SQS free tier + small Fly Machines)"
mentions Keycloak self-hosted on Fly instead of Auth0 and, if needed, adjust the
parenthetical cost components to reflect any small additional compute or
maintenance cost for self-hosting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@docs/full-saga-deployment-plan.md`:
- Around line 232-233: Update the Phase 1 target cost line to reflect the
resolved D2 identity provider choice: replace the reference to "Auth0 free tier"
with "Keycloak self-hosted on Fly" (or similar phrasing) so the Phase 1 target
cost statement and the D2 decision are consistent; ensure the sentence that
currently reads "Phase 1 target: ≤ $15/mo (Postgres free with Fly + Auth0 free
tier + SQS free tier + small Fly Machines)" mentions Keycloak self-hosted on Fly
instead of Auth0 and, if needed, adjust the parenthetical cost components to
reflect any small additional compute or maintenance cost for self-hosting.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2974a9d4-207f-4d3a-9fc2-74280b27c195

📥 Commits

Reviewing files that changed from the base of the PR and between fa4e1db and 9106b8a.

📒 Files selected for processing (1)
  • docs/full-saga-deployment-plan.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
docs/STATUS.md (2)

5-5: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update Last updated to today’s date (2026-05-28).

docs/STATUS.md changed in this PR, but the header still shows 2026-05-26, which makes the status entry stale.

As per coding guidelines, "docs/STATUS.md: STATUS.md is the cross-session entry point. When this changes, the "Last updated:" date should be today. Flag if the date is stale relative to the diff."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/STATUS.md` at line 5, The STATUS.md header shows "Last updated:
2026-05-26" which is stale; update that string to today's date "Last updated:
2026-05-28" by replacing the existing "Last updated:" line in docs/STATUS.md
(search for the literal "Last updated:" header) so the document reflects the
current change set.

103-103: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Decision status is outdated relative to the linked plan.

This line says D1–D4 are still open blockers, but docs/full-saga-deployment-plan.md records them as resolved on 2026-05-27 (Resolved decisions (2026-05-27)). Please align STATUS with the current decision state to avoid misleading the next session.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/STATUS.md` at line 103, Update the Status entry titled "Active multi-PR
effort: Full-saga demo deployment" so it no longer claims "four open decisions
(D1–D4) block Phase 1"; instead reflect that those decisions were resolved on
2026-05-27 per docs/full-saga-deployment-plan.md (for example, change the
wording to note the decisions were resolved on 2026-05-27 and remove the "block
Phase 1" phrasing), keeping the rest of the sentence and link intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@docs/STATUS.md`:
- Line 5: The STATUS.md header shows "Last updated: 2026-05-26" which is stale;
update that string to today's date "Last updated: 2026-05-28" by replacing the
existing "Last updated:" line in docs/STATUS.md (search for the literal "Last
updated:" header) so the document reflects the current change set.
- Line 103: Update the Status entry titled "Active multi-PR effort: Full-saga
demo deployment" so it no longer claims "four open decisions (D1–D4) block Phase
1"; instead reflect that those decisions were resolved on 2026-05-27 per
docs/full-saga-deployment-plan.md (for example, change the wording to note the
decisions were resolved on 2026-05-27 and remove the "block Phase 1" phrasing),
keeping the rest of the sentence and link intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7d602ca2-1360-4e7e-8bc7-533d09a5e19d

📥 Commits

Reviewing files that changed from the base of the PR and between 9106b8a and fee3026.

📒 Files selected for processing (1)
  • docs/STATUS.md

Three small edits:

1. CodeRabbit #1 (full-saga-deployment-plan.md:335): the "Related docs"
   pointer said STATUS.md references this doc "under 'Open issues'" but
   the entry actually lives under "Next." Fixed to match.

2. CodeRabbit #2 (STATUS.md:103): the "Next" entry still said "four open
   decisions (D1-D4) block Phase 1" but the plan doc now lists D1-D4 as
   resolved 2026-05-27. Updated to reflect resolved state + name the
   chosen options + point at Phase 1A as the next concrete work.

3. Seq named as the Phase 3 telemetry choice. Sourced from an article on
   structured logging + distributed tracing for microservices with Seq.
   Seq fits this deployment shape better than App Insights — unified logs
   + traces in one UI, self-hostable on Fly with persistent volume
   (matches D2 Keycloak pattern), free tier covers demo scope, OTLP-native
   (slots into existing OpenTelemetry export with one config-line change).
   Gotcha captured: pin OpenTelemetry.Instrumentation.* packages
   explicitly in Directory.Packages.props, since non-stable RC versions
   for instrumentations like StackExchangeRedis differ across major
   bumps.

No new rule encoding — the article reinforces existing CLAUDE.md rules
(structured logging with message templates, OpenTelemetry instrumentation
patterns) without adding net-new lessons worth encoding.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/STATUS.md (1)

5-5: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the stale "Last updated" date.

The file has changes (line 103), but "Last updated" still shows 2026-05-26. It should be updated to 2026-05-28 (today) or at minimum 2026-05-27 to reflect this PR's modifications. As per coding guidelines, STATUS.md's "Last updated" date should match when the file changes to maintain accurate cross-session tracking.

📅 Proposed fix
-**Last updated:** 2026-05-26 (CatalogService Clean Architecture → VSA collapse; carve-out retired — all five services now share one shape)
+**Last updated:** 2026-05-28 (Full-saga deployment plan integrated; D1–D4 decisions resolved)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/STATUS.md` at line 5, Update the "Last updated" date string in
docs/STATUS.md (the "**Last updated:**" line) from 2026-05-26 to 2026-05-28 (or
at minimum 2026-05-27) so the file's header matches the current PR changes;
locate the "**Last updated:**" token and replace the date accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@docs/STATUS.md`:
- Line 5: Update the "Last updated" date string in docs/STATUS.md (the "**Last
updated:**" line) from 2026-05-26 to 2026-05-28 (or at minimum 2026-05-27) so
the file's header matches the current PR changes; locate the "**Last updated:**"
token and replace the date accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e281204a-3412-4a05-a5d1-78ce6e370be3

📥 Commits

Reviewing files that changed from the base of the PR and between fee3026 and 942e909.

📒 Files selected for processing (2)
  • docs/STATUS.md
  • docs/full-saga-deployment-plan.md

@emeraldleaf
emeraldleaf merged commit 253b6a5 into main May 28, 2026
7 checks passed
emeraldleaf added a commit that referenced this pull request May 28, 2026
* chore(claude): encode distributed rate-limit rule + annotate registration sites

ASP.NET Core's built-in RequireRateLimiting + AddFixedWindowLimiter /
AddSlidingWindowLimiter stores counters in-process — correct for
single-instance deployment, but the limit silently multiplies by N when
scaled to N instances. Each instance enforces its own counter; a client
hitting any instance gets a fresh allowance.

NextAurora is single-instance everywhere today (CatalogService deployed
on one Fly Machine; the other four services local), so the in-memory
limiter is correct *for now*. This PR encodes the rule preventatively
+ marks the two registration sites with the future-swap trigger.

Encoded across the three canonical sites:
- CLAUDE.md "Security Requirements → Rate Limiting": extended bullet
  covering the in-memory weakening + Redis-backed swap + Lua-EVAL for
  atomic increment+TTL.
- .coderabbit.yaml `**/Endpoints/**/*.cs` path: RATE-LIMIT SHAPE block
  flagging missing justification comments + deployment-config scale-out
  without paired limiter swap.
- .claude/agents/architecture-reviewer.md Endpoints pattern checklist:
  Rate-limiter shape bullet (Should-consider on single-instance,
  Must-fix when scaled to 2+ instances).

Reference-comment annotation at both registration sites (the
"AddFixedWindowLimiter" calls in CatalogService/Program.cs and
PaymentService/Program.cs) — documents WHY in-memory is correct today
and WHAT triggers the swap. Mirrors the worked-sketch pattern from PR
#39 (NotificationService DependencyInjection.cs).

Connection to docs/full-saga-deployment-plan.md (PR #42): Phase 3
deliverable already lists the audit + Redis-backed swap when scale-out
happens. The rule encoded here is the rule that audit applies.

Source: a Milan Jovanović article on distributed rate limiting with
Redis (the "Scale Rate Limiting with Redis" piece) — same pattern as
other rule-encoding PRs in this session: read article → encode the
discrete lesson → apply where it'll be findable.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* chore(claude): address CodeRabbit — inline rule comments end with 'See CLAUDE.md.'

CodeRabbit flagged the two rate-limiter annotation comments for not ending
with the canonical grep-auditable marker. The repo convention (CLAUDE.md
"Debugging Discipline") is that any inline comment summarizing a CLAUDE.md
rule ends with `See CLAUDE.md.` so it's findable via grep.

Reshaped both comments: the section reference now reads as a preceding
"Rule: Security Requirements → Rate Limiting." sentence, and the comment
ends with the literal `See CLAUDE.md.` marker. Keeps the section pointer
for human readers while satisfying the grep convention exactly.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@emeraldleaf
emeraldleaf deleted the plan/full-saga-deployment branch June 4, 2026 00:40
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