Skip to content

fix(929): remove altcover imports, correct SVGControl binding redirects, pass client-id to the repair workflow token step - #949

Merged
drmoisan merged 14 commits into
mainfrom
bug/package-manifest-consistency-residuals-929
Sep 30, 2026
Merged

drmoisan merged 14 commits into
mainfrom
bug/package-manifest-consistency-residuals-929

Conversation

@drmoisan

Copy link
Copy Markdown
Owner

Suggested title

fix(929): remove altcover imports, correct SVGControl binding redirects, pass client-id to the repair workflow token step

Summary

  • Removes the two Exists()-guarded altcover Import elements from QuickFiler.Test/QuickFiler.Test.csproj; no packages manifest declared the package, so the imports were silently skipped.
  • Corrects the stale SVGControl/app.config binding redirects: Fizzler now redirects to 1.3.1.0 and System.Runtime.CompilerServices.Unsafe to 6.0.3.0, matching the SVGControl project references and the restored assemblies.
  • Switches .github/workflows/dependabot-repair.yml from the deprecated app-id input to client-id on actions/create-github-app-token@v3; the secret name DEPENDABOT_REPAIR_APP_ID is unchanged and now holds the App's Client ID.
  • Updates the GitHub App installation runbook (issue 911 feature folder) and .github/workflows/README.md to instruct the maintainer to store the Client ID, not the numeric App ID.
  • Adds regression coverage: two in-memory Import-kind tests in ConsistencyVerifier.Tests.ps1 and a new RepositoryTreeConsistency.Tests.ps1 with four tree tests (Import census, SVGControl redirects, workflow input, runbook secret name).

Why

Issue 929 consolidates the residual package-manifest consistency defects: a project's files and its own packages.config must agree, binding redirects must name the assembly version that ships, and the repair workflow must use the non-deprecated token input. The repair script reconciles redirects only for packages it upgrades, so the pre-existing SVGControl drift was hand-corrected and observed read-only with the pure Invoke-BindingRedirectReconciliation function (pre-fix 1 repair per assembly, post-fix 0).

What Changed

Build and configuration

  • QuickFiler.Test/QuickFiler.Test.csproj: two altcover Import lines deleted (570 to 568 lines).
  • SVGControl/app.config: two bindingRedirect lines corrected.

CI workflow and documentation

  • .github/workflows/dependabot-repair.yml: client-id input; header comment describes the Client ID secret.
  • .github/workflows/README.md: secret table row describes the Client ID.
  • docs/features/active/2026-09-19-dependabot-fanout-and-ci-failing-nuget-upgrades-911/runbooks/github-app-installation-token.runbook.md: Part B heading, steps 10 and 22, and the YAML sample.

PowerShell tooling and tests

  • scripts/dependencies/ConsistencyVerifier.psm1: comment-only update (line count unchanged at 499); no detection rule changed, because the absent-from-manifest detector already covers Import elements.
  • tests/scripts/dependencies/ConsistencyVerifier.Tests.ps1: two Import-kind tests; stale fixture comments corrected.
  • tests/scripts/dependencies/RepositoryTreeConsistency.Tests.ps1 (new): four tests that read tracked files only and create nothing on disk.

Evidence and audits

  • docs/features/active/2026-09-28-package-manifest-consistency-residuals-929/: atomic plan (revision 3.2), Phase 0 baseline, regression, QA-gate evidence, and the reduced-audit policy audit, code review and feature audit.

Architecture / How It Fits Together

The new tree test calls Find-PackageAbsentFromManifest from ConsistencyVerifier.psm1 for every project directory that carries a manifest, so an unmanifested Import anywhere in the tree now fails the Pester suite that CI already runs (_pester.yml). The workflow and runbook tests extract the secret name from the workflow and assert it in the runbook, so the two documents cannot drift apart silently.

Verification

Completed (recorded in the feature folder evidence)

  • Fail-before and pass-after: the four tree tests failed on the unfixed tree (4 of 4) and pass after the fix.
  • PowerShell: PoshQC format (no rewrites), PoshQC analyze (pass, 0 findings; the tool reports no count), PoshQC test (local JUnit counts per touched file).
  • CI Pester on implementation head b96926588 (run 36722780748): 379 passed, 0 failed (main baseline run 36666302259: 373); line coverage 94.51 percent (1721 of 1821), equal to baseline; ConsistencyVerifier.psm1 158 covered, 2 missed, equal to baseline.
  • C#: cold package restore; analyzer census 162 items in 17 project files, all declared by their own manifests (no back-fill needed after the analyzer-path hotfix on main); CSharpier check, analyzer rebuild and nullable rebuild green; MSTest 7346 of 7346 with 85.92 percent line and 80.08 percent branch coverage (baseline 85.91 and 80.08).
  • actionlint 1.7.7: no findings for the modified workflow or the repository.
  • Repository hygiene guard: HYGIENE Findings=0.
  • Reduced audit: 0 blocking findings in the policy audit, code review and feature audit; AC1 to AC7 evaluated PASS.

Recommended

  • Confirm the pull request CI run is green on the final head.

Backward Compatibility / Migration Notes

  • The secret DEPENDABOT_REPAIR_APP_ID must hold the GitHub App's Client ID once provisioned. The secret is not yet provisioned, so the token step already fails and this change requires no maintainer action to merge.
  • No C# source file is changed.

Risks and Mitigations

  • MSTest timing tests: the workflow-dispatch run on b96926588 failed one pre-existing concurrency test (QfcItemController_UiThreadDispatcherFixtureTests.Transaction_SecondCallerCannotInstallUntilTheFirstRestores), and one local iteration failed a different timing test (RemainingLoadActive_AcrossAsyncVoidFirstAwait_StaysTrueWhileLoaderProduces). This change edits no C# source and the altcover imports it removes were never resolved by a restore, so the compiled test assembly is unchanged; the reduced audit assessed both failures as not attributable to this change.
  • Rollback: revert the implementation commit; no data or schema migration is involved.

Review Guide

  1. SVGControl/app.config and QuickFiler.Test/QuickFiler.Test.csproj (four changed lines in total).
  2. .github/workflows/dependabot-repair.yml, the workflows README row, and the runbook.
  3. tests/scripts/dependencies/RepositoryTreeConsistency.Tests.ps1 and the two new tests in ConsistencyVerifier.Tests.ps1.
  4. The feature folder is evidence and audit material and can be skimmed.

Follow-ups

Not filed from this branch; listed for the coordinator.

  • Two timing-dependent QuickFiler.Test tests (named above) should be investigated under the determinism rule.
  • Eight tracked *.csproj.bak copies remain; two still contain the altcover token. Removal is a separate housekeeping change.
  • Runbook line 301 still refers to the "App ID location"; a wording update to Client ID is recommended.
  • The dependabot-repair.yml line 14 header comment is about 150 characters and could be re-wrapped.
  • Eleven other app.config files still redirect Fizzler to 1.3.0.0; already recorded in docs/features/potential/2026-08-04-stale-fizzler-and-unsafe-binding-redirects.md.
  • Maintainer follow-up (not a merge gate): acceptance criteria AC18 to AC20 of issue 911 remain deferred until the GitHub App credential is provisioned, and the secret store (repository Actions secrets versus Dependabot secrets for a Dependabot-triggered workflow_run) remains to be confirmed.
  • The Meziantou analyzer-item skew described in the original plan does not survive the analyzer-path hotfix on main (0 undeclared analyzer package folders); no potential entry was authored.

GitHub Auto-close

🤖 Generated with Claude Code

…ive minor-audit folder with acceptance criteria

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

Claude-Session: https://claude.ai/code/session_01KoweznWqwJTkNCf6756FoF
…the pre-existing analyzer package gap

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

Claude-Session: https://claude.ai/code/session_01KoweznWqwJTkNCf6756FoF
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…bsoleted back-fill marked N/A, P1-T13 removed, PoshQC and CI-sourced PowerShell gates)

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com

Claude-Session: https://claude.ai/code/session_01N3r7uhChZ6XRKuQntGLpsG
… deltas R1 to R9, A1 and A2

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
… delta D-1

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
… client-id to the token action

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…k-off

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
…(0 blocking)

Co-Authored-By: Claude Opus 5.5 noreply@anthropic.com
@drmoisan
drmoisan merged commit 66afa63 into main Sep 30, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: package-manifest-consistency-residuals

1 participant