Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .squad/agents/ralph/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment on lines +138 to +158

**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.

127 changes: 127 additions & 0 deletions .squad/decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +1952 to +1960
**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
63 changes: 63 additions & 0 deletions .squad/decisions/decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

---

---

Comment on lines +2272 to +2273
# 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.
Comment on lines +2274 to +2284

## 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.

23 changes: 20 additions & 3 deletions tests/AppHost.Tests/MongoClearDataIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Comment on lines +149 to +162
startGate.Release(2); // open the gate — both workers race for _dbMutex
var results = await Task.WhenAll(firstTask, secondTask);

// Assert
Expand Down
23 changes: 20 additions & 3 deletions tests/AppHost.Tests/MongoSeedDataIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +90 to +103
var results = await Task.WhenAll(firstTask, secondTask);

// Assert
Expand Down
23 changes: 20 additions & 3 deletions tests/AppHost.Tests/MongoShowStatsIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +90 to +109
var results = await Task.WhenAll(firstTask, secondTask);

// Assert
Expand Down
Loading