Repository navigation
Add Start-FinOpsMultitool — cross-platform terminal UI for FinOps scanning - #2155
Zac Larsen (z-larsen) wants to merge 153 commits into
Conversation
… GUI Adds the Azure FinOps Multitool as a new PowerShell cmdlet in the FinOps toolkit. The Multitool is a WPF-based GUI that scans an Azure tenant for cost optimization, governance, and FinOps insights including cost trends, orphaned resources, idle VMs, tag hygiene, reservation/savings plan utilization, AHB opportunities, budgets, anomaly alerts, and policy compliance. - Public/Start-FinOpsMultitool.ps1: thin launcher cmdlet with comment-based help - Private/FinOpsMultitool/: full implementation (24 scanner modules, WPF GUI, Power BI template) - Tests/Unit/Start-FinOpsMultitool.Tests.ps1: Pester unit tests Windows-only (requires WPF support).
microsoft-github-policy-service
Bot
added
Needs: Documentation 📝
microsoft-github-policy-service
Bot
added
the
Tool: PowerShell
|
@microsoft-github-policy-service agree company="Microsoft" |
|
Zac Larsen (@z-larsen) This is exciting! I don't know much about the tool, but would love to learn more. Can you join us at the contributor sync next Wednesday to share? |
|
Thanks, Michael! Would love to join. |
…info - Add contract-aware cost access warning banner (EA/MCA/CSP) on Overview tab - Add contract-specific billing tab messages when billing access unavailable - Add MG hierarchy unavailable info node in tree view with role guidance - Fix tag cost queries: use TagKey grouping type (not Tag/Dimension) - Add batched TagKey+TagValue query attempt with per-tag fallback - Clear skipSubs between batched and per-tag strategies - Add throttle pacing (2s every 2 queries) to avoid 429s - Add EA/MCA cost access detection in Get-CostData - Add runspace pool for API call parallelization
- Document that -SubscriptionId returns an error when the subscription cannot be resolved and nothing can answer a prompt, rather than scanning every subscription. - Refresh ms.date on the six pages this pull request changes. The update workflow is skipped for fork pull requests.
- Correct full-month forecast windows, column matching, and row totals. - Flag actual-only forecast fallbacks. - Keep measured zero utilization without counting absent commitments. - Restrict export totals to selected subscriptions.
- Reject incomplete pages, required-query failures, and invalid costs. - Preserve POST bodies and discard failed attempts before retries. - Calculate hourly vCPU cost from the captured UTC reporting window.
- Separate billed and amortized costs; reject invalid amounts and incomplete coverage. - Preserve subscription scope, currencies, credits, and observed periods. - Keep unknown budget forecasts and unsupported history unavailable. - Surface hub source failures and unverified financial KPIs.
- Respect explicit data-source selection and retain Kusto provenance. - Validate savings currencies and exclude non-usage charges. - Separate month-to-date commitment estimates from the AHB estimate. - Export nested CSV summaries once and use invariant amounts and ISO dates. - Align public help and documentation with the Microsoft style guide. - Add source, savings, export, and pagination regression coverage. Validation: 2812 unit tests passed (7 skipped), 7144 lint checks passed, and no ScriptAnalyzer findings. Two independent reviews approved the batch. Successful online Hub validation remains blocked by a 403 response.
Michael Flanakin (flanakin)
left a comment
Michael Flanakin (flanakin)
left a comment
There was a problem hiding this comment.
🤖 [AI][Claude Code] PR Review
Summary: Ambitious, well-organized addition (108 files, 30 read-only scanner modules, terminal UI, 12 new agent skills). Read-only guarantee holds everywhere it was checked — no mutating Azure calls found in any reviewed file. The dominant issue is a systemic pattern: several modules sum Azure Cost Management amounts across subscriptions without checking currency consistency first, unlike the project's own Get-SavingsRealized.ps1/Get-CostTrend.ps1/Get-CostData.ps1, which correctly throw or track per-currency. In an EA/MCA tenant with subscriptions billed in different currencies, this silently produces wrong dollar totals. There's also one execution-verified macOS bug (8 failing Pester tests) and a fail-fast bug that discards an entire scan's results on the first subscription access error.
Pester run on this branch: 526 passed, 8 failed, 7 skipped — all 8 failures trace to two root causes documented below.
Three smaller, non-blocking polish items were split out to #2335 (v16 milestone) rather than posted inline, since they're real but not urgent: a misattributed diagnostic message, a missing static test-guard for the read-only invariant, and missing comment-based help on the primary TUI entry point.
🚫 Blockers (6)
Get-CostByTag.ps1— a single failed subscription throws and discards the entire scan's results.Get-CostByTag.ps1— sums cost across subscriptions/currencies, labels the total with whichever currency was seen last.Get-AIWorkloadMetrics.ps1— same cross-currency summing bug in AI cost-per-token/request metrics.Get-SharedCostAllocation.ps1— the FinOps Hub data path has no currency-mismatch check (the Live API path in the same function does).Get-UnitEconomics.ps1— currency is correctly flagged"Mixed", but the cost/unit-economics numbers next to it aren't gated on that flag.Read-FinOpsHubData.ps1— macOSstat -f '%Lp'drops the sticky bit, breaking private-directory validation for any path under a sticky world-writable ancestor (e.g./tmp). Execution-verified via the failing Pester run above.
⚠️ Should fix (15)
7-8. Same cross-currency summing pattern in Get-OptimizationAdvice.ps1 and Get-ReservationAdvice.ps1 (Advisor savings estimates).
9-12. Missing nextLink pagination: Get-BillingStructure.ps1 (5 calls), Get-MaccCommitment.ps1 (2 calls), Get-BillingAccount.ps1, MgCostScope.ps1.
13-15. Weak/silent failure logging: Get-TagInventory.ps1, Get-PolicyInventory.ps1, Get-ContractInfo.ps1.
16-17. Invoke-FinOpsMultitool.ps1 — silent Hub-detection failure swallowing, and a shared catch that mislabels an untried scan as failed.
18. MultitoolSafety.Tests.ps1 — the "no undefined commands" self-test false-fails on non-Windows (one of the 8 real Pester failures above).
19. Test coverage: 16 of 30 scanner modules have no test exercising their own domain logic, despite the PR checklist marking unit tests as done.
20. finops-reporting/SKILL.md references a content-humanizer skill that doesn't exist anywhere in the repo.
21. finops-multitool-commands.md's scan-coverage table doesn't disclose that 4 of the 30 modules aren't reachable through the documented menu or -Scans parameter.
💡 Suggestions (8)
Minor: stale .gitignore entry, a malformed survey URL, changelog tense inconsistency, a duplicated bullet, an unjustified tolower(), a USD-only savings figure with no currency label, dead code, and the same hub-path currency pattern as the blockers (lower risk here, but worth the same fix while touching this code).
| elseif ($DataSource.Source -eq 'Hub' -and $DataSource.HubStorage) { | ||
| # Storage reader: small-dataset convenience path (rows loaded into | ||
| # PowerShell). For large hubs, the Kusto path above is preferred. | ||
| $hub = $DataSource.HubStorage | ||
| if ($DataSource.Source -eq 'Hub') { | ||
| Write-FinOpsConsole "" | ||
| Write-FinOpsConsole " Loading cost data from FinOps Hub storage (small-dataset reader)..." -ForegroundColor Green | ||
| Write-FinOpsConsole " For large hubs, query the Kusto database instead (ADX/Fabric, or set FINOPS_HUB_KUSTO_URI for ftklocal)." -ForegroundColor DarkGray | ||
| } | ||
| else { | ||
| Write-FinOpsConsole " Loading Hub tag data for fast tag scans..." -ForegroundColor DarkGray | ||
| } |
There was a problem hiding this comment.
🤖 [AI][Claude Code] 💡 Suggestion
Inside elseif ($DataSource.Source -eq 'Hub' -and $DataSource.HubStorage), the nested if ($DataSource.Source -eq 'Hub') {...} else {...} (966-972) has an unreachable else branch, since the outer condition already guarantees Source -eq 'Hub'. No functional impact, but it reads like a "fast tag scan only" code path that no longer exists — leftover from an earlier refactor.
- Preserve partial cost coverage and currency-safe monetary estimates. - Retain billing pagination and membership lookup failures. - Keep missing inventory, utilization, and carbon evidence unavailable. - Reconcile small allocations and preserve macOS permission bits. - Fix Parquet row mapping and case-sensitive module packaging. - Add real reader and packaged-launch tests with platform CI evidence. - Clarify documentation and financial units in agent skills.
|
Published ae36741e with the September 23-24 review fixes and regression coverage. Follow-up f11f8611 moves the runner-local temporary directory setting to the test steps. GitHub rejected the original job-level Review follow-up
Local validationWindows, PowerShell 7.6.6, Pester 6.0.0:
CI and remaining limitsThe workflow includes Windows, Ubuntu, and macOS multitool jobs, with NUnit artifacts and summaries identifying the tested commit and host. No Azure sign-in or deployment credentials are used. Hosted Linux/macOS results are not yet available; local Windows results don't establish native compatibility. macOS runs the native unit and packaged-launch checks. Microsoft documents NuGet signed-package verification as unsupported on macOS, so real signed Parquet integration runs on Windows and Ubuntu. Production signature verification remains enforced; no bypass was added. Kusto or available CSV exports are the alternatives on macOS. The integration tests don't prove live Azure coverage. A new VM test package is ready; live validation of the current build remains pending. The three follow-ups in #2335 remain deferred to v16: storage error classification, a static read-only scanner guard, and private launcher help. This update doesn't claim to close them. The README retains usage, limitations, and reusable test instructions. PR-specific tracking is kept in this comment instead. |
Add a searchable KPI reference with formulas, inputs, interpretation and limitations. Clarify cost-share denominators, measurement periods and forecast availability. Preserve measured zero unit rates and report export when the KPI catalog is unavailable.
Michael Flanakin (flanakin)
left a comment
Michael Flanakin (flanakin)
left a comment
There was a problem hiding this comment.
🤖 [AI][Claude Code] PR Review — verification of the 2026-09-24 fix commits
Follow-up to the 2026-09-23 review. The fix commits (ae36741 et al.) addressed nearly everything from that review — verified via 4 independent passes, each re-checking a slice of the diff against the original findings, plus a fresh look at files outside the original scope. Full current test suite: 2940 passed, 0 failed.
What's confirmed fixed and correct: all 4 currency-summing bugs (Get-CostByTag.ps1, Get-AIWorkloadMetrics.ps1, Get-UnitEconomics.ps1, plus 2 more found and fixed proactively in Get-OptimizationAdvice.ps1/Get-ReservationAdvice.ps1), the fail-fast bug, the macOS sticky-bit bug (verified empirically on this machine), the mislabeled-scan-failure bug, missing pagination in 5 of 5 flagged files, the stale content-humanizer skill reference, the duplicated SKILL.md bullet, and the non-Windows test false-fail. Scanner test coverage went from 14/30 to 29/30. Great pass overall.
One blocker found in the fix itself: fixing the silent Hub-detection-failure issue introduced two unhandled throws that now crash the whole tool on a transient probe error — including in -NonInteractive/CI mode, where the old code silently (if unhelpfully) fell back. Detail below.
Three should-fix items and three suggestions — all non-blocking gaps or minor issues in the fix commits — were added to the existing tracking issue rather than posted here: #2335
🚫 Blockers (1)
Select-DataSource's new error handling for Hub auto-detection throws unhandled in two spots, hard-crashing-NonInteractive/CI runs on a transient probe failure instead of falling back.
- Preserve selected tenant and subscription scope through Hub discovery failures. - Reconcile AI token totals and account identities with matching UTC cost windows. - Correct commitment metadata, pagination, and unavailable utilization. - Add scoped cost trends, resource periods, policy names, and readable local grids. - Clarify budget sampling and CPU units; close deferred diagnostic/help/test gaps. - Harden credit sorting, CSV formula protection, and the scanner read-only guard. - Validate 3170 unit tests, 7144 lint checks, runtime parity, and independent review.
Review follow-up: fe9ddf1Copilot-assisted implementation update. Published to this PR's branch and mirrored to the standalone TUI at Fixes and why the reported behavior should no longer recur
Validation performed
Remaining limitsLive Azure collection and native macOS/Linux execution were not tested in this round. The AST check is a test-time defense, not a proof about every dynamic command or POST endpoint. Billing-scope commitment results can exceed the selected subscription scope and remain explicitly labeled. Maintainer review and approval are still required; this update does not claim merge approval. |
…review findings Adds an ordinary Cost Management CSV export data source so a scan can run without deploying a FinOps hub, plus the corrections raised in two independent reviews of that change. Added - New `Export` value on `-DataSource` and a data source menu entry. Discovery reads export definitions for the selected subscriptions, their management-group ancestors, and linked billing accounts, then always scans storage accounts in those subscriptions and merges exports Cost Management cannot see, deduped against the definitions. - Four export-backed cost views: cost totals, resource costs, cost by tag, and cost trend. Every other financial scan is rejected with an explicit error and never falls back to live Cost Management costs. - Progress reporting per scope, per subscription, and per storage account, with discovery warnings summarized and detail available under -Verbose. Review findings corrected - Storage-first discovery was gated behind an empty definition result. That reintroduced a previously fixed defect: a cross-tenant scan can return definitions for some subscriptions while a central management-group export stays invisible, so gating hid it. The gate is removed and a regression test now asserts the storage pass runs even when definitions exist. - Partitioned runs were totalled without consulting manifest.json. A run whose parts are partly missing or still being written reported a confident total. The declared partitions are now verified and a short read is reported as incomplete. - A blank FOCUS SubAccountId failed the whole read. FOCUS permits null on tenant-level charges such as MCA purchases and refunds, which the newly discoverable billing-account exports contain. Those rows are now excluded from a scoped read and counted; a malformed non-empty ID still fails. - Resource costs were keyed on resource identifier alone, so a classic export carrying a bare instance name merged same-named resources across subscriptions. The key now includes the subscription. - The unattended single-candidate rule counted candidates this path cannot read, so one readable CSV beside a Parquet folder failed as ambiguous. It now counts readable candidates, and the picker labels the others. - Trend months were bucketed with the host calendar, so a non-Gregorian culture split one month across two rows. The key is now invariant. - The discovery dedupe key trimmed the root folder on the storage side only, so a definition path with a leading slash produced a duplicate candidate. Documentation - Corrected overstated claims: reads are limited to the chosen export folder rather than generally bounded, only the newest run is read, and the container name filter and wider-scope storage destinations are now documented. - Documented the `Export` value in the public command reference. Validation - 3,202 unit tests passed, 0 failed, 8 skipped, offline behind a closed loopback proxy. - 7,144 lint tests passed. - PSScriptAnalyzer reports 0 errors and warnings on the changed files. - Runtime parity with the standalone edition verified across 47 files. - No live Azure calls; all fixtures synthetic.
Current-head review update: fba4d01Copilot-assisted follow-up. The latest changes are in fba4d01 and mirrored to the standalone TUI at 7c39489. Changes since the last summary
Existing paging wrappers and upstream currency checks remain in place. Storage discovery still runs when export definitions are found, so mixed discovery paths aren't dropped. Failed selected-source reads don't become live API scans, and sampled budget coverage stays unverified. Verification
Review statusI checked all 154 threads, replied with current-commit evidence, and resolved 81 verified findings. Seven threads remain open:
The current-head PowerShell Tests workflow is waiting for maintainer approval and has zero jobs, not failing tests. The ms.date and PR Deploy workflows are also awaiting action. No workflows or deployments were approved or dispatched in this round. Michael Flanakin (@flanakin) Brett Wilson (@MSBrett), could you inspect this head, approve the appropriate validation workflows, and re-review the remaining requests? Thread cleanup doesn't clear the existing changes-requested review or replace maintainer approval. |
This branch has not been deployed
6 participants
🛠️ Description
Adds the FinOps multitool to the FinOps toolkit. Discussed with Brett Wilson (@MSBrett), who suggested contributing the tool into the official toolkit.
The multitool scans an Azure environment for cost optimization, governance, and FinOps insights — cost trends, orphaned resources, idle VMs, tag hygiene, reservation and savings plan utilization, Azure Hybrid Benefit opportunities, budgets, anomaly alerts, and policy compliance — and grounds its findings in live resource state.
Everything in this PR is read-only. It needs Reader or Cost Management Reader on the target scope and never creates, changes, or deletes a resource. Remediation and the MCP server were split out to a separate branch and will follow as their own PR.
One scanner engine, two consumers:
Start-FinOpsMultitoolsrc/templates/agent-skills/azor an Azure MCP serverRunning it
The terminal UI uses arrow-key menus when the console supports them. Consoles that can't render those menus — PowerShell remoting sessions, some editor terminals — fall back to numbered prompts, which is also what a screen reader can follow. Both paths run the same scans and produce the same results.
For automation,
-NonInteractivewith-Scans,-DataSource,-SubscriptionId, and-OutputPathsupplies every choice, so the tool runs from a pipeline or a scheduled job:FinOps hub data paths
This addresses Brett Wilson (@MSBrett)'s scaling review. When a FinOps hub is present, cost scans prefer the hub's Kusto database — an Azure Data Explorer or Fabric cluster, auto-discovered through Resource Graph, or a local ftklocal emulator via
FINOPS_HUB_KUSTO_URI— and push aggregation into the engine, returning only summarized result sets. Raw cost rows are never materialized in PowerShell on that path.The storage-export reader remains as a small-dataset fallback rather than the scalable path, and the terminal UI warns before using it on a hub with no reachable cluster, offering the live Cost Management API instead.
Scans
30 scan modules across optimization, governance, cost analysis, commitments, monitoring, Advisor, account, AI and ML, and sustainability. The terminal UI surfaces 26 of them. Results render in the terminal and export to one CSV per scan, a
FinOpsReport.htmlsummary, and aScanSummary.txtfile.📦 Files added / changed
Public/Start-FinOpsMultitool.ps1Invoke-FinOpsMultitool.ps1+FinOpsMultitool.psm1modules/helpers/Get-FOHubProvider.ps1+Invoke-FOHubKustoQuery.ps1agent-skills/finops-multitool/+references/agent-skills/cost-data-source/agent-skills/{power-bi-finops, cost-allocation, …}/Tests/Unit/Start-FinOpsMultitool.Tests.ps1+FOHubProvider.Tests.ps1docs-mslearn/.../powershell/multitool/+docs/multitool.md📸 Screenshots
Screenshots are in the public repo README.
📋 Checklist
🧪 How did you test this change?
🐳 Deploy to test?
N/A — standalone PowerShell tooling, not a template deployment.
🏷️ Do any of the following that apply?
📄 Did you update
docs/changelog.md?📖 Did you update documentation?
docs-mslearn/.../powershell/multitool/, a Jekyll landing page, overview/TOC/changelog entries, and the module README plus thefinops-multitoolandcost-data-sourceskills.