diff --git a/.squad/agents/ralph/history.md b/.squad/agents/ralph/history.md index a91babde..2cc8e36b 100644 --- a/.squad/agents/ralph/history.md +++ b/.squad/agents/ralph/history.md @@ -134,3 +134,30 @@ Initial setup complete. - Remote state: `origin` now has only `dev` and `main` **Board state after:** 0 open issues, 0 open PRs. Board clear. + +### 2026-05-08 — Board Sweep: Release Labeling, Mutex Rename, CI Failures Filed + +**Trigger:** User "Ralph, go" — autonomous board sweep. + +**Board scan result:** 2 open issues (#265, #266). 0 open PRs at scan start. + +**Actions taken:** + +- **Issue #265** (Milestone Review): Decided Option A — release candidate, v1.5.0 minor bump (2 additive user-facing enhancements #259, #260; no breaking changes; CI green). Applied `release-candidate` label, removed `pending-review`, commented decision on issue. Issue auto-closed by `milestone-blog.yml`. +- **Issue #266** (Rename `_clearMutex → _dbMutex`): Delegated to Sam. Sam created branch `squad/266-rename-clear-mutex-to-db-mutex`, renamed field across 7 sites in `src/AppHost/MongoDbResourceBuilderExtensions.cs` (1 declaration + 6 usage sites + 1 comment), ran pre-push gates (build 0 errors, Architecture.Tests 15/15, Domain.Tests 42/42, Integration.Tests 12/12), opened PR #267 targeting `dev`. +- Identified 2 pre-existing CI failures; filed Issue #268 (`squad-mark-released.yml` fails — `GITHUB_TOKEN` lacks `project` scope for GraphQL) and Issue #269 (Blog→README Sync fails — direct push to `main` blocked by branch protection). Both labeled `squad:boromir,bug`. + +**Board state after:** Issue #265 closed, PR #267 open targeting `dev` (awaiting merge), Issues #268 and #269 queued for Boromir. + +### 2026-05-08 — CI Fix Sprint + +**Session issues closed:** + +- **#266** — rename `_clearMutex` → `_dbMutex` in `MongoDbResourceBuilderExtensions.cs` (PR #267, squash-merged) +- **#268** — `squad-mark-released.yml` GraphQL permission error; added pre-flight `GH_PROJECT_TOKEN` validation, fixed `permissions: contents: read`, pinned `actions/github-script@v7` (PR #271, squash-merged) +- **#269** — `blog-readme-sync.yml` direct push to `main` blocked by branch protection; changed to `git push origin HEAD:dev` (PR #270, squash-merged) + +**Notes:** +- Board clear at end of session. No open squad issues or PRs. +- `GH_PROJECT_TOKEN` secret must be set manually in repo Settings → Secrets with a PAT scoped to `project` for squad-mark-released to work. + diff --git a/.squad/decisions.md b/.squad/decisions.md index 19a2253b..c6a79ef8 100644 --- a/.squad/decisions.md +++ b/.squad/decisions.md @@ -1948,3 +1948,130 @@ This model choice supersedes the Layer 0 defaults in all squad sessions going fo - Should this decision apply retroactively to existing tests in the codebase? (Not in scope for this decision; can be a future refactoring sprint.) - Should PR reviews include explicit checks for TDD violations? (Already covered by Aragorn's PR gate; this formalizes the standard.) - If other squad members would benefit from GPT-5.4 overrides, document those decisions separately with similar rationale. + +--- + +## Sprint 18 Release Decisions + +### AppHost.Tests CI Hang Fix — Parallel Collection Serialization + +**Date:** 2025-07-25 +**Author:** Aragorn (Lead Developer) +**Branch:** squad/247-mongo-clear-command-tests +**PR:** #251 + +#### Problem + +`AppHost.Tests` was hanging in CI (PR #251) while all other test jobs were green. Root cause: `xunit.runner.json` had `parallelizeTestCollections: true`. The assembly contains two xUnit collections that each boot a full Aspire host (with DCP + Docker MongoDB). When both collections started simultaneously, they competed for DCP resources, causing `App.StartAsync()` to hang indefinitely. + +#### Fix + +Changed `tests/AppHost.Tests/xunit.runner.json`: + +```diff +- "parallelizeTestCollections": true ++ "parallelizeTestCollections": false +``` + +This serializes xUnit collections, allowing only one Aspire host to start at a time. No Docker volume conflicts, no DCP contention, no hang. + +#### Rationale + +Minimum-correct change: one line, zero code changes, zero regression risk. Sequential execution adds ~5-10 minutes over prior parallel execution — well within the 45-minute CI budget. + +--- + +### PR Review Outcomes — #262 and #257 + +**Author:** Aragorn (Lead Developer) +**Date:** 2026-05-08 + +#### PR #262 — APPROVED and squash-merged ✅ + +**Branch:** `squad/259-extract-withcleardatabasecommand` +**Closes:** #259 +**Author:** Sam (Backend/.NET) + +Sam's refactor cleanly extracts the inline `WithCommand` clear-data block from `AppHost.cs` into `MongoDbResourceBuilderExtensions`. All checklist items passed. CI green. + +Two Copilot inline comments flagged for follow-up (not blocking current single-instance project): + +1. **Static SemaphoreSlim** — `_clearMutex` is `static readonly` at class level, violating the extension's reusability contract. Harmless in this project but should be fixed before a second MongoDB resource is added. +2. **Hard-coded database name in UX strings** — Parameter drives logic but UI strings still hard-code "myblog". + +**Decision:** Both are legitimate design defects but do not affect current behavior. A follow-up issue should be raised. + +#### PR #257 — APPROVED and squash-merged ✅ + +**Branch:** `squad/256-fix-squad-mark-released-token` +**Closes:** #256 +**Author:** mpaulosky + +Correct fix: `secrets.GITHUB_TOKEN` lacks Projects V2 GraphQL mutation rights; `secrets.GH_PROJECT_TOKEN` (a PAT with project scope) is the correct credential. No hardcoded secrets. No `.squad/` files. + +Branch was behind dev after PR #262 landed. Required manual: `git fetch → git merge origin/dev → git push` before merge. **Lesson:** Merge concurrent PRs in strict priority order and update downstream branches before attempting merge. + +--- + +### Issue #249 AppHost Clear Hardening — Implementation Choices + +**Date:** 2026-05-08 +**Author:** Boromir + +Issue #249 asks for three resilience properties on the `clear-myblog-data` operator action. All are AppHost/runtime concerns. + +#### AC1: UpdateState gates only on mongo health + +**Decision:** The existing `UpdateState` lambda was already correct. Added an explicit comment rather than a code change. + +**Rationale:** `UpdateState` receives a snapshot of the resource the command belongs to (`mongodb`). Checking the `web` resource's state from here would couple the clear command's availability to the liveness of the application layer—not a valid reason to disable a DBA operator action. + +#### AC2: Single-run protection via SemaphoreSlim(1,1) + WaitAsync(0) + +**Decision:** Declared `var clearMutex = new SemaphoreSlim(1, 1)` in top-level statements scope and applied `WaitAsync(0)` (non-blocking try-acquire) at the top of the `executeCommand` lambda. Failure to acquire returns `{ Success = false, Message = "..." }` immediately. The semaphore is released in `finally`. + +**Rationale:** + +- `WaitAsync(0)` is the idiomatic .NET non-blocking semaphore try-acquire +- `finally` guarantees release even on early-return paths, preventing permanent lock-out +- Top-level scope is appropriate: the `clearMutex` is a process-lifetime singleton and protects a single resource + +#### AC3: Best-effort per-collection via per-collection try/catch + +**Decision:** Wrapped each `DeleteManyAsync` call in its own `catch` block for +`Exception ex when (ex is not OperationCanceledException)`. Caught exceptions are logged at Warning +level, appended to a warnings list, and do NOT halt the loop. The final result message appends +`⚠️ {N} collection(s) had errors: ...` when warnings exist. `OperationCanceledException` is +intentionally excluded to allow operator-initiated cancellation to propagate normally. + +**Rationale:** Issue #249 AC3 states: *"the action continues remaining collections and returns warnings plus partial-progress results."* This implementation satisfies that literally. + +#### Gimli Follow-up (AC4) + +Tests should cover: (1) UpdateState returns Enabled with mongo Healthy regardless of web state; (2) Two concurrent handler invocations—exactly one succeeds, other returns failure; (3) Simulate a handler where the second collection throws—assert first and third collections appear in results, second in warnings, Success = true overall. + +--- + +### Sprint 18 Release PR #272 Opened + +**Date:** 2026-05-08 +**Author:** Boromir (DevOps) + +Release PR **#272** has been opened to promote `dev` → `main` for Sprint 18. + +**PR:** [#272 — [RELEASE] Promote dev to main — Sprint 18](https://github.com/mpaulosky/MyBlog/pull/272) +**Branch:** `dev` → `main` +**Commits ahead:** 55 +**Sprint 18 PRs included:** #262, #263, #264, #267, #270, #271 + +**CI status at PR open:** + +- ✅ Squad CI (`ci.yml`) — green on latest `dev` commit +- ⚠️ Test Suite — 1 flaky failure: `SeedMyBlogData Concurrent Invocations` (timing race in test harness; not a production regression). Squad CI is the authoritative gate. + +**Next steps:** + +1. Aragorn reviews scope and approves +2. PR CI must pass before merge +3. Merge to `main` via squash merge +4. Tag `main` with appropriate `vX.Y.Z` after CI green diff --git a/.squad/decisions/decisions.md b/.squad/decisions/decisions.md index 8d68b1c5..274a105f 100644 --- a/.squad/decisions/decisions.md +++ b/.squad/decisions/decisions.md @@ -2268,3 +2268,66 @@ Add a pre-commit git hook (`.github/hooks/pre-commit`) that runs `markdownlint-c - Contributors must run `npm install` to get the linter; the hook warns them if they haven't. --- + +--- + +# Decision: GitHub Projects V2 requires a classic PAT with `project` scope + +**Date:** 2026-05-08 +**Author:** Boromir (DevOps Engineer) +**Issue:** #268 +**PR:** #271 (squash-merged) +**Status:** ✅ Implemented + +## Context + +The `squad-mark-released.yml` workflow uses `actions/github-script` to call the GitHub Projects V2 GraphQL API. The previous `permissions: repository-projects: write` block applied only to `GITHUB_TOKEN` and was ineffective for Projects V2 mutations. + +## Decision + +1. **`GH_PROJECT_TOKEN` secret is required** for any workflow that calls the GitHub Projects V2 GraphQL API. The default `GITHUB_TOKEN` cannot be used — it is not an integration token with project scope. + +2. **Required PAT scopes:** + - Classic PAT: `project` OAuth scope + - Fine-grained PAT: "Projects" → read + write + +3. **Workflow `permissions` block** should be `contents: read` (minimum) in workflows that rely solely on a custom PAT for external API access. + +4. **Pre-flight validation** — any workflow using `GH_PROJECT_TOKEN` must include a validation step that checks the secret is set and fails early with an actionable error message if it is missing. + +## Setup + +To configure the secret: +1. Create a classic PAT at https://github.com/settings/tokens with `project` scope +2. Add it as repository secret: Settings → Secrets and variables → Actions → `GH_PROJECT_TOKEN` + +--- + +# Decision: Blog README Sync pushes to `dev`, not `main` + +**Date:** 2026-05-08 +**Author:** Boromir (DevOps Engineer) +**Issue:** #269 +**PR:** #270 (squash-merged) +**Status:** ✅ Implemented + +## Context + +The `blog-readme-sync.yml` workflow reads `docs/blog/index.md` from `main` and writes an updated `README.md`. It previously pushed directly back to `main`, which violates branch protection rules (direct pushes blocked, PR + "Build Solution" check required). + +## Decision + +The workflow's "Commit updated README" step now uses `git push origin HEAD:dev` instead of `git push`. README updates flow into `dev` and are released to `main` via the normal dev→main PR/release cycle. + +## Rationale + +- No new secrets or PAT permissions required. +- Consistent with the project's branch flow: `squad/*` → `dev` → `main`. +- `README.md` on `main` is slightly behind `dev` until the next release, which is acceptable — it reflects the released state. + +## Alternatives Considered + +- **Option A (PAT bypass):** Create a PAT with bypass permission. Rejected — adds secret management overhead and bypasses protection intentionally. +- **Option B (PR from workflow):** Have the workflow open a PR to main. Rejected — requires `pull-requests: write`, adds noise, and needs the "Build Solution" check to pass before merge. +- **Option C (push to dev) ← CHOSEN:** Simple one-line fix, no new permissions. + diff --git a/tests/AppHost.Tests/MongoClearDataIntegrationTests.cs b/tests/AppHost.Tests/MongoClearDataIntegrationTests.cs index 35d7c64f..5038f254 100644 --- a/tests/AppHost.Tests/MongoClearDataIntegrationTests.cs +++ b/tests/AppHost.Tests/MongoClearDataIntegrationTests.cs @@ -141,9 +141,26 @@ public async Task ClearMyBlogData_Concurrent_Invocations_Allow_Only_One_Run() var annotation = GetAnnotation(); - // Act - var firstTask = annotation.ExecuteCommand(MakeContext()); - var secondTask = annotation.ExecuteCommand(MakeContext()); + // Act — dispatch both calls to thread-pool workers and open the gate at the same + // moment so they race to acquire _dbMutex. Without this the async lambda may run + // entirely synchronously (fast local MongoDB) and release the semaphore before the + // second call even starts, causing both to succeed (flake). + var ct = TestContext.Current.CancellationToken; + using var startGate = new SemaphoreSlim(0, 2); + + var firstTask = Task.Run(async () => + { + await startGate.WaitAsync(ct); + return await annotation.ExecuteCommand(MakeContext()); + }, ct); + + var secondTask = Task.Run(async () => + { + await startGate.WaitAsync(ct); + return await annotation.ExecuteCommand(MakeContext()); + }, ct); + + startGate.Release(2); // open the gate — both workers race for _dbMutex var results = await Task.WhenAll(firstTask, secondTask); // Assert diff --git a/tests/AppHost.Tests/MongoSeedDataIntegrationTests.cs b/tests/AppHost.Tests/MongoSeedDataIntegrationTests.cs index f9add810..c1bbcc3c 100644 --- a/tests/AppHost.Tests/MongoSeedDataIntegrationTests.cs +++ b/tests/AppHost.Tests/MongoSeedDataIntegrationTests.cs @@ -81,9 +81,26 @@ public async Task SeedMyBlogData_Concurrent_Invocations_Allow_Only_One_Run() var annotation = GetAnnotation(); - // Act — fire two concurrent seed operations - var firstTask = annotation.ExecuteCommand(MakeContext()); - var secondTask = annotation.ExecuteCommand(MakeContext()); + // Act — dispatch both calls to thread-pool workers and open the gate at the same + // moment so they race to acquire _dbMutex. Without this the async lambda may run + // entirely synchronously (fast local MongoDB) and release the semaphore before the + // second call even starts, causing both to succeed (flake). + var ct = TestContext.Current.CancellationToken; + using var startGate = new SemaphoreSlim(0, 2); + + var firstTask = Task.Run(async () => + { + await startGate.WaitAsync(ct); + return await annotation.ExecuteCommand(MakeContext()); + }, ct); + + var secondTask = Task.Run(async () => + { + await startGate.WaitAsync(ct); + return await annotation.ExecuteCommand(MakeContext()); + }, ct); + + startGate.Release(2); // open the gate — both workers race for _dbMutex var results = await Task.WhenAll(firstTask, secondTask); // Assert diff --git a/tests/AppHost.Tests/MongoShowStatsIntegrationTests.cs b/tests/AppHost.Tests/MongoShowStatsIntegrationTests.cs index 92cab2e6..49920101 100644 --- a/tests/AppHost.Tests/MongoShowStatsIntegrationTests.cs +++ b/tests/AppHost.Tests/MongoShowStatsIntegrationTests.cs @@ -87,9 +87,26 @@ public async Task ShowMyBlogStats_Concurrent_Invocations_Allow_Only_One_Run() var annotation = GetAnnotation(); - // Act — fire two concurrent stats operations - var firstTask = annotation.ExecuteCommand(MakeContext()); - var secondTask = annotation.ExecuteCommand(MakeContext()); + // Act — dispatch both calls to thread-pool workers and open the gate at the same + // moment so they race to acquire _dbMutex. Without this the async lambda may run + // entirely synchronously (fast local MongoDB) and release the semaphore before the + // second call even starts, causing both to succeed (flake). + var ct = TestContext.Current.CancellationToken; + using var startGate = new SemaphoreSlim(0, 2); + + var firstTask = Task.Run(async () => + { + await startGate.WaitAsync(ct); + return await annotation.ExecuteCommand(MakeContext()); + }, ct); + + var secondTask = Task.Run(async () => + { + await startGate.WaitAsync(ct); + return await annotation.ExecuteCommand(MakeContext()); + }, ct); + + startGate.Release(2); // open the gate — both workers race for _dbMutex var results = await Task.WhenAll(firstTask, secondTask); // Assert