Skip to content

An MCP read checks a Query Store plan before anyone forces it (#5632) - #6065

Merged
erikdarlingdata merged 4 commits into
devfrom
feat/5632-force-precheck
Oct 18, 2026
Merged

erikdarlingdata merged 4 commits into
devfrom
feat/5632-force-precheck

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner

Part of #5632.

Why

#5632 will let agents force Query Store plans. Real cases on Darling stores showed what a force needs checked first: statements with a #temp table failed when replayed, a force on a Parameter Sensitive Plan (PSP) variant got 0 executions, Query Store goes READ_ONLY, a target can stop running, and (#5630) the "best" plan of an OPTION (RECOMPILE) statement or of a statement whose plans return very different row counts is the wrong plan to force. This step (A2) builds the one read that answers "may this plan be forced, and what should a person see first". Nothing calls it automatically yet.

What changes

New Darling MCP read get_plan_force_precheck (inputs: database_name, query_id, plan_id, optional compare_plan_id, server_name, hours_back, as_of). Store-only, read-only. Verdict is refuse or no_refusal_found, never "safe". Output has refusals (stable code, one sentence, evidence), shown, unknown.

Refusal codes: the force gate's own blocker names via ForcePlanBotPolicy.Blockers (plus state_unavailable, apc_enabled_for_database, apc_names_this_plan_as_regressed), recompile_hint, temp_table, query_store_read_only, not_run_recently (constant NotRunRecentlyAfter, 2 h), too_few_executions (floor taken from ForcePlanBotSettings.Default.MinReviewExecutions, 25, the same floor as PLAN_REGRESSION's per-plan group), psp_variant, output_differs (PlanInputComparison.OutputGrowthRatio and RowFloor).

shown: both plans' executions, CPU and duration per execution, average rows, last execution, forced flag, failure count and reason, forcing type. It also holds the compiled values under the compiled_value key (masked under maskLiterals, withheld under metadataOnly, never logged), and PlanInputComparison.JudgePlans as a fact (never a refusal). It lists the indexes the candidate plan names that are not in the newest index_object_stats capture ("not in the latest capture", because the capture may not list every index, as the tool description says). It lists agent job and step names that mention forcing (name match only, no step command in the store).

Reuse, not copy: the history tool's raw dedupe CTE and its interval-table FROM/WHERE are now shared constants (HistoryDedupeCte, HistoryTableFromWhere), and its tier gate is a generic TryReadFromTableAsync<T>. HistorySql and HistoryTableSql are byte-identical to before.

Registered: MCP host, web route and catalog entry, WebReadReach (query_store), plain-text.js MCP_TOOL_NAMES, DarlingMcpToolClassification, CatalogCensus (206 / 99 unique), the web IDENTITY read list (the read names one plan, like the history read). No schema change, no new collector read, nothing writes to a monitored server, no Lite twin.

Gate target: when the query has a current PLAN_REGRESSION finding, the target is that finding's own (FactRemediation.ExtractPlanRegressionTargets output, persisted on the finding) with PlanId set to the plan asked about. With no finding the target is built from the store rows: database, query, plan, BestPlanLastSeenUtc = the plan's last run. Defaults: ReplicaRole null, ParameterSensitivityCoFired false, RegressionFactor 0, hashes and CPU per execution unset (display only). Because those two blockers cannot be answered without a finding, parameter_sensitivity_cofired and secondary_replica_evidence are listed under unknown in that case. enablementIsCollected is false (the read never asks the collector catalog), so apc_enablement_unknown cannot fire. See the follow-ups.

Query Store state matches get_query_store_health: actual_state other than READ_WRITE refuses, with the decoded readonly_reason. No row at or before as_of is unknown.

Test plan

  • PlanForcePrecheckLiveTests (8 pins) with VM_PG=1: Total: 8, Errors: 0, Failed: 0 (first run, HEAD 88ce0ce).
    • PLAN_REGRESSION compares plans compiled for different inputs, and its advice to force the "best" plan makes the slow case slower #5630 pattern 1 (OPTION (RECOMPILE) -> recompile_hint), and pattern 2 (about 26,000 vs 50 rows per call -> output_differs)
    • a clean candidate (no check of this read fires, READ_ONLY check is unknown), and a gate blocker (already_forced)
    • not_run_recently + too_few_executions, temp_table (a @table variable does not fire), and psp_variant (siblings go to unknown)
    • query_store_read_only with a READ_ONLY row, and a database with no health row is unknown, not a refusal
    • different compiled values: no refusal, shown, masked to ? under maskLiterals, absent under metadataOnly
  • Proof each new pin fails with its check removed: a scratch branch (deleted) gated the seven new checks behind a false flag: Total: 8, Failed: 5 (recompile, output_differs, run-stopped/thin, temp/PSP, read-only). The clean-candidate, gate-blocker and compiled-value pins do not depend on those checks by design.
  • Contract and census classes plus every class that names a file I changed, VM_PG=1, merged with dev: Total: 845, Errors: 0, Failed: 0 (HEAD 598745a). Includes McpToolGuide, McpReadCommandTimeout, McpPayloadContractCensus, DatabaseNameParamTwin, EngineCapabilityMiss, WebRouteStatementCensus, DarlingWebEndpointsTests, StorageCommandTimeout, WebReadReach, WebDatabaseFilterReads, DarlingMcpWritePolicy, DarlingMcpInstructions, McpToolsListBudget, DarlingMcpHostTests, DarlingMcpToolsTests, QueryStoreHistory* and ForcePlan.
  • Mac build of Darling.Tests: 0 warnings, 0 errors.
  • Full suite: the seat's verify-full.sh at cdc715684. Darling.Tests Total: 34085, Errors: 0, Failed: 0, Skipped: 3922, Not Run: 1. Lite.Tests Total: 10703, Errors: 0, Failed: 0, Skipped: 5. The first run at 598745af3 failed 2 census pins that count every web read (WebReadCancellationPinTests needed plan_id in its cancelled request, and WebDatabaseFilterBehaviourTests counts ten identity reads now). The seat fixed both in cdc715684.

Source-text pins I updated: WebDatabaseFilterReadsTests (identity reads nine -> ten) and McpPayloadContractCensusTests (unknown rostered as a section name, not a severity band). To dodge the cut-key census, the answer is a dictionary, because the census reads an anonymous-object shown = member as a page cut.

Where the readers' rows come from in the running app: query_store_stats / query_store_interval_wide (Query Store collectors), query_store_text, query_plan_dim via query_store_plan_map, query_store_health (hourly collector), index_object_stats (daily), job_history, and plan_correction / findings for the gate.

CHANGELOG entry

SECTION: Added
ENTRY: - An MCP read checks a Query Store plan before anyone forces it ([#5632]) - get_plan_force_precheck runs the force gate and refuses a plan when its statement recompiles, uses a temporary table or has stopped running, when the plan is a Parameter Sensitive Plan variant, when Query Store is read-only, or when the two plans return very different row counts. It shows the compiled values, row counts and missing indexes a person needs before forcing. Nothing forces a plan yet.
REF: [#5632]: #5632

Deferred and follow-ups

Deferred (in this PR but unpinned, or not built):

  • No live pin for the "indexes not in the latest capture" and "job name match" facts. They are implemented and the build covers them. A pin needs index_object_stats and job_history seeds.
  • PSP sibling-variant naming: the store keeps no parent query id for a variant, so unknown says so (psp_variant_siblings).
  • Lite twin of the tool (Darling only by brief). The LiteToolNames list is untouched.
  • The web page does not use the route yet (it is in the read catalog only).

Follow-ups proposed:

  • Pass enablementIsCollected from CollectorCatalog.AppliesTo(PlanCorrectionCollector.Instance, target) so apc_enablement_unknown can fire here as it does for the bot.
  • A Lite twin once Darling's merges.
  • Docs tool counts: docs/mcp-server.md (204) and docs/editions.md (202) already trailed the code's 205 before this PR. They are not touched here.
  • Seat review, two small display points left for the Lite twin round. When the finding read throws, parameter_sensitivity_cofired is listed twice under unknown. A stored interval with no CPU counts as 0 CPU in the shown CPU per execution. Neither changes a refusal.

scratch added 4 commits October 18, 2026 04:56
WebReadCancellationPinTests: the cancelled request now carries plan_id, so the new route reaches its store call instead of returning a missing-parameter envelope.
WebDatabaseFilterBehaviourTests: the runtime census has ten identity reads, not nine.
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.

1 participant