Repository navigation
Add Start-FinOpsMultitool — cross-platform terminal UI for FinOps scanning - #2155
Zac Larsen (z-larsen) wants to merge 163 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
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. |
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 fe9ddf12..0989e6d3)
Summary: Both earlier blockers (hub discovery throws) are fixed, and the 906 multitool unit tests pass locally. No new blockers. 3 should-fix items remain, all in the "silent wrong total" class. Other non-blocking items are tracked in #2335.
⚠️ Should fix (3)
Get-ResourceCosts.ps1: per-subscription fallback overwrites rows that share a ResourceId (understated cost)Get-OptimizationAdvice.ps1: REST fallback re-adds recommendations already collected (doubled savings)Get-CostExport.ps1: dollars on excluded null-subscription rows are never surfaced
- Resource costs: keep every row from the per-subscription fallback; unattributed charges and resource IDs that differ only by case no longer overwrite each other. - Advisor and reservation advice: clear rows read before a Resource Graph failure so the REST fallback doesn't count them twice. - Exports: report the count and per-currency amount of rows with no subscription, which are excluded from subscription totals. - Remove always-true Hub source checks inside the Hub-only branch.
- Cost trend: an omitted subscription billed in a different currency than that month's total now stays unverified, with the reason in QueryErrors, instead of failing the whole trend.
- Cost trend: the warning, coverage note, and report label ("Subscriptions not added") now cover currency rejections as well as failed queries.
- Report printing: details inside the notes panel print at full height, and the docs no longer claim printing expands collapsed details.
- Docs: explain that a management-group response with no rows for any selected subscription queries every subscription instead, and a failed query fails the scan.
- Report text: remove two "above" references that pointed the wrong way after notes moved above their tables.
- Tests: add the different-currency gap-fill scenario and assert the new print rule.
- Cost Data: a forecast that returns no rows or HTTP 204 now keeps the actual costs, and the forecast stays Unavailable instead of failing the scan or repeating month-to-date spend. - Cost Data: management-group forecast currencies are checked for every subscription before any forecast is stored, so a rejected group response can't leave a stale forecast. - Throttling: shared REST retries read the real response headers and wait for the longest Retry-After or Cost Management/Consumption retry-after value, and cap total retry waiting at 120 seconds per call. - Cost by Tag: batches read the real throttling headers and share one deadline, return the throttling error instead of waiting when the delay won't fit, skip requests that can't start in time, and keep a response that finishes at the deadline. - Docs: explain empty forecasts, HTTP 429 handling, and when scans fail or only warn. - Tests: cover empty and 204 forecasts, header handling, the retry budget, the batch deadline, and cleanup.
|
Follow-up in 278b7ae. These changes keep the scans accurate when Azure returns an empty or throttled response.
|
|
Brett Wilson (@MSBrett), when you have a moment, could you take another look? Your Sep 9 review (on 45f6a4e) raised five items. All five are in the current head, 278b7ae, and the six threads are resolved:
Since then, the branch has also gained the Cost Management export source (ordinary CSV exports work without a hub) and explanations of KPI values and scan thresholds. It also has the forecast and retry handling from the Oct 11 update. Thanks! |
# Conflicts: # docs-mslearn/toolkit/powershell/powershell-commands.md
Brett Wilson (MSBrett)
left a comment
Brett Wilson (MSBrett)
left a comment
There was a problem hiding this comment.
Rechecked the deferred concerns in #2335 against 6cc7d317, with an independent Scrutiny cross-check. Thanks for addressing the earlier findings.
Addressed
The original three concerns are implemented: storage ARM lookup errors distinguish authorization/authentication failures from other lookup failures; scanner modules and helpers have a static read-only command guard; and the private launcher has valid comment-based help covering all current parameters. The guard is useful regression coverage, not a claim of absolute enforcement.
The later policy-definition coverage, bounded tenant-keyed management-group probes, billing response ordering, contract-note trimming, direct AHB coverage, empty AI usage-series handling, and discovery-console sanitization are also addressed: issue items #4, #5, #7, #8, #9, #18, and #19. Item #6 duplicates #2.
Remaining deferred items
| Issue item | Current status |
|---|---|
| #10 | Automatic non-interactive Hub discovery can still select API after discovery failures. This is now explicitly documented intent, not a silent or newly introduced defect. |
| #11 | Incomplete export discovery still permits unattended selection when one readable candidate remains (Invoke-FinOpsMultitool.ps1:336-405). |
| #12 | Explicit/non-interactive Hub selection can reach the storage reader after provider-discovery failure without the interactive size/reachability checks (Invoke-FinOpsMultitool.ps1:1060-1073,1130). The README's statement that these warnings still apply needs qualification. |
| #13 | Hub storage still throws on subscriptionless rows, unlike the Export path's exclude-and-report behavior (Read-FinOpsHubData.ps1:1135). |
| #14 | The generic resolver's deduplication key still omits the root folder. Latent: the launcher uses its own correct key (Resolve-CostDataSource.ps1:487-494). |
| #15-16 | Export runs still have no overall byte/row cap, and ambiguous manifestless folders can still have multiple CSV snapshots summed (Get-CostExport.ps1:859-892). |
| #17 | The single-subscription trend path still lacks the other paths' same-month mixed-currency check (Get-CostTrend.ps1:224-233). |
| #20 | An unsafe CSV header still aborts subsequent report writing. Latent with current schemas; the error is surfaced and in-memory results remain (Invoke-FinOpsMultitool.ps1:1719,3189). |
| #21 | Omitted-subscription trend queries remain sequential and unbounded. Long-throttle suppression mitigates repeated waits but does not bound the loop (Get-CostTrend.ps1:263-299). |
These remain deferred follow-ups rather than a blanket set of new merge blockers, so #2335 should remain open.
New finding
P2: A recorded throttle window can be bypassed before its deadline. Once its positive remainder fits the next call's wait budget, the shared helper sends immediately instead of waiting. The parallel Cost by Tag gate has the same behavior. Details are in the inline comment. This is a throttle-compliance/availability issue, not silent cost corruption.
This was a source-only review; no tests, builds, or live Azure calls were run.
| ) | ||
| # An earlier call learned that Cost Management won't answer inside this call's wait budget. | ||
| $throttleKey = Get-CostMgmtThrottleKey -Path $Path | ||
| if ($throttleKey -and (Get-CostMgmtThrottleRemaining -Key $throttleKey) -gt $MaxRetryWaitSeconds) { |
There was a problem hiding this comment.
[P2] Wait for the recorded throttle deadline before sending
If a previously registered long throttle window still has 90 seconds left, the default 120-second budget makes this condition false. Execution then enters the request loop immediately, without waiting for the remaining 90 seconds. The budget determines whether a wait is affordable; it does not mean the server's backoff period has expired. This contradicts the helper's warning that later requests are skipped until the recorded time and can generate another avoidable 429.
Please either wait out the positive remainder and charge it against the retry budget before the first request, or keep returning the skipped response until the window expires. The independent parallel gate in Get-CostByTag.ps1:314-322 needs the same treatment because those jobs call Invoke-AzRestMethod directly. The existing budget-covers-window case in RetryJitter.Tests.ps1:256-280 currently expects an immediate call, so it also encodes this early-send behavior.
There was a problem hiding this comment.
Brett Wilson (@MSBrett) the P2 throttle-window finding is fixed in 0a1cab8. I took the second option you offered: while a query or forecast window is open, requests to that API aren't sent and get the local 429 until the recorded time, whatever their wait budget.
- Shared helper (
Invoke-AzRestMethodWithRetry.ps1:188-192): the gate no longer compares the remaining time with the call's wait budget. Any positive remainder returns the skipped response, as the helper's warning says. Once the window ends, requests go out again. - Cost by Tag (
Get-CostByTag.ps1:316): the parallel gate follows the same rule. No batch call is sent while a window is open, even when the remainder fits the 120-second batch deadline. - Tests: the budget-covers-window case in
RetryJitter.Tests.ps1:256now expects the skip, with budgets of 300 and 30 seconds. A new case at line 283 checks that a request is sent again after the window ends. A new tag-batch test,CostQueryPagination.Tests.ps1:1886, uses a 90-second window as in your example and expects zero calls.
The README and the Learn commands page now say requests are skipped until the recorded time, even when the window is nearly over.
- Cost Management request limit: skip requests until the recorded time, even when the remaining wait fits the call's retry budget (shared REST helper and Cost by Tag batches). - VM cost breakdown: name the required role only for access errors, and explain request limits and timeouts in plain language.
# Conflicts: # docs-mslearn/toolkit/changelog.md
- Linux lookups: take only the first PATH match for id, stat, dotnet, chmod and mkdir. /bin links to /usr/bin there, so each program was listed twice and the call that used the result failed with "The term '/usr/bin/id /bin/id' is not recognized". - Tests: add a guard that scans the module for unrestricted application lookups, and make the Get-Command fixtures return two matches unless one is requested.
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.