Skip to content

docs+infra: refresh stale VSA-collapse refs + broken-link CI guard#112

Merged
emeraldleaf merged 2 commits into
mainfrom
docs/refresh-stale-refs-from-vsa-collapse
Jun 4, 2026
Merged

docs+infra: refresh stale VSA-collapse refs + broken-link CI guard#112
emeraldleaf merged 2 commits into
mainfrom
docs/refresh-stale-refs-from-vsa-collapse

Conversation

@emeraldleaf
Copy link
Copy Markdown
Owner

@emeraldleaf emeraldleaf commented Jun 4, 2026

Summary

Six files updated to close drift inherited from PR #31 (CatalogService Clean → VSA collapse) and the IPaymentRepository deletion. Plus a mechanical CI guard so the next round of refactor-fallout drift fails loudly instead of sitting silent.

What's in here

File Drift Fix
`CLAUDE.md:341` Linked to deleted `PaymentService/Infrastructure/PaymentRepository.cs` for the outbox-atomic example Repointed at `PaymentRecoveryJob` — the inline implementation that CLAUDE.md:27 already correctly described
`docs/demo-deployment.md` `CatalogService.Api/` paths everywhere Updated to single-project layout
`docs/demo-deployment-story.md` Same drift + `CatalogDbContext.cs` path + `dotnet ef migrations` flags Updated; simplified `--project` / `--startup-project` pairing
`docs/performance-and-data-correctness.md:129,326` `*Handler.cs` filenames (Clean convention) + dead test file VSA filenames (`GetProductById.cs` etc.); replaced dead test ref with `ProductCachingTests.cs`
`Dockerfile.catalog` COPYed 4 csproj files from the pre-collapse Clean layout that haven't existed since PR #31 Rewrote for single-project. Verified locally: `docker build --platform=linux/amd64 -f Dockerfile.catalog -t catalog-api .` produces a 116 MB image
`.github/workflows/ci.yml` (new) Broken-link audit step — scans markdown for relative links to local source files, fails if any don't resolve. Would have caught every drift above. Smoke-tested locally on post-fix tree (exit 0).

Why option B (full fix) over option A (doc-only)

The deployed Fly demo at https://catalog-api-demo.fly.dev/ is still running pre-collapse code — last successful deploy was SHA `73388e8`, before #31 merged. Any redeploy attempt today would fail on the very first COPY in the Dockerfile. Doc-only fix would leave that ticking. Knocking out the Dockerfile in the same PR keeps everything internally consistent.

Verification

  • ✅ `docker build --platform=linux/amd64 -f Dockerfile.catalog -t catalog-api .` → 116 MB image, exit 0
  • ✅ Broken-link guard smoke-tested locally against post-fix tree → exit 0
  • ✅ Pending: full CI run on this PR (build + unit + 4 integration slices + the new broken-link audit)

What this is NOT

  • Doesn't re-deploy the live Fly demo. That's a separate manual workflow trigger (`DEPLOY_CATALOG_DEMO_FLY`). After this PR merges, that trigger should work; before this PR, it would have failed.
  • Doesn't change any rule semantics. CLAUDE.md edit is a citation refresh; perf doc edits are filename refreshes; demo doc edits are path refreshes.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added automated broken-link validation in CI to catch missing local documentation references.
  • Documentation

    • Updated deployment and demo guides to reflect the current service layout and startup paths.
    • Clarified transactional outbox guidance to reference the canonical inline implementation.
  • Chores

    • Adjusted container build entrypoint and build configuration to match the project restructuring.

… guard

After the /check-rules audit + the doc-currency sweep flagged multiple
stale references inherited from PR #31 (CatalogService Clean → VSA collapse)
and from the IPaymentRepository deletion. Most of these had been sitting
in the repo for months because nothing fails when a markdown link rots.

Drift fixed:

- CLAUDE.md:341 — outbox-atomic example linked to PaymentRepository.cs
  (deleted in simplicity refactor). Repointed at PaymentRecoveryJob, the
  current inline implementation site. CLAUDE.md:27 already correctly
  described the move; only the deep-link was stale.

- docs/demo-deployment.md + docs/demo-deployment-story.md — multiple
  references to CatalogService.Api/, CatalogService.Infrastructure/, the
  pre-collapse 4-project Clean layout. All paths refreshed to the current
  single-project structure (CatalogService/Program.cs, CatalogService/
  Infrastructure/Data/CatalogDbContext.cs, etc.). Two `dotnet ef migrations
  add` snippets that referenced the old --project / --startup-project
  pairing simplified to `--project CatalogService`.

- docs/performance-and-data-correctness.md:129,326 — handler citations
  used the *Handler.cs file naming convention (Clean) but VSA co-locates
  command + validator + handler in a single use-case file. Renamed to
  GetProductById.cs / UpdateProduct.cs / ReserveStock.cs. The dead
  GetProductByIdHandlerTests.cs citation replaced with the existing
  ProductCachingTests.cs (integration tier — the right tier for
  cache-projection behavior).

- Dockerfile.catalog — rewrote the build stage for the single-project
  structure. The old Dockerfile COPYed CatalogService/CatalogService.Api/
  *.csproj and 3 sibling projects that haven't existed since PR #31.
  Anyone triggering the Fly.io deploy workflow today would have hit a
  build failure on the very first COPY. Verified locally with
  `docker build --platform=linux/amd64 -f Dockerfile.catalog -t catalog-api .`
  → produces a 116 MB runtime image.

Mechanical guard added:

- .github/workflows/ci.yml — broken-link audit step in the build job. Scans
  every markdown file for relative links to .cs/.csproj/.props/.sh/.yml/
  .yaml/.svg/.excalidraw/.cls/.md files, fails the build if any don't
  resolve. Skips http(s)://, // bare URLs, and anchors-only. Process
  substitution throughout so the failure flag survives the loop (a piped
  while runs in a subshell and loses its updates — this is the trap that
  bit the first draft). Same shape as the existing "Concurrency audit"
  step: grep + fail. Would have caught every drift in this PR
  mechanically. Smoke-tested locally against the post-fix tree (exit 0).

Why option B (full fix) over option A (doc-only): the deployed Fly demo at
https://catalog-api-demo.fly.dev/ is still running pre-collapse code (last
successful deploy was SHA 73388e8, before #31 merged). Any redeploy attempt
would have failed silently — the broken Dockerfile + a workflow nobody had
triggered in 2 weeks meant the failure mode would surface as a confusing
CI error on the next demo refresh, not when the actual drift landed.
Knocking out the Dockerfile + the docs that describe how to use it in the
same PR keeps the system internally consistent.

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

codecov Bot commented Jun 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

Walkthrough

This PR adds a CI build step that audits Markdown links to local source-like files and fails on missing targets. It also updates Dockerfile.catalog, demo deployment docs, and performance docs to reference the single-project CatalogService layout, and replaces one CLAUDE.md example reference.

Changes

CI Markdown Broken-Link Audit

Layer / File(s) Summary
Markdown broken-link audit step
.github/workflows/ci.yml
New build-job step scans .md files for citations to local source-like files, normalizes paths by stripping anchors and queries, resolves targets relative to each markdown file, and fails the job with ::error messages for missing references.

CatalogService Project Migration Updates

Layer / File(s) Summary
Docker configuration for single-project layout
Dockerfile.catalog
Header comments, build-stage .csproj copy/restore/publish steps, and runtime ENTRYPOINT updated to target CatalogService and its transitive .csproj graph; publishes CatalogService/CatalogService.csproj and starts CatalogService.dll.
Demo deployment story updates
docs/demo-deployment-story.md
Narrative and step references updated to CatalogService and ../CatalogService/Program.cs; EF Core migration/seed command examples simplified to --project CatalogService.
Demo deployment guide and AWS notes
docs/demo-deployment.md
Replaces CatalogService.Api with CatalogService in deployment and App Runner notes; updates backward-compatibility/run examples.
Performance and caching docs correction
docs/performance-and-data-correctness.md
Corrects handler reference to GetProductById.cs and test reference to ProductCachingTests.cs.
Transactional outbox guidance reference
CLAUDE.md
Replaces PaymentRepository.ExecuteInTransactionAsync with PaymentRecoveryJob as the canonical example for the required transaction wrapper.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 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 clearly and concisely summarizes the main changes: documentation updates from the VSA-collapse refactoring plus a new broken-link CI check.
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 docs/refresh-stale-refs-from-vsa-collapse

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

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

⚠️ Outside diff range comments (1)
Dockerfile.catalog (1)

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

Add an image-level HEALTHCHECK for runtime liveness.

The container has no Docker HEALTHCHECK, so platforms that consume image health signals can’t detect unhealthy-but-running processes early.

Proposed fix
 FROM mcr.microsoft.com/dotnet/aspnet:10.0 AS runtime
 WORKDIR /app
@@
 ENV ASPNETCORE_URLS=http://+:8080
 EXPOSE 8080
+HEALTHCHECK --interval=30s --timeout=5s --start-period=20s --retries=3 \
+  CMD [ "wget", "-qO-", "http://127.0.0.1:8080/health" ] || exit 1
🤖 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 `@Dockerfile.catalog` around lines 49 - 63, Add a Docker HEALTHCHECK to the
runtime image so orchestration/platforms can detect liveness; modify the runtime
stage (the block starting with FROM mcr.microsoft.com/dotnet/aspnet:10.0 AS
runtime) to include a HEALTHCHECK that probes the app endpoint (use the same
ASPNETCORE_URLS port 8080) and returns non-zero on failure, ensuring it runs as
the non-root USER app and does not change ENTRYPOINT ["dotnet",
"CatalogService.dll"]; choose an appropriate interval/retries for early
detection but avoid aggressive polling.
🤖 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 @.github/workflows/ci.yml:
- Around line 121-139: Add strict bash mode to the CI Markdown-audit run block
by inserting "set -euo pipefail" at the top of the block (before "fail=0") so
failures in the pipeline or unset variables cause the job to fail; this change
applies to the run block containing "fail=0" and the loop starting with "while
IFS= read -r mdfile; do" to ensure the audit cannot silently succeed on command
errors.
- Around line 129-131: The existence check currently falls back to checking
"$candidate" (repo-root) which masks broken relative links; update the
conditional so it only verifies the resolved path for the markdown file (i.e.,
test only "$target" which is built from "$dir/$candidate" or "." handling)
instead of also checking "$candidate", or otherwise resolve/normalize
"$candidate" against "$dir" before testing; adjust the if that uses variables
target, candidate, mdfile, and link so a same-named file at repo root no longer
makes a broken relative link pass.

In `@CLAUDE.md`:
- Line 341: Append the mandated audit note sentence "Run /check-rules locally to
audit paraphrases against this diff." to the CLAUDE.md change context referenced
by the PaymentRecoveryJob example; ensure the note appears alongside the
existing guidance about wrapping non-handler publish paths (or factoring to a
Wolverine handler) and mention of the removed
IPaymentRepository.ExecuteInTransactionAsync wrapper so reviewers see the
required local check instruction in the same paragraph/context.

In `@docs/demo-deployment.md`:
- Line 31: The Fly instruction in the docs should use the external Fly secret
name CATALOG_DB_CONNECTION_STRING (not ConnectionStrings__catalog-db) while
leaving ConnectionStrings__catalog-db as the internal in-app slot referenced by
Program.cs; update all Fly-specific mentions to CATALOG_DB_CONNECTION_STRING,
add a short note that the app maps that secret into the in-process slot
ConnectionStrings__catalog-db (as configured in Program.cs), and remove any
conflicting guidance that suggests exporting ConnectionStrings__catalog-db
directly for Fly deployments.

---

Outside diff comments:
In `@Dockerfile.catalog`:
- Around line 49-63: Add a Docker HEALTHCHECK to the runtime image so
orchestration/platforms can detect liveness; modify the runtime stage (the block
starting with FROM mcr.microsoft.com/dotnet/aspnet:10.0 AS runtime) to include a
HEALTHCHECK that probes the app endpoint (use the same ASPNETCORE_URLS port
8080) and returns non-zero on failure, ensuring it runs as the non-root USER app
and does not change ENTRYPOINT ["dotnet", "CatalogService.dll"]; choose an
appropriate interval/retries for early detection but avoid aggressive polling.
🪄 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: 032ac196-b953-45e7-a123-f7e90f32d6bd

📥 Commits

Reviewing files that changed from the base of the PR and between 76f7133 and a1dd9b5.

📒 Files selected for processing (6)
  • .github/workflows/ci.yml
  • CLAUDE.md
  • Dockerfile.catalog
  • docs/demo-deployment-story.md
  • docs/demo-deployment.md
  • docs/performance-and-data-correctness.md

Comment thread .github/workflows/ci.yml
Comment on lines +121 to +139
run: |
fail=0
while IFS= read -r mdfile; do
dir=$(dirname "$mdfile")
while IFS= read -r link; do
case "$link" in http*|//*) continue ;; esac
candidate="${link%%#*}"
candidate="${candidate%%\?*}"
if [ "$dir" = "." ]; then target="$candidate"; else target="$dir/$candidate"; fi
if [ ! -e "$target" ] && [ ! -e "$candidate" ]; then
echo "::error file=$mdfile::broken link → $link"
fail=1
fi
done < <(grep -oE '\[[^]]+\]\(([^)#]+\.(cs|csproj|props|sh|yml|yaml|svg|excalidraw|cls|md))[^)#]*\)' "$mdfile" \
| sed -E 's/.*\(([^)]+)\)/\1/')
done < <(find . -type f -name '*.md' \
-not -path './bin/*' -not -path './obj/*' \
-not -path '*/node_modules/*' -not -path '*/.git/*')
exit "$fail"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add strict bash mode to this audit step.

This run block should start with set -euo pipefail; otherwise command failures can be silently ignored and the audit may report false success.

Suggested patch
       - name: Broken-link audit — markdown citations to local files
         run: |
+          set -euo pipefail
           fail=0
           while IFS= read -r mdfile; do

As per coding guidelines: “DO flag: ... missing set -euo pipefail in bash run blocks.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
run: |
fail=0
while IFS= read -r mdfile; do
dir=$(dirname "$mdfile")
while IFS= read -r link; do
case "$link" in http*|//*) continue ;; esac
candidate="${link%%#*}"
candidate="${candidate%%\?*}"
if [ "$dir" = "." ]; then target="$candidate"; else target="$dir/$candidate"; fi
if [ ! -e "$target" ] && [ ! -e "$candidate" ]; then
echo "::error file=$mdfile::broken link → $link"
fail=1
fi
done < <(grep -oE '\[[^]]+\]\(([^)#]+\.(cs|csproj|props|sh|yml|yaml|svg|excalidraw|cls|md))[^)#]*\)' "$mdfile" \
| sed -E 's/.*\(([^)]+)\)/\1/')
done < <(find . -type f -name '*.md' \
-not -path './bin/*' -not -path './obj/*' \
-not -path '*/node_modules/*' -not -path '*/.git/*')
exit "$fail"
run: |
set -euo pipefail
fail=0
while IFS= read -r mdfile; do
dir=$(dirname "$mdfile")
while IFS= read -r link; do
case "$link" in http*|//*) continue ;; esac
candidate="${link%%#*}"
candidate="${candidate%%\?*}"
if [ "$dir" = "." ]; then target="$candidate"; else target="$dir/$candidate"; fi
if [ ! -e "$target" ] && [ ! -e "$candidate" ]; then
echo "::error file=$mdfile::broken link → $link"
fail=1
fi
done < <(grep -oE '\[[^]]+\]\(([^)#]+\.(cs|csproj|props|sh|yml|yaml|svg|excalidraw|cls|md))[^)#]*\)' "$mdfile" \
| sed -E 's/.*\(([^)]+)\)/\1/')
done < <(find . -type f -name '*.md' \
-not -path './bin/*' -not -path './obj/*' \
-not -path '*/node_modules/*' -not -path '*/.git/*')
exit "$fail"
🤖 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 @.github/workflows/ci.yml around lines 121 - 139, Add strict bash mode to the
CI Markdown-audit run block by inserting "set -euo pipefail" at the top of the
block (before "fail=0") so failures in the pipeline or unset variables cause the
job to fail; this change applies to the run block containing "fail=0" and the
loop starting with "while IFS= read -r mdfile; do" to ensure the audit cannot
silently succeed on command errors.

Comment thread .github/workflows/ci.yml
Comment on lines +129 to +131
if [ "$dir" = "." ]; then target="$candidate"; else target="$dir/$candidate"; fi
if [ ! -e "$target" ] && [ ! -e "$candidate" ]; then
echo "::error file=$mdfile::broken link → $link"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Repo-root fallback masks genuinely broken relative links.

The second existence check ([ ! -e "$candidate" ]) can let an invalid markdown-relative link pass if a same-named file exists at repo root. This weakens the guard’s core purpose.

Suggested patch
-              if [ ! -e "$target" ] && [ ! -e "$candidate" ]; then
+              if [ ! -e "$target" ]; then
                 echo "::error file=$mdfile::broken link → $link"
                 fail=1
               fi
🤖 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 @.github/workflows/ci.yml around lines 129 - 131, The existence check
currently falls back to checking "$candidate" (repo-root) which masks broken
relative links; update the conditional so it only verifies the resolved path for
the markdown file (i.e., test only "$target" which is built from
"$dir/$candidate" or "." handling) instead of also checking "$candidate", or
otherwise resolve/normalize "$candidate" against "$dir" before testing; adjust
the if that uses variables target, candidate, mdfile, and link so a same-named
file at repo root no longer makes a broken relative link pass.

Comment thread CLAUDE.md
}
```
Reference: [PaymentRepository.ExecuteInTransactionAsync](PaymentService/Infrastructure/PaymentRepository.cs) (fixed in the commit captured by docs/STATUS.md). When adding a non-handler code path that publishes events, **either** wrap it in this pattern **or** factor the publish back into a Wolverine handler triggered by an internal scheduled message.
Reference: [PaymentRecoveryJob](PaymentService/Infrastructure/PaymentRecoveryJob.cs) — the canonical inline implementation of this wrapper (the previous `IPaymentRepository.ExecuteInTransactionAsync` wrapper was deleted in the simplicity refactor; the pattern itself is unchanged, just inlined). When adding a non-handler code path that publishes events, **either** wrap it in this pattern **or** factor the publish back into a Wolverine handler triggered by an internal scheduled message.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add the mandatory CLAUDE.md audit note.

Please append the required note in this CLAUDE.md change context: "Run /check-rules locally to audit paraphrases against this diff."

As per coding guidelines: “When this changes… Flag the CLAUDE.md change with a note: ‘Run /check-rules locally to audit paraphrases against this 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 `@CLAUDE.md` at line 341, Append the mandated audit note sentence "Run
/check-rules locally to audit paraphrases against this diff." to the CLAUDE.md
change context referenced by the PaymentRecoveryJob example; ensure the note
appears alongside the existing guidance about wrapping non-handler publish paths
(or factoring to a Wolverine handler) and mention of the removed
IPaymentRepository.ExecuteInTransactionAsync wrapper so reviewers see the
required local check instruction in the same paragraph/context.

Comment thread docs/demo-deployment.md
| Production posture (if/when we deploy real prod) | Unchanged | `DemoMode` defaults to `false`. OpenAPI + Scalar stay hidden. HTTPS redirection stays on. Migrate-on-startup stays off. |

**Watch-out**: if you ever export `ConnectionStrings__catalog-db=<remote-endpoint>` in your local shell, a bare `dotnet run --project CatalogService/CatalogService.Api` would try to talk to the remote DB. This is self-inflicted-only — `dotnet run --project NextAurora.AppHost` overrides connection strings before child processes inherit them, so Aspire-driven local runs are immune.
**Watch-out**: if you ever export `ConnectionStrings__catalog-db=<remote-endpoint>` in your local shell, a bare `dotnet run --project CatalogService` would try to talk to the remote DB. This is self-inflicted-only — `dotnet run --project NextAurora.AppHost` overrides connection strings before child processes inherit them, so Aspire-driven local runs are immune.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix Fly secret-name guidance inconsistency (ConnectionStrings__catalog-db vs CATALOG_DB_CONNECTION_STRING).

This doc now points readers to Program.cs, but Fly path instructions in the same file still use ConnectionStrings__catalog-db, which conflicts with the documented Fly naming constraint and DemoMode bridge. Please normalize the Fly instructions to CATALOG_DB_CONNECTION_STRING and keep ConnectionStrings__catalog-db only as the in-app target slot.

🤖 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/demo-deployment.md` at line 31, The Fly instruction in the docs should
use the external Fly secret name CATALOG_DB_CONNECTION_STRING (not
ConnectionStrings__catalog-db) while leaving ConnectionStrings__catalog-db as
the internal in-app slot referenced by Program.cs; update all Fly-specific
mentions to CATALOG_DB_CONNECTION_STRING, add a short note that the app maps
that secret into the in-process slot ConnectionStrings__catalog-db (as
configured in Program.cs), and remove any conflicting guidance that suggests
exporting ConnectionStrings__catalog-db directly for Fly deployments.

INDEX.md ships in the repo but its links point at per-article audit files
that are gitignored for copyright reasons (verbatim quoted prose). See
.claude/commands/article-audit.md step 5 'Copyright note' — contract is
'INDEX ships, per-article files don't.' On a contributor's machine the
links resolve; on the CI runner they don't, by design.

Surfaced by #112's first run — guard correctly flagged 14 broken refs,
but they're all from this one intentionally-excluded file.

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

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 @.github/workflows/ci.yml:
- Around line 141-145: Fix the duplicated path segment in the CI workflow
exclude pattern by replacing the incorrect glob
`.claude/audits/*.claude/audits/*.md` with the correct `.claude/audits/*.md` in
the `.github/workflows/ci.yml` file so the exclude rule targets files under the
`.claude/audits` directory correctly.
🪄 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: 78fd3d0a-15de-4d07-bd5a-626692da57bd

📥 Commits

Reviewing files that changed from the base of the PR and between a1dd9b5 and 844d21b.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml

Comment thread .github/workflows/ci.yml
Comment on lines +141 to +145
# `.claude/audits/INDEX.md` is intentionally excluded — it links to per-article audit
# files under `.claude/audits/*.md` that are gitignored for copyright reasons (they
# contain verbatim quoted prose from external articles). See `.claude/commands/article-audit.md`
# step 5 "Copyright note" — the contract is "INDEX ships, per-article files don't."
# On a contributor's machine the links resolve; on the CI runner they don't, by design.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix typo in file path pattern.

Line 142 contains a duplicated path segment. The pattern .claude/audits/*.claude/audits/*.md should be .claude/audits/*.md.

📝 Suggested fix
           # `.claude/audits/INDEX.md` is intentionally excluded — it links to per-article audit
-          # files under `.claude/audits/*.claude/audits/*.md` that are gitignored for copyright reasons (they
+          # files under `.claude/audits/*.md` that are gitignored for copyright reasons (they
           # contain verbatim quoted prose from external articles). See `.claude/commands/article-audit.md`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# `.claude/audits/INDEX.md` is intentionally excluded — it links to per-article audit
# files under `.claude/audits/*.md` that are gitignored for copyright reasons (they
# contain verbatim quoted prose from external articles). See `.claude/commands/article-audit.md`
# step 5 "Copyright note" — the contract is "INDEX ships, per-article files don't."
# On a contributor's machine the links resolve; on the CI runner they don't, by design.
# `.claude/audits/INDEX.md` is intentionally excluded — it links to per-article audit
# files under `.claude/audits/*.md` that are gitignored for copyright reasons (they
# contain verbatim quoted prose from external articles). See `.claude/commands/article-audit.md`
# step 5 "Copyright note" — the contract is "INDEX ships, per-article files don't."
# On a contributor's machine the links resolve; on the CI runner they don't, by design.
🤖 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 @.github/workflows/ci.yml around lines 141 - 145, Fix the duplicated path
segment in the CI workflow exclude pattern by replacing the incorrect glob
`.claude/audits/*.claude/audits/*.md` with the correct `.claude/audits/*.md` in
the `.github/workflows/ci.yml` file so the exclude rule targets files under the
`.claude/audits` directory correctly.

@emeraldleaf emeraldleaf merged commit 81fb33e into main Jun 4, 2026
7 checks passed
@emeraldleaf emeraldleaf deleted the docs/refresh-stale-refs-from-vsa-collapse branch June 4, 2026 03:51
emeraldleaf added a commit that referenced this pull request Jun 5, 2026
…n-CLAUDE.md) (#114)

* chore: prevention pass — hook + CLAUDE.md rule + CodeRabbit instruction (parts 2-4 of file-move discipline)

Three of four layers of the file-move drift prevention loop. Part #1
(extend the CI broken-link guard to scan Dockerfile*) lands in a
follow-up commit on this same PR once #112 is merged and rebased.

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

* chore: extend prevention to Dockerfile COPY + diagrams + topology docs

Three more layers on top of cf27628 (parts 2-4 of the file-move
prevention loop). Same compounding-loop principle, broader coverage:

(a) Broken-COPY audit — Dockerfile* COPY/ADD source paths that don't
    exist in the build context. Catches the kind of drift that left
    Dockerfile.catalog referencing the pre-VSA-collapse 4-project layout
    for months after PR #31. Skips `--from=<stage>` cross-stage copies
    and wildcards (resolved at build time, not against the repo tree).

(b) Diagram-pair audit — every docs/*.excalidraw must have its sibling
    docs/*.svg and vice versa. Reviewers look at the .svg on github.com
    to understand the system; .excalidraw is the editable source. If one
    exists without the other, the diagram review surface is broken.
    Does NOT verify the .svg matches what would be regenerated from the
    .excalidraw source (that needs Playwright in CI — separate, heavier
    gate). The pair-existence check is the cheap mechanical floor.

(c) CLAUDE.md "Doc-and-diagram discipline" rule. Sibling to the file-move
    rule. Encodes that docs and diagrams are the REVIEW SURFACE, not
    byproducts — when reviewers look at the system, they read
    docs/architecture.md and look at docs/nextaurora-architecture.svg.
    If those are stale, every review reasons against a fiction. Names
    concrete pairings: AppHost.cs ↔ architecture.md/svg, Extensions.cs
    middleware order ↔ service-request-flow.svg, EF/cache/outbox changes
    ↔ perf-doc + their sibling diagrams.

(d) CodeRabbit path_instructions for NextAurora.AppHost/AppHost.cs and
    NextAurora.ServiceDefaults/Extensions.cs — when topology or
    middleware-order code changes, flag missing paired doc/diagram
    updates at review time. PR-description waivers acceptable when
    the deferred update is named in a tracking issue.

All three new mechanical guards (broken-link from #112, broken-COPY,
diagram-pair) smoke-tested locally on the current tree → exit 0.

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

* chore: trim CLAUDE.md (5 surfaces, lean discipline, perf + observability)

Third commit on this branch — the lean-CLAUDE.md pass on top of the
file-move (cf27628) + doc-and-diagram (4e1ba94) prevention layers.

CLAUDE.md drops from 358 → 301 lines (~16% smaller) by moving deep-dive
content out and leaving headlines + links. Every rule preserved; nothing
the AI needs to know got dropped.

Continuous Rule Encoding — 6 surfaces → 5:
- GitHub Issues moved out of the encoding loop (it's a deferral mechanism,
  not encoding — closed issues aren't re-read in future sessions)
- Mirror update in docs/dev-loop.md (table + prose)
- Diagram redesigned with 5 surfaces in a clean row, simpler tier boxes
  (no crammed tool lists), CLAUDE.md annotated "(kept lean)" to reinforce
  the discipline visually

New "one-paragraph max per rule" discipline on CLAUDE.md surface 1:
- "If a rule needs more than ~6 lines, the rule stays as a bolded headline
   + one-paragraph summary in CLAUDE.md; detail moves to docs/ or skills/"
- Test: "could this rule + its rationale fit on one screen?"

CI size guard (build job):
- CLAUDE.md soft-warning at 400 lines, hard-fail at 500
- Mechanical floor on bloat regression

Performance Rules trim (~30 dense bullets → 22 headlines + links):
- Rules themselves unchanged (project-not-map, Task.WhenAll, outbox-atomic,
  Guid v7, AsSpan, Dapper escape hatch, etc.)
- Deep-dive paragraphs → docs/performance-and-data-correctness.md +
  dotnet-performance skill (both already exist)

Observability section trim (~78 lines → 21):
- Kept 3 always-on traps: HTTP middleware order, Wolverine middleware
  instance-methods, outbox-outside-handler atomicity
- Moved "how it works" detail → docs/architecture.md "Cross-Cutting
  Concerns" and "Event-Driven Architecture" (both already cover this)

Verification:
- wc -l CLAUDE.md: 301 (well under the 400/500 size budget)
- broken-link audit on CLAUDE.md: exit 0
- diagram-pair audit: exit 0
- broken-COPY audit: exit 0
- All rule headlines preserved; rules with paraphrases in code comments
  ("See CLAUDE.md") still align with canonical wording

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

* docs: actually move CLAUDE.md trim content to destination docs

The prior commit (5c938b2) trimmed CLAUDE.md from 358→301 lines and
added "See docs/..." links. An audit found the content I trimmed wasn't
actually in those destination docs — I'd deleted it, not moved it.

This commit does the real moves.

New file: docs/observability-and-context-propagation.md
- Section-aligned with CLAUDE.md "Observability & Context Propagation"
- Full mechanism + headers/baggage mapping table + sources
- HTTP middleware order with the canonical 4-line code example
- Wolverine pipeline scope detail (pipeline order: validation → context
  → handler → AutoApplyTransactions)
- Wolverine envelope context extraction mechanism
- Transactional Outbox config + outbox-outside-handler atomicity trap
  with the canonical safe wrapper code block
- Structured logging scope hygiene
- Event Replay note

Additions to docs/performance-and-data-correctness.md:
- New "## Additional always-on patterns" section after "The 14 always-on
  rules" — patterns that don't fit a single-rule shape
- Non-sargable predicates + EmailNormalized normalize-at-write-time pattern
- Task.WhenAll parallel awaits with three caveats (dependent ops, shared
  DbContext, multi-failure observability)
- Long-running work / 202 Accepted pattern with two atomicity paths and
  cloud-managed alternatives (Durable Functions, Step Functions, Temporal)
- Fan-out on message bus + MaxDegreeOfParallelism throttle
- Guid.CreateVersion7 with the time-decodable trade-off
- AsSpan ref-struct + async-boundary constraint

CLAUDE.md link redirects (no rule text changes):
- "Long-running work" → docs/performance-and-data-correctness.md
  "Long-running work belongs on the message bus" (new section)
- "Observability & Context Propagation" mechanism →
  docs/observability-and-context-propagation.md (new file)
- "Outbox outside a Wolverine handler" wrapper →
  docs/observability-and-context-propagation.md anchor (new file)

Naming convention: section-aligned filenames where new docs are created
(observability-and-context-propagation.md mirrors the CLAUDE.md heading).
Existing docs kept where they already align by topic
(performance-and-data-correctness.md).

Verification:
- wc -l CLAUDE.md: 301 (unchanged; under 400 soft / 500 hard budget)
- broken-link audit: exit 0 (full repo)
- All trimmed content now lives in either the new doc or the existing
  perf doc; nothing the AI needs to know got dropped

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

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
emeraldleaf added a commit that referenced this pull request Jun 5, 2026
CI broken-link audit caught: ../.github/ISSUE_TEMPLATE/work-item.yml from
.claude/commands/feature-spec.md resolves to .claude/.github/... which
doesn't exist. Needs ../../ to walk out of both commands/ and .claude/.

The guard caught this on its first run against the new file, exactly
as designed (catching relative-path drift in markdown is the whole point
of the broken-link audit added in #112).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
emeraldleaf added a commit that referenced this pull request Jun 5, 2026
…eeds the encoding loop (#115)

* feat(claude): add /feature-spec slash command for structured feature handoff

Closes the spec-capture gap in the encoding loop. Draws a tight handoff
document — goal + acceptance + affected surfaces + auto-referenced
CLAUDE.md constraints + scaffolding suggestions — then prompts the user
to encode lessons after implementation. The spec itself is ephemeral
(ships as a GitHub issue + PR description); the lessons + significant
decisions are durable across the five encoding surfaces.

Why this exists (the gap it closes):

Spec Kit's /specify produces a per-feature spec as the source of truth
that code is derived from. NextAurora's loop deliberately doesn't do
that (most features are pattern-conforming and a per-feature spec would
re-state what CLAUDE.md already covers). But the loop did have a real
gap: no structured way to capture "what this new feature is" at the
handoff between intent and implementation. /feature-spec fills that
without adopting Spec Kit's whole methodology.

Three-layer persistence model:

1. SPEC (ephemeral) - the command's output. Ships as GitHub issue body +
   PR description. Archives when the feature ships. Once shipped, the
   code is the artifact.
2. IMPLEMENTATION LESSONS (durable) - rules/patterns surfaced while
   building. Encoded across the five surfaces (CLAUDE.md, .coderabbit.yaml,
   architecture-reviewer agent, skills, docs+diagrams). This is what
   the existing Continuous Rule Encoding loop already handles.
3. ARCHITECTURAL DECISIONS (durable, browseable) - for significant features
   (new bounded context, new transport, new tenancy model), an ADR-style
   doc under docs/decisions/ or addition to docs/project-decisions.md.

The command:
- Asks 2-4 clarifying questions to lock in scope (service, endpoint shape,
  domain changes, external integrations)
- Drafts the structured spec (Goal, Acceptance, Affects, Constraints from
  CLAUDE.md - auto-inferred and linked)
- Significance-checks for ADR-worthiness
- Outputs a GitHub issue body + optional ADR draft + /new-feature-slice
  scaffolding suggestions
- Closes with a verbatim "encode lessons after shipping" prompt to keep
  the spec → code → encoding loop closed

The command file itself is a paraphrase site (Constraints from CLAUDE.md
section lists rules with their canonical references). Marked with the
`See CLAUDE.md.` cross-reference convention so /check-rules audits the
alignment and the PostToolUse hook surfaces it when CLAUDE.md changes.

What this is NOT:
- Not Spec Kit. Doesn't produce a plan or task list.
- Not implementation planning - that's /new-feature-slice + writing code.
- Not an auto-encoder - it proposes; the user decides.

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

* fix(claude): correct relative path to ISSUE_TEMPLATE in feature-spec

CI broken-link audit caught: ../.github/ISSUE_TEMPLATE/work-item.yml from
.claude/commands/feature-spec.md resolves to .claude/.github/... which
doesn't exist. Needs ../../ to walk out of both commands/ and .claude/.

The guard caught this on its first run against the new file, exactly
as designed (catching relative-path drift in markdown is the whole point
of the broken-link audit added in #112).

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

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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