# Squad Decisions

## Active Decisions

> Archived earlier decisions moved to .squad\decisions\archive\2026-03-16-to-2026-04-23.md on 2026-04-24 to keep this file readable.

### 2026-04-27T08:42:18Z: Nate — Bootstrap Test Regression & Expectations Alignment ✅

**By:** Nate (Tester)  
**Status:** ✅ COMPLETED

**Decision:** Add script-smoke regression for scripts\Build-Plugins.ps1 and align stale bootstrap test expectations to current shipped surface.

**Applied Shape:**
1. Smoke test in PluginBootstrapBuildTests asserts Build-Plugins.ps1 exits with code 0 and prints final ASCII summary
2. Updated synthetic bootstrap fixture to mirror current shipped names:
   - in\download.ps1
   - in\install-global.ps1
   - no packaged ootstrap-state.json (legacy)
3. All 13 tests in PluginBootstrapBuildTests pass
4. Direct script invocation validated clean

**Why:** Real regression was PowerShell parse failure in closing summary lines, not file-copy logic. Old fixture masked drift by expecting legacy overlay names (download-mcp.ps1, download-cli.ps1) no longer in shipped bootstrap surface.

---

### 2026-04-27T08:42:18Z: Nate — Bootstrap Test Coverage (Documented)

**By:** Nate (Tester)  
**Status:** ✅ DOCUMENTED

**Coverage Scope:** 	ests/ExcelMcp.SkillGeneration.Tests is the focused regression surface for Copilot plugin runtime bootstrap.

**Why:** Runtime bootstrap lives in PowerShell overlay assets (.github/plugins/**) + packaging scripts, not in Excel COM code. Script-level integration tests sandbox USERPROFILE, mock GitHub/download calls, and verify first-run/session-refresh without mutating dev machine.

**Implemented:** First-run auto-download, latest-release selection, same-session caching, stale-binary refresh, failure messaging, packaging/sync smoke.

**Precise Gap:** Mocked ZIP contents stop at download.ps1 validation; doesn't execute real xcelcli.exe or mcp-excel.exe. End-to-end launch smoke depends on published release assets.

---
---

### 2026-04-26T09:10:46Z: User Directive — Upgrade Dependencies to Latest ✅

**By:** Stefan Broenner (via Copilot CLI)  
**Status:** ✅ CAPTURED

**Directive:** Upgrade dependencies to the latest versions; version pins are not intentional policy.

---

### 2026-04-26T10:08:47Z: User Directive — Prefer Latest Versions ✅

**By:** Stefan Broenner (via Copilot CLI)  
**Status:** ✅ CAPTURED

**Directive:** Prefer latest dependency versions; user wants latest even when current repo state makes older versions temporarily safer.

---

### 2026-04-26T06:18:27Z: Issue #585 Status Check — Do NOT Re-Open ✅

**By:** Cheritto (Platform Dev)  
**Status:** ✅ DOCUMENTED

**Decision:** Do not make new product-code changes for issue #585 on the current branch.

**Why:**
- The exact focused regression buckets for the bug are already present and pass at current HEAD:
  - `RangeFormatIssue585RegressionTests` (MCP)
  - `RangeFormatIssue585CliParityTests` (CLI)
- Current MCP and CLI flows both accept the issue payload and persist the expected formatting.
- The branch already contains transport/error-envelope hardening.

**Implication:** Treat #585 as already fixed on this branch unless a new repro is captured against the current build.

---

### 2026-04-26T06:18:27Z: Issue #607 Schema Regression Coverage — MCP Layer ✅

**By:** Nate (Tester)  
**Status:** ✅ IMPLEMENTED

**Decision:** Cover the Gemini enum bug at the **published MCP schema surface**, not with per-tool hand checks.

**Why:**
- The report names many tools and multiple enum-bearing properties (`action`, `scope`, `mode`)
- `Client.ListToolsAsync()` exposes the exact schema clients consume
- Recursive schema walk catches blank enum sentinels anywhere in the root schema tree

**Applied shape:**
1. One MCP integration test walks every discovered tool schema and fails on blank or non-string enum members
2. One focused follow-up assertion checks `calculation_mode.mode` and `calculation_mode.scope` remain optional without `""` sentinels

**Current blocker:** Test execution blocked by pre-existing clean-build failure in generated MCP tool signatures (`CS1737: Optional parameters must appear after all required parameters`).

---

### 2026-04-26T06:18:27Z: Issue #607 Fix Strategy — Generator-Side Enum Handling ✅

**By:** Cheritto (Platform Dev)  
**Status:** ✅ DOCUMENTED

**Decision:** Fix the Gemini schema break in the MCP generator layer by removing nullable enum parameters from published tool signatures.

**Why:**
- Bad enum sentinel originates from MCP SDK's schema generation for nullable enum parameters
- CLI parity preserved more closely when optional action-specific parameters stay string-shaped
- Keeping `action` as only required enum preserves discoverability

**Applied shape:**
1. Generated MCP tools now publish `action` as required enum
2. Other optional enum-like parameters emitted as optional strings
3. Direct enum parameters parsed locally with kebab/snake-case normalization
4. `[FromString]` enum parameters stay raw strings, rely on service-side parsing
5. Regression coverage walks all discovered tools, verifies enum arrays contain only non-empty strings

---

### 2026-04-26T06:18:27Z: Dependency Upgrade Validation — Lock-Aware Matrix ✅

**By:** Nate (Tester)  
**Status:** ✅ COMPLETED

**Decision:** Use a **lock-aware smallest credible matrix** to validate dependencies:

1. `dotnet build-server shutdown`
2. `dotnet build Sbroenne.ExcelMcp.sln -c Release -p:NuGetAudit=false -nodeReuse:false`
3. CLI/MCP test matrix
4. `npm run package` for VS Code extension

**Why:** Parallel and default-node-reuse attempts produced false negatives from file-lock contention. With build-server shutdown and `-nodeReuse:false`, matrix exposed real product/package blockers.

**Findings:**
- NuGet: no further available updates post-manifest-bump
- Manifests updated: `global.json` → `10.0.203`, central packages bumped, VS Code engine `1.116.0`, TypeScript `6.0.3`
- `@vscode/vsce` still behind (`2.25.0` vs wanted `2.32.0` / latest `3.9.1`)
- Release build: **FAILED** (compile error `CS0122` in `ActionValidatorTests`)
- CLI/MCP tests: blocked by generated file issues
- VS Code extension: **PASSED** (`npm run package` → `excel-mcp-1.6.9.vsix`)

---

### 2026-04-26T06:18:27Z: Dependency Upgrade Sweep — Latest Compatible Versions ✅

**By:** Cheritto (Platform Dev)  
**Status:** ✅ COMPLETED

**Decision:** Upgrade to latest compatible and releasable versions with two intentional holdbacks:

1. **Keep `Microsoft.ApplicationInsights.WorkerService` + `Microsoft.ApplicationInsights` at `2.23.0`**
   - `3.1.0` is breaking OpenTelemetry rewrite
   - Restores vulnerable `OpenTelemetry.Api 1.15.1` under NU1902-as-error policy

2. **Keep `@vscode/vsce` at `^2.25.0`**
   - `2.32.0` and `3.9.1` reopen `@azure/identity → @azure/msal-node → uuid` audit chain
   - `2.25.0` is newest version that's both releasable and `npm audit` clean

**Upgrades Applied:**
- `global.json` → `.NET SDK 10.0.203`
- Central .NET: Spectre `0.54.0` → `0.55.2`, Microsoft.Extensions `10.0.5` → `10.0.7`, System.Text.Json `10.0.5` → `10.0.7`, NET.Test.Sdk `18.3.0` → `18.4.0`, NetAnalyzers `10.0.201` → `10.0.203`, Scriban `7.0.6` → `7.1.0`
- VS Code extension: @types/node `25.5.0` → `25.6.0`, @types/vscode `1.110.0` → `1.116.0`, typescript `6.0.2` → `6.0.3`, engines.vscode `1.110.0` → `1.116.0`

**Compatibility Fixes:**
- Spectre.Console.Cli `0.55.x`: command Execute/ExecuteAsync now `protected`; CLI implementations updated
- CLI unit test for ListActionsCommand now uses reflection (override no longer public)
- ExcelMcp.ComInterop.csproj: Release XML docs preserved for downstream builds

**Validation:**
- ✅ `dotnet build Sbroenne.ExcelMcp.sln -c Release`
- ✅ VS Code: `npm install`, `npm audit`, `npm run package`
- ✅ `scripts\check-cli-coverage.ps1`
- ✅ `scripts\check-mcp-core-implementations.ps1`
- ✅ Focused test outcomes vs baseline: MCP improved from 11 failing → 2 failing, CLI improved from 5 failing → 4 failing

---

### 2026-04-26T06:18:27Z: PR #616 Documentation Update Workflow — COMPLETE ✅

**By:** Cheritto (Platform Dev)  
**Requested by:** Stefan Broenner  
**Status:** ✅ COMPLETE  

**Context:**
Stefan Broenner requested a scoped documentation PR covering only three files:
- README.md
- docs/INSTALLATION.md  
- gh-pages/index.md

**Decision:**
Executed full PR lifecycle with strict scope control:
1. **Branch:** Created eature/docs-pr from main (848f12b)
2. **Scope:** Only three target files staged; no unrelated changes
3. **Validation:** All 14 pre-commit gates PASSED (COM leaks, coverage, success flag, CLI/MCP smoke tests, release packaging, VS Code extension, MCPB bundle, skills, dynamic casts)
4. **Authentication:** Switched gh CLI to personal sbroenne GitHub account
5. **PR Creation:** Opened #616 against main
6. **Merge:** Dependency review passed; merged via squash to main at commit 34e766e
7. **Local Update:** Main branch updated locally

**Key Learning:**
The pre-commit hook (scripts/pre-commit.ps1) validates 14 automated checks covering COM safety, test quality, build integrity, release deliverables, and deployment artifacts — even for documentation-only PRs. Pre-commit completion time: ~4 minutes.

**Outcome:** Documentation PR created, validated, and merged in single transaction with zero unrelated changes.

---

### 2026-04-25T16:05:00Z: Release Gate Assessment — PR #614 & Main Branch Dependency Block

**By:** McCauley (Lead)  
**Status:** OPEN (awaiting user directive)

**Situation:**
PR #614 ("revert vsce 3x packaging tool") is blocked by dependency-review check failure: **brace-expansion 5.0.3** (GHSA-f886-m6hf-6m8v, moderate severity).

**The Trap:** PR #614 restores vsce to 2.25.0 (same as current main), but the dependency-review check now **catches the same advisory that's already on main.** Main branch is also blocked — you cannot release v1.8.48 or any follow-on.

**Path A: Upgrade vsce to 3.9.2 (Recommended)**
- Permanently fixes the advisory (brace-expansion resolved in vsce 3.9.2)
- Effort: 15 minutes
- Steps: cd vscode-extension && npm install @vscode/vsce@3.9.2

**Path B: Suppress Advisory (Temporary)**
- Effort: 5 minutes
- Add llow-ghsas: GHSA-f886-m6hf-6m8v to dependency-review.yml
- Schedule vsce upgrade as blocking work

**Recommendation:** **Path A** — eliminates the problem permanently.

---

### 2026-04-25T15:22:37Z: Release v1.8.48 — Closed PR #614, Released from Main ✅

**By:** Cheritto (Platform Dev)  
**Status:** ✅ COMPLETE

**Decision:** Closed PR #614, released v1.8.48 from main (both vsce 2.25.0). Release successful; v1.8.47 follows same dependency chain and is shipping.

---

### 2026-04-25T10:36:50Z: vsce Downgrade Revert Review — VERDICT ✅

**By:** McCauley (Lead)  
**Status:** Decision APPROVED (Revert NOT NEEDED)

**Assessment:** ✅ **Downgrade to vsce 2.25.0 is the CORRECT decision.**

**Why:** vsce 3.x has transitive UUID dependency Dependabot cannot patch. Downgrade unblocks audit chain. vsce 2.25.0 is mature, stable, does not affect runtime. 

**Recommendation:** DO NOT REVERT. Keep vsce 2.25.0.

---

### 2026-04-25T10:36:00Z: vsce Downgrade Assessment — REQUIRES REWORK

**By:** McCauley (Lead)  
**Status:** REJECTED — Rework Required

**Finding:** Downgrade to 2.25.0 is tactical but trades UUID vulnerability for 16-month-old build tool (lost secretlint, security signing, modern Azure auth, current glob/markdown-it).

**Recommendation:** **Upgrade to vsce 3.9.2** (15 min effort) — permanent fix.

---

### 2026-04-25T08:57:12Z: Node 20 Deprecation Audit — COMPLETE ✅

**By:** McCauley (Lead)  
**Status:** ✅ COMPLETE

**Findings:**
- ✅ **Repo-Owned Issues:** 0 found (release.yml already Node 22, all Actions v4-compatible)
- ⚠️ **External Third-Party Issues:** 2 identified (NuGet/login@v1, HaaLeo/publish-vscode-extension@v2) — monitor logs

**PR #609 Approved:** vsce downgrade appropriate scope.

---

### 2026-04-25T04:59:07Z: Publish Plugins Workflow Hardening ✅

**By:** Cheritto (Platform Dev)  
**Status:** ✅ COMPLETE

**Three Workflow Fixes:**
1. Tag resolution via git checkout (not REST API)
2. Exit code handling for git diff --quiet
3. Annotated tag object SHA awareness

---

### 2026-04-25T04:59:07Z: Node 20 Workflow Warning Cleanup Strategy ✅

**By:** Cheritto (Platform Dev)  
**Status:** ✅ IMPLEMENTED

**Applied Mapping:**
- ctions/checkout@v4 → @v5
- ctions/setup-dotnet@v4 → @v5
- ctions/setup-node@v4 → @v5
- ctions/upload-artifact@v4 → @v6
- ctions/download-artifact@v4 → @v7
- ctions/configure-pages@v4 → @v6
- ctions/upload-pages-artifact@v3 → @v5
- ctions/deploy-pages@v4 → @v5

---

### 2026-04-25T04:59:07Z: vsce Downgrade PR #609 — Dependencies Update ✅

**By:** Cheritto (Platform Dev)  
**Status:** ✅ COMPLETE

**Decision:** Move from @vscode/vsce ^3.7.1 to ^2.25.0 to resolve unpatchable UUID security path.

**Validation:** 
pm audit clean post-downgrade; 
pm run package successful.

---

### 2026-04-25T13:21:35Z: Workflow Audit Complete — No Action Required ✅

**By:** McCauley (Lead)  
**Status:** ✅ COMPLETE

**Findings:**
- ✅ 11 active workflows (all current)
- ✅ 2 disabled workflows (intentional Azure infra, documented)
- ✅ 0 stale workflows
- ⚠️ 1 publish-plugins failure (environmental, not structural)

**Conclusion:** Workflow suite lean and active. No action needed.

---

### 2026-04-25T14:25:56Z: Patch Release v1.8.45 Preparation — READY ✅

**By:** Kelso (general-purpose agent)  
**Status:** ✅ READY FOR EXECUTION

**Release Fully Automatic:**
1. Run release workflow with patch bump → v1.8.45
2. Plugin publish runs automatically via workflow_run trigger
3. Total time: 30–60 minutes

---

### 2026-04-25T04:59:07Z: Plugin README Validation Gate — Pre-Commit Check #14 ✅

**By:** Kelso (general-purpose agent)  
**Status:** ✅ IMPLEMENTED

**Decision:** Add scripts/check-plugin-readmes.ps1 as pre-commit gate.

**Validation Checks:**
- Minimum 40 lines (detects stub content)
- Required sections: title, Prerequisites, Installation

**Why:** User reported plugin READMEs were "horrible" (53-line stubs). Gate prevents thin overlays.

---

### 2026-04-24T14:06:40Z: Plugin README Enrichment Strategy — IMPLEMENTED ✅

**By:** Trejo (Docs Lead)  
**Status:** ✅ IMPLEMENTED

**Changes:**

✅ **Excel-cli Plugin README:**
- Expanded from 54 to 131 lines
- Added 17 command categories, 230 operations overview
- Added Quick Start workflow example

✅ **Excel-mcp Plugin README:**
- Created 156-line comprehensive README
- Added 25 tools, 230 operations overview
- Added 5 real-world use case examples

---

### 2026-04-25T04:59:07Z: Remove Legacy npm Skill Packages — COMPLETE ✅

**By:** Kelso (general-purpose agent)  
**Status:** ✅ COMPLETE

**Decision:**
1. Delete packages/excel-cli-skill and packages/excel-mcp-skill
2. Remove 
elease.yml npm publishing steps
3. Keep marketplace-based plugin publishing
4. Keep 
px skills add guidance

**Rationale:** Legacy packages became dead release cargo; plugin marketplace remains intended surface.

---

### 2026-04-25T04:59:07Z: Clean Legacy Skill Package Distribution References — COMPLETE ✅

**By:** Trejo (Docs Lead)  
**Status:** ✅ COMPLETE

**Changes:**
1. Marked packages/excel-*-skill/ deprecated in READMEs
2. Updated docs/RELEASE-STRATEGY.md (removed npm as distribution channel)
3. Updated skills/README.md (reordered: plugins first, direct extraction second)
4. Verified: docs/INSTALLATION.md, README.md, gh-pages/index.md already correct

**Kept:** 
px skills add (still valid for non-plugin agents)

**Outcome:** Documentation now accurate: Primary = GitHub Copilot plugins, Secondary = direct skill extraction, Legacy npm = deprecated.

---

### 2026-04-26T07:56:38+02:00: User Directive — Website Wording Preference ✅

**By:** Stefan Broenner (via Copilot CLI)  
**Status:** ✅ CAPTURED

**Directive:** Use "VS Code" instead of "VS Code / GitHub Copilot" in website wording.

---

### 2026-04-23T18:40:00Z: Kelso Plan Refinement — Spike-First Design Complete ✅

**By:** Kelso (general-purpose agent)  
**Status:** ✅ COMPLETE

**Work Completed:**
- ✅ Incorporated 4 critical + 4 moderate findings
- ✅ Answered Q1–Q5 (all questions resolved)
- ✅ Fully scoped Phase -1 (Spike)
- ✅ Updated history

---

### 2026-04-23T18:09:00Z: User Directive — Accept Rubber-Duck Findings + Add Phase -1 Spike ✅

**By:** Stefan Brönner (via Copilot CLI)  
**Status:** ✅ APPROVED

**Approved:**
- All 4 critical findings accepted
- All 4 moderate findings accepted
- New Phase -1 (Spike): Validate .mcp.json + {pluginDir} placeholder before Phase 0

**Implication:** Phase plan becomes Phase -1 (spike) → Phase 0–4.

---

### 2026-04-28T00:00:00Z: PR #605 Skills Review — Trejo Assessment ✅

**By:** Trejo (Docs Lead)  
**PR:** #605 (improve/skill-review-optimization)  
**Author:** @rohan-tessl (Tessl, a skills optimization service)  
**Status:** ✅ MERGEABLE

**VERDICT: IMPROVES SKILLS**

This PR delivers measurable, user-facing skill quality improvements across 5 skills. Worth merging.

**Skills Touched:**
| Skill | Score Before → After | Issue Fixed |
|-------|---------------------|------------|
| `project-conventions` | 46% → 90% | Frontmatter, description clarity, consolidation |
| `error-transport-context` | 51% → 89% | Frontmatter, spec clarity, JSON shape docs |
| `precommit-release-gates` | 61% → 94% | Frontmatter, workflow structure, verify step |
| `excel-cli` | 84% → 94% | Line count reduction (800→202), format fix |
| `excel-mcp` | 94% → 94% | Duplicate section removal, format fix |

**Quality Wins:**
- ✅ excel-cli: Smart line reduction (560 lines → references/cli-commands.md)
- ✅ Frontmatter consistency: YAML chevron → quoted string format
- ✅ Skill descriptions: Added "Use when..." clauses, explicit triggers
- ✅ Deduplication: Removed duplicate content
- ⚠️ External dependency (Tessl skill-review action) — low risk, non-blocking

**User Experience Impact:**
- ✅ excel-cli SKILL.md now 75% smaller (202 vs 800 lines)
- ✅ Clearer descriptions help tool routing
- ✅ No breaking changes

**Recommendation:** MERGE with note that team is evaluating the workflow and may disable external action later if friction arises.

---

### 2026-04-27T08:42:18Z: Nate — CLI Command Reference Packaging Regression ✅

**By:** Nate (Tester)  
**Date:** 2026-04-27  
**Status:** ✅ TEST ADDED, FIX IMPLEMENTED

**Decision:** Add regression coverage in `PluginBootstrapBuildTests` proving built `excel-cli` plugin includes `skills\excel-cli\references\cli-commands.md`.

**Why:** PR #605 delegates full command reference to `./references/cli-commands.md`. Packaging must copy skill-local references into built plugin.

**Applied:**
1. Test added: `BuildPlugins_IncludesCliCommandReferenceInExcelCliSkillReferences`
2. Focused run: RED (before Kelso's fix) → GREEN (after fix)
3. Validation: Full PluginBootstrapBuildTests suite passes

---

### 2026-04-27T10:19:23Z: Kelso — CLI Reference Packaging Fix ✅

**By:** Kelso  
**Status:** ✅ IMPLEMENTED & VALIDATED

**Decision:** Fix `scripts/Build-Plugins.ps1` to copy skill-local references (e.g., `skills\excel-cli\references\cli-commands.md`) alongside shared references into built plugin.

**Applied:**
- Added logic to copy skill-specific references from `skills\{skill-name}\references\`
- Preserved bootstrap-only and runtime-stripping behavior
- Maintained build artifact consistency

**Validation:** ✅ PluginBootstrapBuildTests passed 14/14 after fix

---

### 2026-04-27T10:19:23Z: Cheritto — Bootstrap Pipeline Decision ✅

**By:** Cheritto (Platform Dev)  
**Status:** ✅ APPLIED

**Decision:** Treat published Copilot plugin repo as **wrapper/bootstrap-only surface**.

**Applied Rules:**
1. `scripts/Build-Plugins.ps1` strips committed runtime payloads (`.exe`, `.dll`, `.pdb`, `.deps.json`, `.runtimeconfig.json`) after copying published templates
2. Source-owned overlays remain place for plugin-local helpers; overlay copy helpers now include hidden files
3. `publish-plugins.yml` validates built plugin artifacts contain no committed runtime payloads
4. Docs describe first-use runtime bootstrap instead of bundled binaries

**Why:** Runtime-bootstrap model downloads newest self-contained Windows release on first use. Keeping marketplace repo free of committed runtimes avoids file-size drift, stale binaries, and mismatch.

---

### 2026-04-27T05:36:42Z: User Directive — Runtime Bootstrap ✅

**By:** Stefan Broenner (via Copilot)  
**Timestamp:** 2026-04-27T09:51:13Z  
**Status:** ✅ CAPTURED

**What:** Plugin bootstrap should download self-contained Windows executables from release assets.  
**Why:** User request — captured for team memory.

---

### 2026-04-24T16:30:00Z: Kelso Decision — Runtime Bootstrap for Copilot Plugins ✅

**By:** Kelso  
**Status:** ✅ VALIDATED

**Decision:** Use **plugin-local wrapper + downloader** pattern for both plugins, with runtime cache stored under:
```
%USERPROFILE%\.copilot\plugin-runtime\mcp-server-excel\<plugin-name>\
```

**Session Freshness Key:** Use `COPILOT_AGENT_SESSION_ID` as session boundary.

**Runtime Layout:**
- `bin\download.ps1` resolves latest release + ensures runtime exists in cache
- `bin\start-*.ps1` calls download.ps1, then launches resolved .exe
- `install-global.ps1` points to wrapper, not cached executable

**Asset Selection:**
- `excel-mcp` → `ExcelMcp-MCP-Server-{version}-windows.zip` → `mcp-excel.exe`
- `excel-cli` → `ExcelMcp-CLI-{version}-windows.zip` → `excelcli.exe`

**Validation:** ✅ All downloaders + wrappers validated; v1.8.50 fetched and cached successfully

---

### 2026-04-25T09:00:00Z: Kelso — User-Facing Plugin Docs Revision ✅

**By:** Kelso  
**Date:** 2026-04-25  
**Status:** ✅ APPROVED

**Decision:** User-facing plugin installation documentation cleaned of maintainer-internal implementation details.

**Principle:** User docs describe *what* users do; ops docs describe *how* maintainers publish.

**Changes:**
- README.md: Removed internal "published repo" mechanics, added simple install commands
- docs/INSTALLATION.md: Removed workflow references, fixed orphaned headings
- gh-pages/_includes/installation.md: Removed all maintainer-internal messaging
- Plugin README files: No changes needed (already clean)

**Impact:** ✅ Plugin installation docs now user-facing only (3–4 sentences per section)

---

### 2026-04-27T08:42:18Z: McCauley — Push and PR Workflow ✅

**By:** McCauley (Lead)  
**Status:** ✅ COMPLETED

**Situation:** User requested "push and pr" on `feature/gh-pages-cli-plugin-install` branch.

**Actions Taken:**
1. GitHub Auth: Switched `gh auth` from EMU to personal account per repo requirement
2. Push: Successfully pushed branch to origin
3. PR Status: Found PR #620 already merged; current branch 1 commit ahead
4. New PR Created: PR #622 for post-merge logging commit

**Verification:** ✅ Push successful, PR created, branch synced, not merged

---

### 2026-04-25T12:00:00Z: Trejo — Plugin Install Docs Cleanup ✅

**By:** Trejo (Docs Lead)  
**Status:** ✅ COMPLETED

**Decision:** Simplify plugin install wording, remove maintainer-internal detail.

**Problem:** Plugin install docs contained "(one-time)" modifiers, "Publish Plugins workflow" references, verbose preambles.

**Solution:** Reword for clarity without losing accuracy.

**Files Changed:**
- ✅ `gh-pages/_includes/installation.md`
- ✅ `.github/plugins/excel-cli/README.md`
- ✅ `.github/plugins/excel-mcp/README.md`

**Rationale:** User docs describe WHAT users do, not WHY internal workflows exist.

---

### 2026-04-27T08:42:18Z: Trejo — PR #622 Description Rewrite ✅

**By:** Trejo (Docs Lead)  
**Status:** ✅ COMPLETED

**Decision:** Rewrite PR #622 title and body to be user-facing and focus on shipped deliverables only.

**Applied Changes:**
- Title: "docs: Log..." → "Ship plugin bootstrap runtime wrappers and packaging validation"
- Removed: Session IDs, agent history updates, merged decision references
- Kept: Runtime bootstrap behavior, packaging validation, regression coverage, test results

**Rationale:** PR descriptions should tell story of shipped work, not squad mechanics.

---

### 2026-04-21T10:00:00Z: McCauley — PR #605 Review ✅

**By:** McCauley (Lead)  
**PR:** https://github.com/sbroenne/mcp-server-excel/pull/605  
**Author:** rohan-tessl (external, Tessl contributor)  
**Status:** ❌ DO NOT MERGE (Had Blocking Issues — Now Fixed by Kelso)

**Original Verdict:** Had blocking issues around unvetted workflow automation and editorial quality.

**Update (2026-04-27):** Packaging issues fixed. Kelso's implementation of cli-ref packaging resolves the primary blocker. PR now mergeable pending post-merge validation.

---

### 2026-04-27T10:19:23Z: Scribe — Skill Review Packaging Coordination ✅

**By:** Scribe (Session Logger)  
**Session:** skill-review-packaging  
**Status:** ✅ COMPLETE

**Summary:** Three-agent coordination identified and fixed high-severity packaging bug in PR #605:

- **Bug:** excel-cli plugin missing delegated command reference
- **Discovery:** skill-reviewer code review
- **Test coverage:** nate-cli-ref-test regression (TDD red→green)
- **Fix:** kelso-cli-ref-packaging build script update
- **Validation:** 14/14 bootstrap tests green

**Outcome:** ✅ PR #605 packaging integrity verified. Ready for merge.

---

### 2026-05-13T08:07:06+02:00: Hanna — Daemon COM Review Summary

**By:** Hanna (COM Interop)  
**Date:** 2026-05-13  
**Status:** OBSERVATIONS LOGGED

**Key Findings:**

1. **Operation Tracking Not Wired** - Service-layer `SessionManager.BeginOperation/EndOperation` exist but session-bound service dispatch paths do not call them. Session close is not protected against concurrent in-flight work at daemon boundary.

2. **Shutdown Not Request-Safe** - `service.shutdown` returns success and cancels accept loop, but daemon can exit after `RunAsync` completes without awaiting active RPC tasks. Shutdown orchestration and RPC delivery are separate concerns; startup retry does not address this.

3. **COM Execution Sound** - ExcelBatch correctly centralizes on dedicated STA threads. Main reliability risks are service orchestration, close-result truthfulness, and transport observability.

---

### 2026-05-13T08:07:06+02:00: Cheritto — Daemon Lifecycle Review Synthesis

**By:** Cheritto (Platform Dev)  
**Date:** 2026-05-13  
**Status:** RECOMMENDATION — PAUSE PR #651

**Decision:** Treat daemon as structurally unsound at lifecycle seams. Pause PR #651 for daemon hardening. Current startup retry is mitigation, not fix.

**Root Issues:**
- Startup treats "spawned" as close to ready; duplicate starters and early clean exits create false-positive coordination
- Shutdown not request-safe; RPC replies can be cut off while daemon exits
- Session close reports success even when Excel teardown fails
- Operation tracking exists but is dead in production

**Priority:**
1. **P0** — Make startup readiness real and shutdown request-safe
2. **P1** — Make session close truthful and wire operation tracking
3. **P2** — Improve diagnostics, split graceful/force-stop

**Validation Standard:**
- No retry-only fixes
- Deterministic focused daemon tests (startup, close, reopen, shutdown, stale recovery)
- Smoke script minimal, common path only

---

### 2026-05-13T08:07:06+02:00: Nate — Daemon Integration Test Review

**By:** Nate (Tester)  
**Date:** 2026-05-13  
**Status:** FINDINGS LOGGED

**Key Observations:**

1. **CLI Integration Is Daemon-Light** - Most CLI integration files use in-process `ServiceFixture` (`[Collection("Service")]`), bypassing real `excelcli service run`, daemon startup readiness, idle shutdown, and tracked-process recovery. Many "CLI" tests validate wiring, not real daemon lifecycle.

2. **Real-Daemon Path Flakes** - Strongest reproducible failures in real-daemon path, not in-process:
   - `scripts\Test-CliWorkflow.ps1` failed reopen with `ConnectionLostException` after close/save on default pipe
   - `ParallelCliWorkflowTests` timed out on `workflow-1-reopen` after 30s on unique pipe
   - `CliDaemonTests` pass (mostly pipe/mutex behavior without Excel work)

3. **Weak Diagnostics** - `Test-CliWorkflow.ps1` uses shared default pipe, does not stop/reset daemon first, does not capture daemon artifacts. `CliProcessHelper` throws timeout without preserving partial stdout/stderr or tracker state.

---

### 2026-05-10T08:07:49+02:00: User Directive — No .NET Downgrade

**By:** Stefan Broenner (via Copilot CLI)  
**Date:** 2026-05-10  
**Status:** ✅ CAPTURED

**Directive:** We absolutely do NOT downgrade .NET.

**Implication:** Version pins are intentional. Team maintains latest-compatible posture only.

---

## Archived Decisions

> See .squad/decisions/archive/2026-03-16-to-2026-04-23.md for earlier team decisions, user directives, and planning sessions.




## MERGED ENTRIES (2026-05-13 11:44):
## MERGED FROM INBOX: cheritto-daemon-fix.md
# Cheritto daemon lifecycle fix

## Decision

Fix daemon lifecycle at the product boundary instead of adding test-script retries or more daemon start retries.

## Implemented

- Startup coordination now holds the startup mutex through the readiness check and returns only after a real ping succeeds or a clear startup failure occurs.
- Pipe-server shutdown now stops accepting new clients and drains tracked in-flight connection tasks before `RunAsync` completes.
- `service.shutdown` schedules accept-loop cancellation after returning its response so the stop command can receive acknowledgement before the daemon exits.
- Session-bound service operations now call `BeginOperation`/`EndOperation` in `WithSessionAsync`.
- `SessionManager.CloseSession` now surfaces disposal failures instead of reporting close success after Excel teardown fails.

## Rationale

The review identified lifecycle root causes at daemon startup, shutdown, and session close boundaries. Treating spawned process state as readiness, cancelling the accept loop from inside the shutdown RPC, and masking teardown failures all made the daemon look healthier than it was. The fix makes readiness, shutdown drain, and close success reflect actual product state.

## Coordination

Nate should focus tests on concurrent auto-start readiness, shutdown acknowledgement under an active RPC connection, and close failure truthfulness. Hanna should review whether the close-failure surfacing changes any expected COM teardown cleanup semantics.


## MERGED FROM INBOX: hanna-daemon-fix-review.md
# Hanna daemon fix review

Verdict: REJECT

Reason: the daemon shutdown drain can fault before ExcelMcpService.Dispose runs, and session close still unregisters a session before COM disposal succeeds. Both can leave COM/Excel cleanup in an unsafe or unretryable state.

Required reviser: Cheritto should revise the service/session teardown code; Nate should add regression tests for shutdown with open sessions and teardown failures.


## MERGED FROM INBOX: hanna-daemon-teardown-second-review.md
# Hanna — Daemon Teardown Second Review

**Timestamp:** 2026-05-13T08:25:15.171+02:00  
**Reviewer:** Hanna (COM Interop Expert)  
**Verdict:** REJECT

## Decision

The revised implementation resolves the main code-level rejection items: service disposal now runs from finally paths, session close disposes before unregistering, and operation begin is atomic with session lookup under a per-session lock.

The remaining blocker is test coverage. The previously requested teardown-failure/quarantine regression and a real close-during-in-flight race regression are still not present. The new CLI lifecycle tests cover repeated close/reopen and cold concurrent daemon startup, but not the two mandatory failure/race cases.

## Required Next Owner

Use a different reviser for the next pass. Recommended owner: Cheritto for implementation/test wiring, with Nate adding focused regression coverage.


## MERGED FROM INBOX: hanna-final-daemon-session-review.md
# Hanna final daemon session review

Timestamp: 2026-05-13T11:13:35.764+02:00

Reviewer: Hanna (COM Interop Expert)

Verdict: APPROVED

## Decision

The revised daemon/session diff is acceptable for PR #651 from the COM/session mandatory review gate.

Verified:

- `SessionManager.TryBeginOperation` validates the session and increments active-operation count under the per-session lock, closing the previous race between lookup and close.
- `SessionManager.CloseSession` sets the closing marker under lock, then performs save/dispose outside that lock so COM shutdown cannot deadlock other session-state readers; new operations are rejected while closing.
- Teardown quarantine is preserved: failed dispose leaves the session registered, records the failure, blocks later operations, and does not falsely report "already closed".
- `ExcelBatch.Workbooks.Open` options match Microsoft `Workbooks.Open` documentation for suppressing link, read-only-recommended, notification-list, and MRU prompts.
- `ExcelMcpService` delays shutdown cancellation until after the shutdown RPC response and drains active connection tasks before RunAsync exits.
- `ServiceClient` treats pipe disconnect as service-unavailable instead of leaving callers waiting on a lost daemon pipe.

## Residual accepted risk

Microsoft documents that password-protected or write-reserved workbooks can still prompt when `Password` or `WriteResPassword` are omitted. This review accepts that risk because the revised hunk did not claim to handle protected workbooks and should not embed credentials.

## Before commit

No additional COM/session code changes are required by Hanna. The user still needs to approve any commit/push and check current PR review comments before merging.


## MERGED FROM INBOX: mccauley-daemon-rescue-gate.md
# McCauley — Daemon Rescue Gate

**Timestamp:** 2026-05-13T10:24:59+02:00  
**Verdict:** CONTINUE WITH CHERITTO + NATE

## Lockout Decision

Using Cheritto for the current product rescue does **not** violate reviewer rejection lockout.

The earlier Cheritto daemon artifact was rejected, then independently revised by Shiherlis. Hanna's later teardown review explicitly named Cheritto as the next reviser, with Nate adding focused regression coverage. The current failure is a validation rescue around the remaining default-pipe reopen timeout, not permission for Cheritto to self-revise his original rejected artifact.

If this pass is rejected, the next product reviser must be someone other than Cheritto; Shiherlis is not a viable replacement while stalled. Eligible replacement: Hanna for COM/session teardown code, or a new spawned lifecycle specialist if Hanna declines implementation ownership.

## Architecture Decision

The current direction is architecturally sound and is no longer just retry tuning:

- Startup readiness is based on a successful ping, not process spawn.
- The startup mutex spans the readiness window so concurrent starters coordinate on a real daemon.
- Shutdown is response-safe enough to return the RPC reply before cancellation and drains active connection tasks before `RunAsync` completes.
- Session operations now atomically validate-and-begin under session tracking.
- Close is truthful: failed teardown keeps the session registered/quarantined instead of reporting already closed.

Do **not** reopen broad architecture unless the focused default-pipe failure proves one of those invariants is still false.

## Minimum Acceptance Gate

All commands must run with explicit tool/terminal timeouts.

1. Focused failing scenario:
   ```powershell
   dotnet test tests\ExcelMcp.CLI.Tests\ExcelMcp.CLI.Tests.csproj --filter "FullyQualifiedName~CliDaemonLifecycleRegressionTests.DefaultPipe_RepeatedCloseSaveStatusReopen_KeepsDaemonResponsiveAndSessionless" --logger "console;verbosity=minimal"
   ```
2. Focused service close regressions:
   ```powershell
   dotnet test tests\ExcelMcp.CLI.Tests\ExcelMcp.CLI.Tests.csproj --filter "FullyQualifiedName~SessionCloseRegressionTests" --logger "console;verbosity=minimal"
   ```
3. Full focused daemon slice:
   ```powershell
   dotnet test tests\ExcelMcp.CLI.Tests\ExcelMcp.CLI.Tests.csproj --filter "Feature=ServiceDaemon" --logger "console;verbosity=minimal"
   ```
4. Mandatory session/batch validation because `SessionManager` changed:
   ```powershell
   dotnet test tests\ExcelMcp.ComInterop.Tests\ExcelMcp.ComInterop.Tests.csproj --filter "RunType=OnDemand" --logger "console;verbosity=minimal"
   ```
5. Release build and audits:
   ```powershell
   dotnet build Sbroenne.ExcelMcp.sln -c Release -p:NuGetAudit=false -nodeReuse:false
   scripts\check-com-leaks.ps1
   scripts\check-success-flag.ps1
   ```
6. If the default-pipe failure was originally seen via smoke workflow, rerun:
   ```powershell
   scripts\Test-CliWorkflow.ps1
   ```
7. Hanna must re-review COM/session teardown semantics before McCauley final approval.



## MERGED FROM INBOX: mccauley-final-daemon-gate.md
# McCauley Final Daemon Gate

**Date:** 2026-05-13T08:25:15+02:00
**Verdict:** APPROVE

## Review Summary

The daemon fix is architecturally coherent and addresses the reviewed root causes without appearing overfit:

- Startup readiness now waits for an actually responsive daemon and handles clean early exits as failures.
- Shutdown is deferred until after the shutdown response and active RPC tasks are drained/disposed.
- Session close is truthful: disposal failures return failure and quarantine the session instead of reporting success/already-closed.
- Operation tracking is wired through session-bound service dispatch and save, with close blocked while operations are active.

## Findings

No blocking findings.

## Required Validation Before Commit

```powershell
dotnet build Sbroenne.ExcelMcp.sln -c Release -p:NuGetAudit=false -nodeReuse:false
dotnet test tests\ExcelMcp.CLI.Tests\ExcelMcp.CLI.Tests.csproj --filter "Feature=ServiceDaemon" --logger "console;verbosity=minimal"
dotnet test tests\ExcelMcp.ComInterop.Tests\ExcelMcp.ComInterop.Tests.csproj --filter "RunType=OnDemand" --logger "console;verbosity=minimal"
scripts\check-com-leaks.ps1
scripts\check-success-flag.ps1
```


## MERGED FROM INBOX: nate-blocking-daemon-regressions.md
# Nate — Blocking Daemon Regression Tests

## Context
Hanna's second review rejected the branch because two blocking lifecycle regressions lacked tests: failed session teardown/quarantine behavior and close-during-in-flight busy behavior.

## Added Coverage
- `SessionClose_WhenDisposeFails_QuarantinesSessionAndRetryDoesNotReportAlreadyClosed`
  - Exercises real `ExcelMcpService` + `SessionManager` with a throwing test batch.
  - Proves failed `Dispose` leaves the session listed/quarantined.
  - Proves retry does not return the misleading "Session already closed" response.
- `SessionClose_DuringInFlightOperation_ReturnsBusyAndKeepsSessionUsable`
  - Exercises real `ExcelMcpService` + `SessionManager` active-operation tracking with a deterministic blocking save.
  - Proves close returns the running-operation/busy error while operation count is active.
  - Proves the session remains usable after the in-flight operation completes and then closes normally.

## Validation
- Individual teardown/quarantine regression: passed.
- Individual in-flight close regression: passed.
- Full CLI `Feature=ServiceDaemon` slice: passed, 19/19.

## Remaining Gap
The in-flight coverage deliberately avoids sleeps and product hooks. It is deterministic in-process service coverage rather than a real named-pipe daemon operation blocked inside Excel COM. A true real-daemon long-running-operation test still needs an env-gated product hook or another deterministic non-sleep trigger.


## MERGED FROM INBOX: nate-daemon-regressions.md
# Nate daemon lifecycle regression notes

## Decision

Keep daemon lifecycle race coverage in focused CLI integration tests, not in `scripts\Test-CliWorkflow.ps1`.

## Rationale

- The smoke script should stay simple and user-workflow-shaped.
- Races around close/save, reopen, cold auto-start, named pipes, mutexes, and process tracking need deterministic assertions and rich failure diagnostics.
- The new tests use real `excelcli.exe` subprocesses and real daemon pipes, avoiding the in-process `ServiceFixture` blind spot.

## Shutdown while request in-flight

I did not add this regression yet because there is no deterministic test hook for holding a daemon request mid-flight without relying on timing or very slow Excel operations.

Minimal product hook request for Cheritto:
- Add a test-only diagnostic command behind an explicit environment gate, for example `diag.block-until-cancelled`.
- It should run through the real daemon RPC path, increment active operation state, block until either shutdown/cancellation arrives or a caller-supplied timeout expires, then return a structured result.
- With that hook, the CLI test can start the blocking request, call `service stop`, and assert the daemon exits cleanly, the client gets a structured cancellation/service-unavailable result, and no session/tracker state is left behind.


## MERGED FROM INBOX: shiherlis-daemon-teardown-revision.md
# Shiherlis — Daemon Teardown Revision

## Decision

Use per-session locks in `SessionManager` to make operation start and close mutually exclusive at the session boundary, and keep sessions registered until `IExcelBatch.Dispose()` succeeds.

## Applied Shape

1. `TryBeginOperation` validates the session, checks timeout/dead-process/quarantine state, and increments the active operation count in one lock.
2. `CloseSession` takes the same per-session lock, blocks normal close while operations are active, saves under the lock when requested, then disposes before metadata removal.
3. Dispose failures leave the session registered and record a teardown-failed quarantine message. Normal operations are blocked; a later close retry can attempt teardown again.
4. Daemon connection drain observes per-connection faults so service disposal still runs.

## Why

This preserves truthfulness during failed COM teardown: a session is not marked closed until Excel cleanup actually completes. It also closes the lookup-to-begin race where a request could acquire a batch after close validation but before operation tracking.


