fix(#507): guard Globals in RibbonController.Engines

.claude/agent-memory/atomic-executor/MEMORY.md

@@ -65,7 +65,8 @@
 - [TaskVisualization #298 ScoCollection + live-bridge exemptions](project_taskvis_scocollection_and_livebridge_exemptions.md) — ScoCollection<T> forces a Swordfish ProjectReference on test assemblies; a default-factory live-form bridge must be method-level exempt
 
 ## Nullable / C# language
-- [Nullable per-file pragma gate mechanics](project_nullable_pragma_gate_mechanics.md) — HISTORICAL: solution-wide TWAE once aborted on vendored SVGControl CS0649 + UtilitiesCS CS0618/CS0168, needing an isolated `UtilitiesCS.csproj -t:Rebuild -p:BuildProjectReferences=false` + grep CS86xx. Verified 2026-08-07: the full-solution TWAE gate now returns EXIT 0 / 0 errors — re-measure before assuming it fails. Supersedes the #364, epic, and net481-mechanics variants.
+- [Nullable per-file pragma gate mechanics](project_nullable_pragma_gate_mechanics.md) — HISTORICAL: solution-wide TWAE once aborted on vendored SVGControl CS0649 + UtilitiesCS CS0618/CS0168, needing an isolated `UtilitiesCS.csproj -t:Rebuild -p:BuildProjectReferences=false` + grep CS86xx. Re-verified 2026-08-08 (#507) with CI's own command (`/t:Rebuild`, TWAE, NO `/p:Nullable=enable`): genuinely EXIT 0 / 0 errors, so the enforced gate really does pass. The 195 UtilitiesCS + 219 TaskMaster error population appears only when `/p:Nullable=enable` is added, which no gate does — see [[project-507-nullconditional-return-triggers-cs8603-under-genuine-nullable-check]]. Still use `/t:Rebuild` (not `/t:Build`) so the pass is not vacuous. Supersedes the #364, epic, and net481-mechanics variants.
+- [CLAUDE.md nullable command != the CI gate](project_507_nullconditional_return_triggers_cs8603_under_genuine_nullable_check.md) — ci.yml omits `/p:Nullable=enable` and relies on per-file `#nullable` pragmas; forced-flag CS86xx in an unannotated file is NOT a blocker. Repro the CI command before failing an AC
 - [Nullable remediation annotation patterns](project_nullable_remediation_annotation_patterns.md) — net481 has no post-condition attrs; struct `= default!`; `.ToString()!` for string cells; IsNullOrEmpty overload gotcha; `x!.M()` for defensive flow-state
 - [Nullable annotation CS8632 scoping](project_nullable_annotation_cs8632_scoping.md) — `Type?` in nullable-disabled projects emits CS8632; wrap in `#nullable enable annotations`/`restore annotations`, not whole-file
 - [init/record struct fails CS0518 on net48](project_record_struct_isexternalinit_netfx.md) — ANY init accessor needs IsExternalInit (absent on net48, no polyfill); use a constructor-initialized readonly struct with get-only props

.claude/agent-memory/atomic-executor/project_507_nullconditional_return_triggers_cs8603_under_genuine_nullable_check.md

@@ -0,0 +1,55 @@
+---
+name: project-507-nullconditional-return-triggers-cs8603-under-genuine-nullable-check
+description: CLAUDE.md's nullable toolchain command (/p:Nullable=enable) is NOT the gate CI enforces; ci.yml omits that flag and relies on per-file #nullable pragmas, so forced-flag CS86xx diagnostics in unannotated files are not merge blockers
+metadata:
+  type: project
+---
+
+CLAUDE.md documents the nullable toolchain stage as
+`msbuild TaskMaster.sln /t:Build ... /p:Nullable=enable /p:TreatWarningsAsErrors=true`.
+The gate that actually governs merge is different. `.github/workflows/ci.yml`
+("Build with nullable warnings treated as errors") runs:
+
+```
+msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true
+```
+
+It uses `/t:Rebuild` (deliberately, to defeat the incremental up-to-date vacuity) but it does
+**not** pass `/p:Nullable=enable`. Its own comment states enforcement "relies entirely on each
+file's own `#nullable enable` pragma (the repo's per-file opt-in convention)".
+
+**Consequence.** Adding `/p:Nullable=enable` force-enables nullable analysis across every file in
+the solution, including the many thousands that were never annotated. That produces a large,
+pre-existing error population (measured 2026-08-08: 195 in `UtilitiesCS.csproj`, 219 in
+`TaskMaster.csproj`) that is red on `main` independently of any change under review. Diagnostics
+surfaced only by that flag, in files with no `#nullable enable` pragma, are artifacts of a
+non-enforced configuration — not merge blockers.
+
+**Worked example (#507).** Changing
+`internal IAppItemEngines Engines => Globals.Engines;` to `Globals?.Engines;` in
+`TaskMaster/Ribbon/RibbonController.Intelligence.cs` (a file with no `#nullable` pragma, in a
+project with no `<Nullable>` element) does emit a new `CS8603: Possible null reference return`
+under a forced `/p:Nullable=enable` isolated rebuild. Under CI's real gate it emits nothing: a
+full `/t:Rebuild ... /p:TreatWarningsAsErrors=true` of the whole solution with the change applied
+returned `EXIT_CODE=0`, zero errors, zero CS8603, zero `RibbonController` diagnostics. The
+sibling `SB` property in the same file already returns `null` from a non-nullable declared return
+type, so the pattern is pervasive and pre-existing, not newly introduced.
+
+**Why:** the repo is mid-migration to nullable reference types via per-file opt-in. The
+project-wide flag is a strictly-stronger configuration that no gate enforces, so measuring against
+it manufactures blockers that cannot be resolved without annotating files far outside a
+minor-audit scope.
+
+**How to apply:** when the nullable stage appears to fail, first check whether the diagnostic is
+in a file carrying `#nullable enable`. If it is not, reproduce the CI command verbatim
+(`/t:Rebuild`, no `/p:Nullable=enable`) before reporting a blocker or leaving an AC unchecked.
+Only diagnostics that survive CI's command are real. Do not resolve a forced-flag-only diagnostic
+by adding `!` or a `Type?` annotation to an unannotated file — `Type?` in a nullable-disabled
+context emits CS8632 (see [[project_nullable_annotation_cs8632_scoping]]).
+
+The separate, still-valid caveat: a solution-wide `/t:Build` nullable pass can be vacuous because
+MSBuild's up-to-date check ignores a changed `/p:` property and skips `CoreCompile`. Confirm via
+output-DLL mtime or use `/t:Rebuild`. See [[project_nullable_pragma_gate_mechanics]].
+
+Related: [[project_incremental_build_vacuous_baseline]],
+[[project_dotnet_coverage_denominator_nondeterminism]].

.claude/agent-memory/feature-review/MEMORY.md

@@ -58,3 +58,5 @@
 - [two vstest binaries: binding-redirect trap](project_two-vstest-binaries-binding-redirect.md) — #503: the TestWindow vstest.console.exe drops the app.config redirect and fakes 26 Moq failures; use Extensions\TestPlatform + /Settings:TaskMaster.runsettings
 - [package-counter delta proves new-type coverage](project_package-counter-delta-corroborates-new-type-coverage.md) — #503: when per-class detail is stripped to JaCoCo summaries, an unchanged package `missed` with `covered` up by exactly the new types' line total proves both the new-code floor and changed-line no-regression
 - [mandated nullable solution gate is vacuous](project_nullable_build_gate_is_vacuous.md) — #503: /t:Build with only /p: changes skips CoreCompile so the gate cannot fail; force /t:Rebuild on the changed project and attribute the errors by file
+- [null-conditional fix relocates NRE, check callers](project_null-conditional-fix-relocates-nre-check-callers.md) — #507: `Globals.Engines`->`Globals?.Engines` matched sibling `SB` precedent and passed full evidence, but all 11 real `RibbonViewer.cs` callers are unguarded, so the NRE just moves one frame later; grep every call site before crediting a throw->null fix with resolving the reachable crash
+- [coverage hook needs label+coverage+PASS/FAIL on one line](project_coverage-hook-label-plus-verdict-same-line-507.md) — #507 R1: `Test-LanguageCoverageRow` requires the language label, a coverage keyword, and PASS/FAIL all on the SAME line, and rejects any label+coverage line carrying a banned narrowing word anywhere; dot-source and simulate before finalizing, don't trust a wrapped narrative paragraph

.claude/agent-memory/feature-review/project_coverage-hook-label-plus-verdict-same-line-507.md

@@ -0,0 +1,39 @@
+---
+name: coverage-hook-label-plus-verdict-same-line-507
+description: validate-feature-review-coverage.ps1 requires the language label token, a coverage keyword, AND PASS/FAIL all on the SAME physical line, with no banned narrowing word anywhere on any line satisfying label+coverage
+metadata:
+  type: project
+---
+
+#507 remediation-cycle-exit review (2026-08-08): `Test-LanguageCoverageRow` in
+`.claude/hooks/validate-feature-review-coverage.ps1` is stricter than prose-level summarization
+suggests. It filters `policy-audit` text to lines matching a language label (`C#`, `CSharp`,
+`csharp`, `.NET`, `dotnet` for CSharp — note `csharp` matches case-insensitively as a substring, so
+a bare artifact path like `` `artifacts/csharp/coverage.xml` `` counts as a label line), then
+further filters those to lines also containing a coverage keyword
+(`coverage|lcov|line[s]?\s+hit|pester`), then requires at least one of those lines to also contain
+literal `PASS` or `FAIL`. Prose spread across multiple wrapped markdown lines (e.g. cycle-1's
+`#507` policy-audit, which had "C#" and "coverage" on one line and "FAIL"/"non-blocking" several
+lines later in the same paragraph) does NOT satisfy this — I confirmed by dot-sourcing the hook and
+calling `Test-LanguageCoverageRow` directly against that exact file, which returned
+`"CSharp coverage rows contain neither a PASS nor a FAIL verdict."` even though the paragraph read
+as compliant to a human. Separately, ANY line matching label+coverage (not just the verdict line)
+that also contains a banned narrowing phrase (`informational only|context only|out of plan
+scope|out of scope|not applicable|N/A|UNVERIFIED`, case-insensitive) unconditionally fails the
+check, even if a different line later gives a clean PASS/FAIL.
+
+**Why:** Confirmed cycle-1's own `policy-audit.2026-08-08T17-45.md` would NOT have passed this
+hook's coverage check as literally worded (verified via direct dot-source simulation), despite the
+review having proceeded to remediation. Writing a short, single-line, unambiguous verdict like
+`` "C# coverage verdict: FAIL (repo-wide raw coverage below floor, pre-existing, non-blocking
+disposition)." `` reliably passes; relying on a longer narrative paragraph does not.
+
+**How to apply:** Before finalizing any policy-audit with a coverage section, dot-source
+`.claude/hooks/validate-feature-review-coverage.ps1` and call `Test-LanguageCoverageRow` directly
+against the drafted text for each changed language (pass `$null`/`$null` for RepoWidePct/BranchPct
+when the canonical artifact is intentionally absent, matching the reviewed feature's actual state).
+Also simulate `Invoke-FeatureReviewCoverageValidation` end-to-end with a synthetic
+`policy-audit-path`/`code-review-path`/`feature-audit-path` payload before reporting the final
+tokens, to catch path-regex or cross-artifact-timestamp mismatches too. See
+[[taskmaster-validator-memories-are-cross-repo]] for why the cross-repo heading-template memories
+do not apply here — this hook, not a heading validator, is the real gate.

.claude/agent-memory/feature-review/project_null-conditional-fix-relocates-nre-check-callers.md

@@ -0,0 +1,39 @@
+---
+name: null-conditional-fix-relocates-nre-check-callers
+description: when a bugfix changes a throwing property/method to return null via ?. (matching a sibling precedent), always enumerate every real caller before crediting the fix with resolving the reachable crash — an unguarded caller just gets the same NRE one frame later
+metadata:
+  type: project
+---
+
+#507 (`RibbonController.Engines`: `Globals.Engines` -> `Globals?.Engines`) looked like a clean,
+minimal, sibling-precedent-matching fix (the `SB` property in the same file already used the same
+pattern) and was fully evidenced (baseline/final toolchain + coverage, expect-fail/post-fix
+regression tests). But grepping every call site of the changed member
+(`rg '\bEngines\b' TaskMaster`) showed all 11 real production callers, in a sibling file
+(`RibbonViewer.cs`) the plan explicitly forbade touching, dereference the result with zero null
+guard. Before the fix: NRE thrown inside the property getter. After: the same click still throws an
+NRE, just one frame later at the call site — the crash is relocated, not eliminated. This is not
+"silent" (still an unhandled exception) and not a regression (nothing relies on the throw for
+control flow — no try/catch, no `!= null` check anywhere), but it does mean the fix's real-world
+impact is limited to property-boundary contract conformance, not resolution of the issue's own
+described reachable-crash symptom.
+
+**Why:** the issue's own risk section can pre-disclose this tradeoff ("shifts the failure mode ...
+widening caller guards is out of scope") and still be worth flagging plainly as a Blocking finding,
+because the AC text ("Engines returns null instead of throwing") is literally true and verified, yet
+a reader could easily believe the underlying user-facing bug is now closed when it is not, for any
+of the enumerated reachable callbacks in the issue's own "Reachable callbacks" list.
+
+**How to apply:** whenever a diff changes a member from throwing to null-returning (or otherwise
+weakens a fail-fast contract) to match a sibling precedent, grep every call site of that member
+across the whole repo (not just the changed file), and for each one ask: does this site null-check
+before use? If none do, state plainly that the crash relocates rather than resolves, cite the
+specific call sites, and rate it Blocking — do not let a strong evidence trail (passing toolchain,
+targeted regression tests, explicit issue-level scope disclosure) substitute for this specific
+end-to-end check. Also check whether the *same* sibling precedent property has the identical
+unguarded-caller pattern (it did here, for `SB`) — that tells you this is a pre-existing codebase
+convention, not a brand-new defect class, which is relevant context for severity/disposition
+language even when the finding stays Blocking.
+
+Related: [[feedback_test-file-500-line-limit]] (the accompanying test-file-size Blocking finding in
+the same review — a two-test addition to an already-452-line file crossed 500 lines again).

.claude/agent-memory/orchestrator/MEMORY.md

@@ -1,3 +1,5 @@
+- [CLAUDE.md nullable command != the CI gate](project_claudemd_nullable_command_diverges_from_ci.md) — ci.yml omits `/p:Nullable=enable`; forced-flag CS86xx in a file with no `#nullable` pragma is a FALSE blocker (~414 errors red on main too). Repro ci.yml's command before accepting a subagent's nullable failure
+- [Agent-worktree discovery + evidence hygiene](project_agent_worktree_discovery_and_evidence_hygiene.md) — the `\.claude\` test-glob exclusion discards ALL assemblies in an agent worktree (filter on the RELATIVE path; 0 discovered = filter bug); and never commit raw Cobertura dumps (81 MB on #507)
 - [Store-lockup watchdog null-model hazard](project_store_lockup_watchdog_null_model_hazard.md) — #260 watchdog is live; new startup COM attribution scopes need a responder phase-branch that returns before the disable-service write, or the watchdog thread crashes on the null store model
 - [VS Code extension location](project_extension_location.md) — the extension lives at `extensions/drm-copilot/`, not at the repo root.
 - [Verify package.json before vsce work](feedback_vsce_verify_package_location.md) — in multi-package repos, never assume the repo root is the publishable extension; locate it first.

.claude/agent-memory/orchestrator/project_agent_worktree_discovery_and_evidence_hygiene.md

@@ -0,0 +1,50 @@
+---
+name: agent-worktree-discovery-and-evidence-hygiene
+description: In a .claude/worktrees agent checkout the standard "exclude \.claude\" test-discovery rule discards every assembly; and never commit raw Cobertura dumps as evidence
+metadata:
+  type: project
+---
+
+Two mechanics that cost real cycles on #507.
+
+## 1. The `\.claude\` test-discovery exclusion inverts in an agent worktree
+
+The standing rule is: when globbing for `*.Test.dll`, exclude any path containing `\.claude\`,
+because ~20 stale `.claude/worktrees/agent-*` checkouts hold old builds that produce bogus
+`AssemblyInitialize` signature failures.
+
+That rule assumes you are running from the main checkout. An isolated agent worktree is itself
+rooted at `...\.claude\worktrees\agent-<id>\`, so **every** absolute path under it contains
+`\.claude\`. Applying the filter to the absolute path discovers zero assemblies and vstest exits 1
+with no useful message.
+
+Correct form: scope `Get-ChildItem` to the worktree root, then filter on the path **relative** to
+that root, excluding nested `.claude` trees, `\obj\`, and `\ref\`:
+
+```powershell
+$rel = $_.FullName.Substring($root.Length)
+if ($rel -match '\\bin\\Debug\\' -and $rel -notmatch '\\obj\\' -and
+    $rel -notmatch '\\ref\\' -and $rel -notmatch '\.claude') { $_.FullName }
+```
+
+Correct discovery yields 9 assemblies (one per `*.Test` project). **A discovery count of 0 is a
+filter bug, never a real failure** — say so explicitly in the delegation prompt, because an executor
+that trusts a 0-count will report a false blocker.
+
+## 2. Never commit raw Cobertura dumps as evidence
+
+An executor committed `phase0-baseline-coverage.cobertura.xml` (37 MB) and
+`phase2-final-coverage.cobertura.xml` (44 MB) as evidence — about 1.42 million inserted lines, for a
+one-line production bugfix. Commit `d0955dc4` ("docs(#503): replace raw cobertura coverage evidence
+with jacoco summaries") had already established the opposite convention.
+
+The evidence conventions require **numeric coverage headlines** in the markdown artifacts, not the
+raw dumps. The dumps are regenerable from the `dotnet-coverage merge ... -f cobertura` command
+recorded in the vstest artifact.
+
+**How to apply:** check `git diff --stat` before opening a PR. A six- or seven-figure insertion count
+on a small change means an agent committed generated output. Removing it in a follow-up commit is not
+enough — the blob stays in history; rewrite the branch (`reset --soft` to the merge base, restage
+without the files, recommit, `push --force-with-lease`) before the PR exists.
+
+Related: [[feedback_commit_before_ci_gate]], [[feedback_commit_all_evidence_clean_worktree]].