[erikdarlingdata]

Claude posting for Erik Darling.

Fixes #6569.

Slice 1: recompile a module. Slice 2a (below): create a plan guide, with Undo. Slice 2b (drop_plan_guide and its Undo, a re-create from the journaled definition) is also in, so all slices are on this branch.

Why

An agent that finds a stale plan on a procedure, function or trigger had to run sp_recompile outside Darling, with no journal row. Slice 1 adds that as a Level 3 server action in the existing shape (the closest model is evict_cached_plan).

What changes

• Action recompile_module, tool recompile_module (DarlingMcpPlanForceTools.RecompileModule.cs, a new file of the same partial class, so it shares the answer and refusal helpers and DarlingMcpPlanForceTools.cs changes by one word, partial). Inputs: database_name, module_name (schema.name), server_name, agent_action, dry_run (default true).
• Gate: agent_actions_enabled, a Level 3 request, the per-server recompile_module switch, the remediation login, through ServerActionGate like the others. A dry run is gated too.
• Executor (SqlServerRemediationExecutor.RecompileModule.cs): one batch in the request's database resolves the name with OBJECT_ID(@p0), reads the type and HAS_PERMS_BY_NAME on the resolved schema and name, and (real run only) runs sp_recompile on that same object. Refusals, nothing run: bad_module_name, bad_database, module_not_found, not_a_module (anything but P, PC, FN, IF, TF, FS, FT, TR), recompile_permission_denied, permit_mismatch, plus postgres_target (before the gate, like evict). The write is EXEC sys.sp_recompile @objname = @resolved; inside that batch, with the input name as a typed parameter (@p0), never batch text. A lost reply is outcome_unknown as for the other writes. The name is normalized to [schema].[name] (exactly two parts, each at most 128 characters) and that form is the permit's id.
• Journal: action_called (before = database, module, object type), action_done or action_failed, kind module_recompile, an Agent changes row with no Undo (not in UndoableKinds), answer undo: null.
• Switch, everywhere the three others are: ServerActionNames (All, Label, engine validity: SQL Server only), ServerActionForKind, the refusal sentence, set_server_action / get_server_actions text, the Viewer (switch label, warning text, Agent changes kind word), the web switch page and Agent changes page, plain-text.js, the write policy and InvestigationRules agent-tool lists, the web-endpoint no-route list, the brief composer's Level 3 sentence, AGENTS.example.md, README, docs/mcp-server.md, and the grants table in docs/darling-agent-dispatch.md (ALTER on the object).
• Migration V218 (server-action-recompile-module) widens ck_config_server_actions_action to add recompile_module, guarded so a second run replaces nothing; it inserts no row (starts off everywhere). Viewer probe sentinel (CHECK definition names the word), MapProbedSchemaVersion arm, GetBoolean(190), StorageVersion.SchemaVersion = 215, a rewind entry in RungRewindArtifacts, and a census entry for the rung in MigrationDataMovingRungCensusPins.
• Instructions census constant: 254 tools / 142 unique to Darling (on top of dev's 253 after Darling MCP can set a collector's database scope and reset a scope's schedules (#6396) #6616).

Migration rung: V218. Dev's top is V217 (V215 agent-table indexes, V216 client timeouts on by default, V217 query store hints), and TheLadder_IsDenseAboveTheHistoricalGap fails on a gap. If dev takes more rungs first, this PR renumbers (mechanical: the Migration(...) line, StorageVersion, the sentinel arm and GetBoolean index, the rewind and census entries). sp_recompile exists on SQL Server and Azure SQL Database per Microsoft's documentation; I did not run against an Azure SQL Database (no target here), so the Azure claim rests on the documentation.

Test plan

• Build Darling.Tests -m:1: 0 errors (the 5 warnings are in files this PR does not touch).
• Unit (RecompileModuleTests): name normalization (plain, bracketed, dotted inside brackets, ]], a quote), refusals for one-part, three-part, empty, unbalanced, control characters and over-128 names, statement text with a doubled quote, the refusal order and the allowed type set, permit mismatch (other action, other server, other object, no permit) with nothing run, the switch lists, journal kind and no undo.
• Live SQL Server, sql2019 only (PlanForceToolLiveTests.Recompile.cs, throwaway database and procedure, dropped after): dry run shows USE [db]; EXEC sys.sp_recompile @objname = N'[dbo].[p]'; and runs nothing; a real run drops the procedure's cached plan (dm_exec_procedure_stats execution count 3 before, 0 after; the observable marker), journals action_called/action_done with the database and [dbo].[p] target, answers undo: null and the journal id, no Undo on the Agent changes row; names with a quote or bracket resolve through the parameter to module_not_found and dbo.t is intact; a table is not_a_module; a missing name or another database is module_not_found; DENY ALTER gives recompile_permission_denied and the plan stays; PostgreSQL target; switch off; no Level 3; actions off; no credential: each refused with nothing run. All pass.
• Live PostgreSQL (own throwaway rig, stopped after): the rung accepts recompile_module, still refuses drop_database by constraint name, a second run keeps the same constraint, and a store rewound one rung refuses the word until the rung re-runs.
• Revert-proved: with the widened CHECK reverted the rung test fails with 23514 ... ck_config_server_actions_action; restored.
• Ran (green): ServerActionGateTests, ServerActionSwitch* (store and write-live), PlanEvictTests, PlanForceToolLiveTests (all, on sql2019), the migration ladder and data-moving census, every *Census* and *Ratchet*, DarlingMcpInstructions*, DarlingMcpWritePolicy*, DarlingMcpInvestigationToolsTests, PlanForceNoWritePathTests, McpToolGuide*, ViewerSchemaVersionGateTests. Test expectations I had to move because they list tools or files: the served-writes list, the owner-source method list, the "executor named in one service file" list (now two files), and the tool counts.
• One failure not mine as far as I can tell, not verified on dev: PlanForceToolLiveTests.AnUndo_OfAnotherKind_OrAnUnknownCall_OrABadLogin_IsRefusedWithoutTouchingTheServer expected bad_login, got not_found (the test's SQL login on this laptop is a 2-character name). The undo path is untouched by this PR.
• Not run: the full suite, Lite.Tests, pre-push.sh (brief says CI decides); an Azure SQL Database.

Considered, not added

• An excluded-database check on force and evict (the Azure kill and recompile_module have one; force and evict do not).
• A refusal for database triggers and other non-schema-scoped objects: OBJECT_ID cannot see them, so they come back as module_not_found.

What the coordinator should double-check

• The rung number (V218 here; renumbered if dev takes more rungs first).
• DarlingMcpPlanForceTools became partial; the tool method is the last member of the new file on purpose (the payload-contract census reads a tool's span to the next tool or end of file, and the body's own catch must not fall inside the one-line tool method).

Slice 2a: create_plan_guide with Undo

• Slice 2a done on this branch. [x] Slice 2b (drop and its Undo) done, see ## Slice 2b.
• Tool create_plan_guide (DarlingMcpPlanForceTools.PlanGuide.cs, executor SqlServerRemediationExecutor.PlanGuide.cs). Arguments database_name, name, statement, type (OBJECT or SQL; TEMPLATE is refused as bad_guide_type), module_or_batch, params, hints (must be an OPTION (...) clause). Every value is a typed parameter of sys.sp_create_plan_guide; none joins batch text. Same gate as recompile_module (service switch, Level 3, per-server switch, excluded database, engine; PostgreSQL refused as postgres_target). Dry run is the default.
• Match before create: OBJECT: the module must resolve to a procedure, scalar or multi-statement table-valued function, or DML trigger (not_a_module otherwise) and statement must appear in OBJECT_DEFINITION byte for byte (binary collation; statement_not_in_module). SQL: counts cached batches compiled in the guide's own database that equal the batch (or the statement) and reports it; zero is allowed and the answer says it matches nothing cached right now; null when the login cannot read the cache. A name already in the database is guide_exists. All refused before anything runs.
• Validate after create: the same batch runs sys.fn_validate_plan_guide on the new guide; a row means the guide is dropped in that batch and the call is failed / guide_invalid with the server's message, journaled action_failed.
• Journal and Undo: kind plan_guide_create; action_called holds the whole definition (before.guide: name, type, statement, module_or_batch, params, hints; before.database); target.id is the guide name. The Agent changes row has Undo through the existing plan-undo path (UndoPlanChangeAsync, web route, Viewer command and undo_plan_change all reach it): show the statement, confirm, drop with sys.sp_control_plan_guide N'DROP', journal action_undone as the person. It is gated by that server's create_plan_guide switch and login. A guide already gone is refused guide_not_found and changes nothing; a guide that no longer holds the journaled statement and hints (replaced under the same name) is refused guide_changed and not dropped.
• For 2b: SqlServerRemediationExecutor.DropPlanGuideAsync(permit, name, expected, dryRun) already exists (accepts a drop_plan_guide permit) and PlanGuideDefinition is the journaled shape; the create path takes a PlanGuideDefinition, so 2b can re-create from the journal.
• Switches: create_plan_guide and drop_plan_guide are in V218's CHECK (no new rung). The rung's guard, the Viewer probe sentinel and the test rewind now key on drop_plan_guide, the newest word, so a dev store that already ran the earlier V218 would still widen. Both are in ServerActionNames.All, the labels, the Viewer and web switch lists. The drop_plan_guide switch is read by the slice 2b tool. create_plan_guide is in the agent-action and write-policy lists and the web unserved list; the instructions census is 255 / 143.
• Permissions (matched to Microsoft Learn): sp_create_plan_guide Permissions: an OBJECT guide needs ALTER on the referenced object; a SQL or TEMPLATE guide needs ALTER on the current database. sp_control_plan_guide Permissions: an OBJECT guide needs ALTER on the referenced object; all others ALTER DATABASE. The create and drop batches now check only the grant that applies (the other is reported as satisfied), the refusal sentence names the one that is missing (does not hold ALTER on <module> for OBJECT, does not hold ALTER on the database <db> for SQL), and the grants row in docs/darling-agent-dispatch.md says the same, with the undo following the drop split. Live test ALoginWithAlterOnTheProcedureOnly_CreatesAnObjectGuide_AndIsRefusedASqlGuideWithThePermissionSentence: the test login is taken out of db_owner and granted ALTER on the procedure only; a SQL guide is refused guide_permission_denied (dry run and write, nothing created), an OBJECT guide is created, and the person's undo drops it.
• Engine (checked on Microsoft Learn): the sp_create_plan_guide page says "Applies to: SQL Server, Azure SQL Database, Azure SQL Managed Instance, SQL database in Microsoft Fabric". The sp_control_plan_guide page says "Applies to: SQL Server" only. This tool needs both (it drops an invalid guide with sp_control_plan_guide, and the Undo drops with it), so the claim is narrowed to SQL Server: an Azure SQL Database (edition 5) or Managed Instance (edition 8) target is refused before the gate as engine_not_supported, next to the PostgreSQL refusal, with the reason in the tool description, and the docs say SQL Server only. Live test AGuideOnAnAzureSqlTarget_IsRefusedAsEngineNotSupported_ForADryRunAndAWrite (both editions, nothing created, nothing journaled). Links: https://learn.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-create-plan-guide-transact-sql and https://learn.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-control-plan-guide-transact-sql
• Tests: PlanGuideTests (argument checks, refusal order, statement text, permit mismatch, switch lists, rung accepts both names and still refuses an unknown one: unit, plus a store fact that needs DARLING_TEST_PG), PlanForceToolLiveTests.PlanGuide.cs (live: dry run, OBJECT create plus journal plus Undo, Undo of a gone or replaced guide, SQL guide with zero matches, quote in the statement, validation failure, refusals, excluded database, PostgreSQL target, switch/level/actions off).
• Build Darling.Tests -m:1: 0 errors.
• Ran green on sql2019 only (own throwaway PostgreSQL on port 55691, stopped after; scratch databases and the temporary sysadmin login I made on sql2019 were dropped, none left): PlanForceToolLiveTests (all parts), PlanGuideTests (the V218 store fact ran, not skipped), RecompileModuleTests, ServerActionGateTests, PlanForceNoWritePathTests, *Migration*, RungRewind*, ViewerSchemaVersion*, DarlingMcpWritePolicy*, DarlingMcpInstructions*, McpPayloadContract*, *RemediationExecutor*, every *Census* and *Ratchet* class: 977 run, 1 failed (a wrong expectation in my own live test, fixed, see below); after the fix the guide live tests plus PlanGuideTests ran 34, 0 failed. Not run: the full suite, Lite.Tests, pre-push.sh (CI decides).
• What the server did that the first batches guessed at (measured on SQL Server 2019 with a scratch database):
  • sys.fn_validate_plan_guide returns msgnum, severity, state, message: the batch reads message, which is right. A valid guide returns no row.
  • sys.plan_guides.query_text and hints read back byte-identical to what was sent, including tabs, a line break, runs of spaces and OPTION (RECOMPILE, MAXDOP 1); so the guide_changed check needed no change.
  • A mismatched USE PLAN is NOT refused by sp_create_plan_guide: the guide is created, and sys.fn_validate_plan_guide then returns message 8698 ("USE PLAN hint contains plan that could not be verified to be legal for query"). So the batch's validate-then-drop path is the one that fires. AGuideThatFailsValidation_IsNotLeftBehind_AndIsJournaledFailed now asserts guide_invalid, the server's message, no guide left, one action_failed and no action_done (before it accepted any failed).
  • A second guide on a statement an enabled guide already matches is refused by the server itself (error 10502) inside the batch.
• Defects the live run found, all in my tests, not the batches: the SQL-guide message assertion looked for a sentence the code does not say (now No cached batch in this database matches this guide); the cached-count test ran the statement with a trailing ; while the count is of byte-equal batch text (now run without it); the quote test expected it''s where the statement's own doubled quote doubles again (it''''s).
• Revert-proof: with the OBJECT statement-match refusal disabled (if (false && !statementInModule)), AStatementNotInTheModule_AndATakenName_AndATemplate_AreRefused_AndNothingRuns fails (Expected "refused", Actual "dry_run"); restored, it passes.
• Considered, not added: counting cached batches the way the server matches (it ignores whitespace, comments and trailing semicolons; the count is byte-equal text, and the message says "equal", so it can undercount a batch that differs only by a trailing semicolon). No concrete harm: the guide is created either way and the count is informational.
• Considered, not added: a re-check of the switch inside the Write hook beyond what RunAgentActionAsync does for the other kinds (no concrete trigger beyond the existing one); a size cap per field tighter than 4,000,000 characters (a USE PLAN hint carries a whole plan).

Slice 2b: drop_plan_guide with Undo

• Tool drop_plan_guide (arguments database_name and name), in DarlingMcpPlanForceTools.PlanGuide.cs, same gate as 2a (service switch, Level 3, per-server drop_plan_guide switch, excluded database, engine; PostgreSQL postgres_target, Azure SQL Database and Managed Instance engine_not_supported). Dry run by default. DropPlanGuideAsync (2a) now serves both: with a drop_plan_guide permit it is the drop action. In ONE batch on the request's database it reads the guide from sys.plan_guides (query text, scope type, the module's two-part name for OBJECT, scope batch, parameters, hints, is_disabled), refuses guide_not_found and a TEMPLATE guide (new reason guide_template, because Undo cannot re-create one), checks the grant for the type, then runs sys.sp_control_plan_guide N'DROP', @name. Every value is a typed parameter. The dry run shows the statement and the definition that would be journaled; the write passes that definition back as expected, so a guide replaced between the dry run and the write is refused guide_changed.
• Journal: kind plan_guide_drop; action_called holds before.database and before.guide (name, type, statement, module_or_batch, params, hints, is_disabled), target.id is the guide name, then action_done / action_failed; an Agent changes row with Undo (the plan-undo path the web route, the Viewer and undo_plan_change all reach; IsPlanChange now includes the kind).
• Undo (DecideAndRunGuideRecreateUndoAsync): show the create statement (plus the DISABLE statement when it was disabled), confirm, then re-create through 2a's CreatePlanGuideAsync (same match checks, validate-then-drop-on-failure), disabling it after validation in the same batch when it was disabled. A name that exists now is guide_exists: nothing changes. It is gated by that server's drop_plan_guide switch and remediation login (the switch of the action being undone, as 2a's undo uses create_plan_guide); CreatePlanGuideAsync accepts a permit of either action for that.
• Finding from the live run: a login with ALTER on the procedure only does not see a SQL guide in sys.plan_guides at all, so the drop read it as not found. The batch now reads the database grant first; a guide that is not found for a login without ALTER on the database is refused guide_permission_denied with the permission sentence ("cannot see it: does not hold ALTER on the database"), not guide_not_found. A login with the grant still gets guide_not_found. The same applies to 2a's undo of a create for such a login (it had no test).
• "Stored but no tool reads it" removed from this body, the dispatch doc, and the comments. Docs: dispatch doc bullet, grants row, action list; docs/mcp-server.md (six acting tools, agent-action list); AGENTS.example.md; the set_server_action / get_server_actions text and the brief composer's Level 3 sentence now name create_plan_guide and drop_plan_guide (2a had left them out). Lists: write policy, InvestigationRules, web no-route list, plain-text.js, Agent changes kind words (web and Viewer). Instructions census 256 tools / 144 unique to Darling.
• 2a defect found by the wider run, fixed: McpToolGuideTests.EveryConvertedHead_StaysAtOrUnder620Characters failed on create_plan_guide (629 characters); the head is shortened.
• Build Darling.Tests -m:1: 0 errors; the one warning is in ChangeWatchDetectionLiveTests.cs, which this PR does not touch.
• Live on sql2019 (PlanForceToolLiveTests.PlanGuideDrop.cs, throwaway database each, own PostgreSQL on port 55692 stopped after): dry run (nothing dropped, nothing journaled, default true); an OBJECT guide and a disabled SQL guide with parameters dropped, gone, journal holds the definition, Undo shows then re-creates and the sys.plan_guides row (text, scope, scope batch, parameters, hints, is_disabled, scope object id) equals the original; Undo while a same-name guide exists is guide_exists and the replacement is untouched; guide not found and TEMPLATE refused (dry run and write), TEMPLATE survives; with ALTER on the procedure only the OBJECT guide drops (and its Undo re-creates it) and the SQL guide is refused with the permission sentence; switch off, no Level 3, actions off, excluded database, PostgreSQL target, Azure editions 5 and 8: each refused, guide still there, nothing journaled. Plus unit tests in PlanGuideTests (DISABLE statement text, name check, lists).
• Revert-proved: with the TEMPLATE refusal disabled, AGuideThatIsNotThere_AndATemplateGuide_AreRefused_AndNothingIsDropped fails (Expected "refused", Actual "dry_run"); restored, it passes.
• Ran: PlanForceToolLiveTests (all), PlanGuideTests, RecompileModuleTests, *ServerAction*, PlanForceNoWritePathTests, *AgentChangeUndo*, *Migration*, *RungRewind*, *WritePolicy*, *NeverAgent*, *Instructions*, *PayloadContract*, every *Census* and *Ratchet*, *InvestigationBrief*, *InvestigationRules*, *ViewerSchema*, *McpToolGuide*, *InvestigationTools*, *WebEndpoints*: 1600 run, 1 failed (the 620-character head above, fixed; McpToolGuideTests, PlanGuideTests and DarlingMcpInstructions* re-run after: 45 run, 0 failed).
• Not run: the full suite, Lite.Tests, pre-push.sh (brief says CI decides).
• The temporary sysadmin login I made on sql2019 for the live setup was dropped; no scratch database is left.
• Considered, not added: comparing params and is_disabled between the dry run and the write (the guide_changed check compares statement and hints only; no concrete trigger). A guard for an OBJECT guide whose module no longer resolves (the module name reads null, the grant check refuses it, so nothing is dropped).

CHANGELOG entry

SECTION: Added
ENTRY:

• An agent can recompile a procedure, function or trigger ([Darling server actions: recompile a module, and create or drop a plan guide #6569]) - New Level 3 server action recompile_module runs sp_recompile on one module in the request's database through the remediation login, behind its own per-server switch (off by default; migration V218 widens the switch table), dry run by default, journaled with no undo. It also adds create_plan_guide, a Level 3 server action that creates one OBJECT, SQL or TEMPLATE plan guide (with OPTION hints, an XML showplan or no hints, whatever sp_create_plan_guide takes) with sp_create_plan_guide after matching it and validating it (an invalid guide is dropped again), journaling the whole definition, with an Undo from Agent changes that drops the guide by name; its switch and the drop_plan_guide switch are in migration V218. It also adds drop_plan_guide, a Level 3 server action that reads one guide's whole definition from sys.plan_guides and drops it with sp_control_plan_guide (a missing guide is refused; every guide form is dropped, TEMPLATE included), journaling the definition, with an Undo from Agent changes that re-creates the guide in the same form (validated, and disabled again if it was disabled) and is refused if a guide with that name exists by then. A database the server's entry excludes is refused, for a dry run too. An Undo of a plan guide create or drop acts only on a row an agent's create or drop tool wrote, applies the create's checks to the journaled definition, and refuses an excluded database or an Azure SQL or PostgreSQL target. A SQL guide's cached-batch count reads only its own database, and a permission refusal for a guide on a DML trigger names the trigger's table. Self-managed stores get the updated journal rule at the service's next start, or by re-running provision-roles.sql.
  REF:
  [Darling server actions: recompile a module, and create or drop a plan guide #6569]: Agents can recompile a module and create or drop a plan guide through Level 3 server actions, with Undo (#6569) #6635

Round 1 review

1. A database excluded in the server's entry is refused. Fixed (commit 8d88331; test ADatabaseTheServersEntryExcludes_IsRefused_ForADryRunAndAWrite_AndNothingIsReadOrRun). recompile_module now checks the server entry's excluded list with the same helper and the same sentence as the Azure kill ("excluded in this server's entry, so Darling does not act in it"), reason database_excluded, after the gate and before the dry run reads anything. The test checks the dry run and the real run: both refused, the procedure keeps its three cached runs, no journal call row. Reverting the check makes the test fail.
2. The name parameter fits every valid two-part name, and the journal names the object that ran. Fixed (same commit; test AValidTwoPartNameLongerThan256Characters_Resolves_AndRuns_AndTheJournalNamesTheResolvedObject, plus ANameOverTheLimit_IsRefusedBeforeTheServerIsReached and a unit assertion that the longest normal form is 517 characters). @p0 is sized ModuleRecompileStatement.MaxNormalLength (517), not 256. The answer's before and the journal's action_called.before now carry schema, name and module as the server resolved them (OBJECT_SCHEMA_NAME and o.name of the resolved id), and the done message names the resolved object. The test uses a 128-character schema and a 128-character procedure (261 characters normalized) on sql2019, past the old 256. Reverting the size to 256 makes it fail. Note: sql2019's server collation is case sensitive, so the test cannot show a name that differs from the input only in case; the journal assertions there check the resolved parts for the long name.
3. The check and the recompile run in one batch. Fixed (same commit). The facts read and sys.sp_recompile are one statement batch: it resolves the id, checks the type is one of the accepted codes and that the login holds ALTER, and runs sp_recompile on that object's schema and name only when @run is 1 and both hold. It returns found, type, grant, resolved schema and name, and whether it ran. The dry run uses the same batch with @run 0 and still shows the exact statement. All existing live recompile tests pass; the table-name test still refuses not_a_module with nothing run. A lost reply on a real run is still outcome_unknown.
4. The grant the docs name is the one that works. Fixed (same commit; docs and refusal text, no behavior test needed). docs/darling-agent-dispatch.md and the recompile_permission_denied sentence now say: a procedure or function needs ALTER on the module; a DML trigger needs ALTER on its table, which also allows changing and emptying that table, so grant it only where that is acceptable.
5. A live undo test no longer depends on journal row id 1. Fixed (same commit; AnUndo_OfAnotherKind_OrAnUnknownCall_OrABadLogin_IsRefusedWithoutTouchingTheServer). The bad-login case uses a journal row the test seeds. Also found while fixing it: "bad login!" is a valid person name (inner spaces and ! are allowed since the name rules changed), so the old case did not test the login refusal at all; it now uses a login with a control character. The class passes with the test run alone.

Migration: dev's top rung is V217 (V215 agent-table indexes, V216 client timeouts on by default, V217 query store hints), so this PR's rung is V218 (renumbered from V215 in the dev merge below).

Tests run (rig on its own throwaway PostgreSQL, sql2019 only, throwaway databases dropped; none left): PlanForceToolLiveTests (all parts), ServerActionGate*, PlanEvict*, *Recompile*, *Migration*, DarlingMcpWritePolicy*, DarlingMcpInstructions*, McpPayloadContract*, *RemediationExecutor*, every *Census* and *Ratchet* class: 931 run, 0 failed. After the last merge of dev (docs and write-policy text only) the build is 0 errors and the recompile, write-policy, payload and instructions classes pass except one class that needs the PostgreSQL rig, which I had already stopped (it passed in the full run before). Not run: the full suite and pre-push.sh (CI decides). The build shows 8 warnings, all in files this PR does not touch (dev's own).

Considered, not added: refusing an over-long input separately (the parser already refuses a part over 128 characters before the server is reached, test added); a retry on a lost reply (the existing outcome_unknown answer already says running it again does no harm).

Review round 2

Claude posting for Erik Darling.

Commit beafc0b. Four items, each Fixed.

1. Both plan guide Undos apply the create's checks, and only to a row an agent action wrote. Fixed (DarlingMcpPlanForceTools.PlanGuide.cs, AgentChangeUndo.PlanUndo.cs).
   • (a) The re-create Undo rebuilds the definition with PlanGuideDefinition.TryCreate, the call the create tool uses, so hints that do not start with OPTION (, an OBJECT module that does not normalize to [schema].[name], a TEMPLATE type and over-length text are refused as not_undoable. Both Undos run the name through PlanGuideDefinition.TryName and refuse a name over 128 characters instead of letting NVarChar(128) cut it.
   • (b) Both Undos, once the server row is read, refuse an excluded database (database_excluded), an Azure SQL Database or Managed Instance (edition 5 or 8, engine_not_supported) and a PostgreSQL target (postgres_target), each recorded with session.RecordRefusalAsync.
   • (c) ReadPlanFactsAsync, for the two guide kinds, refuses not_undoable unless the journal row's actor is agent:... (the create and drop tools always record "agent:" + action.AgentId in RunAgentActionAsync, for every token), the request row exists with the same server_id as the journal row, and the request's stamped level is 3 or more. I checked the caller paths: the actor shape does not depend on the token, so there is no person actor to keep undoable for these two kinds.
   • Tests (PlanForceToolLiveTests.PlanGuideUndoChecks.cs, live, sql2019 and own PostgreSQL): TheUndoOfADropWhoseJournaledDefinitionTheCreateRefuses_IsRefused_AndNothingRuns (bad hints, module that does not normalize, TEMPLATE, name over 128; the real row still undoes), TheUndoOfACreateWhoseJournaledNameIsOver128Characters_IsRefused_AndNothingRuns, TheUndoOfAGuideCreateOrDrop_OnAnExcludedDatabase_AnAzureTarget_OrAPostgreSqlTarget_IsRefused_AndRecorded (four cases, both kinds, refusal row checked), AGuideRowAnAgentActionDidNotWrite_IsNotUndone_ButARealCreateAndDropStillAre (person actor, request on another server, request with no row, request not Level 3; real rows still undo). Revert-proved (c): with the check disabled the last test fails; restored.
2. Every hint stays allowed; the tool description says which hints change results. Fixed, description only. After the <<GUIDE>> marker, the create_plan_guide text now says table hints that change isolation or locking (NOLOCK, READUNCOMMITTED, READPAST, TABLOCKX, XLOCK and the like) change which rows the query returns or how it blocks other sessions, and an agent uses them only when the person asked or after comparing the query's results and run time with and without the hint, stating what it compared. The head before the marker is unchanged. No hint list, no code change, no test.
3. The SQL guide match count reads only the guide's own database. Fixed. The create batch counts with AND st.dbid = DB_ID(). The count text (MatchWords, the tool description, RemediationResult.CachedMatches, the dispatch doc) now says "in this database". Test ASqlGuideCountsOnlyTheBatchesCachedInItsOwnDatabase: a batch cached only in a second database counts 0, then counts once it is cached in the guide's database. PlanGuideTests and the one live message assertion were updated for the new sentence.
4. Grant text says what each grant allows, and names a trigger's table. Fixed. docs/darling-agent-dispatch.md: the force_plan, create and drop rows say a procedure or function guide needs ALTER on it (which also lets the login change its code), a DML trigger guide needs ALTER on the trigger's table (ALTER cannot be granted on a trigger; also lets the login alter or truncate the table), and a SQL guide needs ALTER on the database (also lets the login create users, change roles and schemas, and add database DDL triggers: grant it only where that is acceptable). Both batches now read the trigger's parent table (OBJECT_SCHEMA_NAME(parent_object_id), OBJECT_NAME(parent_object_id)), and the create and drop permission refusals name that table. Tests: unit TheRefusalForAnObjectGuideOnATrigger_NamesTheTriggersTable_AndOneOnAProcedureNamesTheProcedure; live ALoginWithoutAlterOnTheTriggersTable_IsRefusedAnObjectGuideOnTheTrigger_WithTheTableNamed (create and drop refused with the table named; after GRANT ALTER on the table both go through, so the grant the docs name is the one that works).

Ran (own PostgreSQL on port 55639, stopped after; sql2019 only, scratch databases and a temporary sysadmin login dropped after): PlanForceToolLiveTests (all parts), PlanGuideTests, RecompileModuleTests, ServerActionGateTests, PlanForceNoWritePathTests, *AgentChangeUndo*, *AgentChangeToolUndo*, *DarlingMcpWritePolicy*, DarlingMcpInstructionsBudget*, *McpToolGuide*, McpPayloadContract*, every *Census* and *Ratchet*, *PlanForceAction*, *AgentActionGate*: 1020 run, 1 failed (PlanGuideTests.TheMatchWords_SayWhatAGuideAppliesTo, a message assertion I had not updated; fixed in beafc0b and rerun green). The first census run caught DROP DATABASE text in my new live file (UnpooledDropConnectionCensusTests); the test now uses a second SqlScratch instead, and the census passes. Not run: the full suite and pre-push.sh (CI decides).

Considered, not added: a check that the live routes still send the request at Level 3 when the Undo runs. The Undo uses the level the request was stamped with when the tool ran; no concrete trigger for a route change making an earlier change wrong to undo.

Guide forms

Claude posting for Erik Darling.

The plan guide tools now take every guide form sp_create_plan_guide takes, so a dropped guide can always be undone. This replaces the earlier OPTION-only create checks and the TEMPLATE refusals in slices 2a and 2b and in review round 2 (the test names and revert-proof lines above that mention them are superseded by this section).

What changes

• PlanGuideDefinition.TryCreate (SqlServerRemediationExecutor.PlanGuide.cs): Hints is now nullable. Hints are an OPTION (...) clause as before, an XML showplan (text that starts with <, after an optional <?xml ...?> prologue), or none (empty or null becomes NULL: the guide removes the statement's own OPTION hints). NULL goes through the create parameter (DBNull.Value), PlanGuideStatement.Create prints NULL, and the journal image keeps it as JSON null. Type TEMPLATE is accepted: module_or_batch must be empty, params is required, hints are checked by SQL Server. Kept: the length limits, TryName, a bad type, an empty statement, OBJECT with params, a module that does not normalize. A TEMPLATE guide runs under the SQL guide permission (ALTER on the database, Microsoft's rule), the create batch skips the cached-batch count for it, and the result carries no count (MatchWords says no count applies).
• Drop (DropPlanGuideBatchSql and DropPlanGuideAsync): @refuse_template is gone and guide_template is removed, so the drop takes a TEMPLATE guide; the expected-definition compare is NULL-safe on both sides (hints that changed to or from NULL count as changed); the journaled image reads the type from scope (1 OBJECT, 2 SQL, 3 TEMPLATE), takes no batch for a TEMPLATE guide, and keeps NULL hints as NULL (it was turned into an empty string).
• Both Undos: the drop Undo re-creates any guide the drop journaled through TryCreate, and the create Undo's expected definition keeps NULL hints, so both still match the guide by statement and hints.
• Tool text (after the <<GUIDE>> marker, heads unchanged): create_plan_guide names the three hint forms with one line of guidance each and the TEMPLATE type (empty module, params required, PARAMETERIZATION hints, statement and params from sys.sp_get_query_template); hints is now optional. drop_plan_guide says every form is dropped and re-created by the Undo. docs/darling-agent-dispatch.md rows updated to match.

Tests

• Unit (PlanGuideTests): XML showplan hints (with and without prologue) for OBJECT and SQL, NULL hints for each type, TEMPLATE shape and its refusals (module present, params missing or blank), a bare < or a prologue with no body refused, NULL printed in the statement, no cached count for TEMPLATE. The old TEMPLATE-is-bad-type rows now expect the new argument refusals; no test deleted.
• Live on sql2019 and an own PostgreSQL (PlanForceToolLiveTests.PlanGuideForms.cs): AGuideWithXmlShowplanHints_NullHints_OrOfTypeTemplate_IsDropped_AndItsUndoRebuildsTheSameDefinition makes the three guides by hand, drops each with drop_plan_guide, Undoes each, and checks the whole sys.plan_guides row (text, scope, batch, parameters, hints, disabled, scope object) is the same again. ACreateWithXmlShowplanHints_NullHints_OrOfTypeTemplate_IsUndoneByDroppingTheGuide creates the three forms with create_plan_guide and Undoes each. AGuideThatIsNotThere_IsRefused_AndATemplateGuide_IsDropped_AndUndone replaces the old TEMPLATE refusal test.
• Revert-proved: with the OPTION-only hint check put back, both new live tests and the two unit XML tests fail; restored, they pass.
• Ran: PlanForceToolLiveTests (all parts), PlanGuideTests, RecompileModuleTests, ServerActionGateTests, PlanForceNoWritePathTests, *AgentChangeUndo*, *AgentChangeToolUndo*, *DarlingMcpWritePolicy*, DarlingMcpInstructionsBudget*, *McpToolGuide*, McpPayloadContract*, every *Census* and *Ratchet*, *PlanForceAction*, *AgentActionGate*: 1034 run. The one failure was McpToolGuideTests.EveryConvertedHead_StaysAtOrUnder620Characters (my first edit lengthened the drop_plan_guide head to 624); I put the head back and moved the sentence after the marker; that class and the budget, payload and write-policy classes then passed (345 run). Own PostgreSQL stopped after; sql2019 scratch databases and logins were dropped by the tests (checked: none left). Not run: the full suite and pre-push.sh (CI decides).

Considered, not added

• Checking the XML showplan beyond "starts with <": SQL Server parses it, and an invalid one fails the create or the post-create validation, which drops it. No trigger that the cheap check lets through something harmful.
• Checking that a TEMPLATE guide's hints are PARAMETERIZATION hints: SQL Server refuses others.
• An empty hints from an agent that forgot the argument creates a guide with NULL hints (it removes the statement's own OPTION hints). The tool text says so and the dry run shows hints: null; a dry run is the default.

CHANGELOG draft (the ## CHANGELOG entry above is extended): create_plan_guide and drop_plan_guide take every guide form SQL Server takes (OPTION hints, XML showplan hints, no hints, TEMPLATE guides), and the drop Undo re-creates any of them.

Double-check: the (object?)guide.Hints ?? DBNull.Value create parameter and the NULL-safe compare in DropPlanGuideBatchSql; the drop's journal image for a TEMPLATE guide (no batch).

Agent action rows

Claude posting for Erik Darling.

The service now writes the agent action rows in the investigation journal itself, and the store keeps those rows to the service. Before this, a plan guide or plan force Undo acted on the action_called row, but that row was written on the MCP tools' own login (mcp), which holds INSERT on the journal. So a row that no agent action wrote could sit in Agent changes and be offered for Undo. Now:

• action_called, action_done and action_failed are owner-only journal events, like server_action_switched (Agent dispatch: turn a server's agent action switches on and off from the viewer, the web and MCP #6269): the journal trigger refuses them from mcp and admin. The list is AgentActionEvents.OwnerOnly; JournalOwnerEventList appends it, and Darling/tools/provision-roles.sql has the same text (the trigger function and its step 3i header comment).
• InvestigationRequestStore takes an optional agentActionSource (the store owner's data source). RunAgentActionAsync runs every transaction on it: the request checks (token, claim, request, Level, server: unchanged), the action_called insert, the write hook's own transaction with its action_done or action_failed, and the late action_done. The store changes the Level 2 tools make (mute rules, custom alert rules, thresholds, signature merges) are in the same transaction, so they now run on the owner login too; the service's code decides what is written and the agent's arguments are validated as before. A store with no owner source (tests, the Viewer) runs on its own source.
• The plan tools (force, unforce, evict, kill, create and drop plan guide, recompile) pass the owner source they already hold. The mute, custom-rule and signature-merge tools get it from DarlingAgentActionSeat.Owner (the host builds the seat from StoreOwnerSource); set_alert_threshold_override takes StoreOwnerSource as a service parameter.
• refused and journal_capped stay writable by mcp and admin on purpose: the claim and submit tools journal them as mcp. RecordAgentActionRefusalAsync and CheckServerActionAsync therefore stay on the store's own source.
• Other writers checked: the web and the service's command executor already use the owner (postgres is the owner there); the Viewer never writes these events; Undo writes action_undone and agent_actions_switched (not moved). One path was not on the owner: the undo_plan_change MCP tool built its AgentChangeUndo on mcp, and an Undo whose reply was lost journals an action_called marker (kind plan_undo, best effort). It now builds it on the owner source it already requires (owner ?? postgres), so that marker is still written.
• Rows written before the upgrade stay as they are; nothing is migrated.
• A hand-provisioned store needs Darling/tools/provision-roles.sql run again to get the new trigger list. A managed store takes it at the next start.

CHANGELOG draft (extends the entry above)

Agent changes only list what the service itself journaled: the action_called, action_done and action_failed rows can no longer be written by the mcp or admin logins, only by the service. A store provisioned by hand needs provision-roles.sql run again. Rows written earlier are unchanged.

Test plan (this part)

• Build Darling.Tests succeeds (also after merging the branch's latest commit); the 7 warnings are in files this change does not touch.
• PostgreSQL rig, -class runs of *AgentLevel2Store*, *InvestigationRequestEventsLiveTests*, *DarlingManagedRolesTests*, *ProvisionRoles*, *InvestigationRequestStoreLiveTests*: 142 passed, 0 failed. They cover: the managed and the script copy of the trigger refuse each of the three events from the writer role; the real mcp and admin logins are refused on a managed and a script-provisioned store; the gate on mcp's own source stops at action_called with 42501 and writes nothing, and on the owner source journals action_called|action_done.
• *AgentChangeUndo*, *AgentActionGate*, *AgentLevel2*, *SignatureMerge*, *AgentChangeToolUndo*, *ThresholdOverride*, *Census*, *Ratchet*: 754 run, 2 failed: AgentLevel2ToolPolicyTests.TheInvestigationsArm_ServesExactlyTheThirteenActionTools_OfTheWrites and TheInstructionsCensus_UnderTheInvestigationsArm_CountsTheThirteenAsServed_.... The served roster now holds create_plan_guide at index 2 (the plan guide slices); this change adds and removes no tool. I did not run them on the branch tip before my change, so whoever owns the plan guide slices should confirm.
  • Update (finish lane): the two failures were stale expected lists, not a policy gap. The investigations arm already serves all sixteen agent action tools from InvestigationRules.AgentActionToolNames (the list force uses), including recompile_module, create_plan_guide and drop_plan_guide. Fixed in c7cb50a: both tests now list the sixteen and are renamed TheInvestigationsArm_ServesExactlyTheActionTools_OfTheWrites and TheInstructionsCensus_UnderTheInvestigationsArm_CountsTheActionToolsAsServed_AndStillSaysTheWritesAreOff; the instructions text now names recompile and plan guide beside plan force, plan evict and session kill (the intro sentence and the Level 3 clause); the count in the text is taken from the list, not typed.
• PlanForceToolLiveTests (all parts, including JournalOwner and PlanGuideForms), on sql2019 (login sa, no login created) and an own PostgreSQL rig: 123 run, 0 failed, 0 skipped (460 s). No pm_pf_* database or login left on sql2019, and no plan guide.
• Revert-proved: with the three event names taken out of both trigger texts (the managed JournalOwnerEventList concat and the provision-roles.sql list; AgentActionEvents.OwnerOnly left alone), *DarlingManagedRolesTests*, *ProvisionRoles*, *InvestigationRequestEventsLiveTests*, *AgentLevel2Store*, *InvestigationRequestStoreLiveTests*: 142 run, 4 failed (TheScriptsJournalTrigger_RefusesEveryEventKindAndMark_FromTheMcpRole, TheJournalTrigger_Refuses...FromTheMcpRole_AndTheServiceStillWrites, AgentLevel2StoreGrantLiveTests.TheBringYourOwnScript_..., ...TheManagedBatch_...). Restored; same classes pass.
• Finish lane, on the pushed tip: *AgentLevel2*, the events, roles, provision and store live classes, *AgentChangeUndo*, *AgentChangeToolUndo*, *PlanGuideTests*, *RecompileModuleTests*, *DarlingMcpWritePolicy*, *DarlingMcpInstructions*, *McpToolGuide*, *McpPayloadContract*, *McpToolsListBudget*, *AgentActionGate*, *ServerActionGate*, *PlanForceNoWritePath*, every *Census* and *Ratchet*: 1109 run, 2 failed on the first pass, both fixed in c7cb50a and rerun green (795 run, 0 failed): McpToolsListBudgetTests.ConvertedTools_HonorD2sAbsoluteCaps_AndNoMarkerReachesTheWire (the create_plan_guide hints parameter description was 264 against the 230 cap; shortened to 178, the three hint forms stay in the tool text after the marker) and PlanForceNoWritePathTests.OnlyTheForceToolClass_TakesTheStoreOwnerSource (its list now includes DarlingMcpThresholdOverrideTools.SetAlertThresholdOverride, which takes the owner source since the agent action rows change; the rule, that only action tools take it, holds).
• No full suite and no pre-push.sh (CI decides, per the brief).

Considered, not added

• A clean refusal for the Level 2 store tools (mute, custom rule, threshold, merge) when the worker has not yet published the owner source: with no owner source the call runs on mcp, the first action_called insert is refused by the trigger, and nothing is changed. The window is the first seconds after the service starts, before any agent can hold a claim.

Final review round

Claude posting for Erik Darling.

Four changes in behavior:

1. A store set up by the script gets the owner-only journal rule at start. The service's start already re-applied the password rules on a self-managed store. It now checks the journal's owner-row function in the same step, under the same provisioning lock and in the same transaction: when its text differs from what this build writes, it is replaced on the owner connection. A store upgraded by replacing the binaries therefore stops accepting action_called, action_done and action_failed from the mcp and admin logins at the next start. A store below V191 (no journal, or no practice_of) is left alone, as the script leaves it. A login that cannot replace the function gets the one existing warning, which now reads "password or journal rules" and names provision-roles.sql. The three DarlingWorker call sites are unchanged (SelfManagedStoreRulesStartPinTests passes).
2. drop_plan_guide refuses a guide its Undo could not re-create. After the guide is read, its definition goes through the same checks a create makes. When it fails (hints or statement over 4,000,000 characters, or an OBJECT guide on a module name that does not normalize), the answer is refused, reason guide_not_undoable, with the check's own message, on a dry run and on a write, and nothing is dropped. The refusal is journaled like the tool's other refusals. The read of the guide still goes through the gate first (it needs the permit), so the refusal comes before the write and before any journal row of the action.
3. The web plan Undo runs on the owner source. Its journal writer is now built on ownerSource.Read() when that is published, as the MCP undo_plan_change does, so a result-unknown row is no longer refused when the web connection is the admin login. IsSignedInAdmin and CanDecide stay on the web login. UndoPlanChangeAsync in DarlingWebEndpoints became internal so a test can drive it.
4. Tool text. undo_plan_change names all four kinds (plan_force, plan_unforce, plan_guide_create, plan_guide_drop) and the switch for each (force_plan, create_plan_guide, drop_plan_guide). Its answer's "how to put it back" hint no longer says only force or unforce, and the get_agent_changes note lists the four kinds.

Two edits the merge with dev forced:

• The writes-on MCP instructions were 8016 characters against the 8000 cap once dev's newer text and this PR's were both in. The "only the ... tools act on one" sentence now says "the server action tools", and the Level 3 parenthetical says "the last five listed". AgentLevel2MuteRuleToolTests pinned the old parenthetical wording, so its string follows.
• ProvisioningSerializationTests holds the password rules' command to the locked transaction. Its rule still holds; its regex now matches the new lambda (journal SQL added to the same single command).

Test plan

• StoreServerPasswordRulesLiveTests (new: AStoreHoldingTheOlderJournalFunction_GetsTheOwnerOnlyEventListFromTheServicesStart_...: older list installed, an mcp-grant insert of action_called accepted before, refused 42501 after; a second start leaves the function row version unchanged; a login that is not the owner gets one warning naming the script and false; rules in place from another role warn nothing)
• PlanForceToolLiveTests on sql2019 (new: AGuideTheUndoCouldNotReCreate_... with a 4,000,000-character hint the server took; AWebPlanUndo_OnAStoreWhoseWebLoginIsHeldByTheJournalTrigger_..., a web Undo whose reply is lost writes its plan_undo / unknown row), whole class run
• Revert-proof: with the three fixes switched off, all three new tests fail; restored and they pass
• SelfManagedStoreRulesStartPinTests, InvestigationRequestEventsLiveTests, PlanGuideTests, AgentChangesWebLiveTests, DarlingCommandExecutorTests, McpIncidentActionsLiveTests, PlanForceNoWritePathTests, McpToolsListBudgetTests, DarlingMcpInstructionsBudgetTests, McpPayloadContractCensusTests, DarlingManagedRolesTests, ProvisionRoles*, ProvisioningSerializationTests, AgentChangeUndoLiveTests, McpToolGuideTests, every *Census* class: first batch 882 tests, 2 failures (the two edits above); after the fixes the re-runs (108, 80 and 545 tests) have 0 failures
• Full suite and pre-push.sh not run (laptop at its thermal limit; CI decides). The last merge of origin/dev (after the tests above) was not re-tested.
• Build: 0 errors. The 6 warnings are CA2016 in DarlingMcpAlertTools.cs, which this PR does not touch (from dev).

Considered, not added

• The alert-link function (investigation_request_alerts_owner_rows) is not re-applied at start: its text did not change in this PR, so no store holds a stale copy from it.
• A shared "one in place" check for the three functions: the journal check mirrors the password one on purpose, so each keeps its own comparison and its own not-applicable rule.

Round 3 (bot review) fixes

Claude posting for Erik Darling.

1. Journal identity. recompile_module's three refusals before the gate (bad_module_name, bad_database, postgres_target) now pass agentTokenName to RefuseBeforeGateAsync, as the kill, evict and plan guide tools do, so a named agent token's refusal is journaled under the token's name. Test: AgentTokenIdentityGateLiveTests.EachRecompileModuleRefusalBeforeTheGate_IsJournaledUnderTheTokensName (three cases). A named token calls recompile_module with a different agent_id, and the refusal row's actor is the token's name. Revert proof: with the three call sites put back, all three cases fail.
2. OBJECT guide refusal text. Checked on sql2019 first: as sa, an OBJECT guide on dbo.p; a user holding only GRANT ALTER ON OBJECT::dbo.p (HAS_PERMS_BY_NAME(DB_NAME(), 'DATABASE', 'ALTER') = 0, on the object = 1) does see the guide in sys.plan_guides (scope_type_desc OBJECT). So the !found && !grantedDb branch in SqlServerRemediationExecutor.PlanGuide.cs now also requires expected?.Type not to be OBJECT: an OBJECT guide that is gone answers guide_not_found ("already gone"). The drop action has no journaled type and keeps the permission answer. Test: PlanForceToolLiveTests.ALoginWithAlterOnTheProcedureOnly_UndoingAnObjectGuideThatIsAlreadyGone_SaysItIsGone_NotThatTheGuideIsHidden (the narrowed login creates the OBJECT guide, sa drops it by hand, then the Undo answers guide_not_found with "already gone" and no database ALTER sentence). Revert proof: with the old condition it fails. The probe database and login were dropped.
3. Dry-run switch sentence. recompile_module, create_plan_guide and drop_plan_guide now call GateRefusalAnswerAsync(...) like the three older server action tools, so a dry run refused for want of the claim also says whether that server's switch for the action is on. No existing test pinned the old text; the targeted classes pass unchanged.
4. Byte order marks. Removed from DarlingMcpInvestigationToolsTests.cs and PlanForceNoWritePathTests.cs; the diff against origin/dev on both shows no first-line change.
5. Also fixed: a CS8602 possible-null warning in CreatePlanGuideAsync (permit!.Database, as the drop path already has), which showed up in the merged build.

Run: sql2019 and a throwaway PostgreSQL rig, nothing skipped. AgentTokenIdentityGateLiveTests plus *PlanForceToolLive* first (145 passed), then the brief's whole class list plus *McpWriteToolWording* after the last dev merge: Total 1283, Failed 0, Skipped 0. The four new cases failed (4 of 4) with the fixes reverted.

Considered, not added

• Note 2 (Azure SQL switch): filed as Darling: a server action switch can be turned on for an Azure SQL Database target whose tool always refuses it #6727, not in this round.
• Note 4 (the migration's name): stays as it is.

Dev merge

Merged dev cb20aea (3.10.0 plus V215-V217) into this branch (head 9a5fd0e); the migration is now V218 (server-action-recompile-module). Darling build 0 warnings, 0 errors. Targeted classes (*Migration*, *SchemaVersion*, *PgSchemaGenerator*, *PlanGuide*, *RecompileModule*, *ServerAction*, *StoreServerPasswordRules*, *SelfManagedStoreRules*, *InvestigationRequestEvents*, *McpToolsListBudget*, *Instructions*, *McpPayloadContract*, *DarlingManagedRoles*, *ProvisionRoles*, *DarlingCustomViews*, *Census*, *Ratchet*): Total 1185, Failed 0, Skipped 0, on a throwaway PostgreSQL rig (stopped after). The live SQL Server tests were not rerun in this merge (no sa password available to this lane); the merge changes no behavior. CatalogCensus reads 264 tools / 152 Darling-only (dev's 261/149 plus this PR's 3).

Round 3: merged dev fca8641 again (head c9a4d34). Census 273 / 112 / 161 (dev 270 / 112 / 158 plus this PR's 3 tools); plain-text.js and PlanForceNoWritePathTests.cs take the union of both sides. Darling build 0 warnings, 0 errors; 1283 tests, 0 failed, 0 skipped.

Seat fix after CI (36e70cd)

CI leg 8 failed RemediationOfflineTests.EveryWrite_TakesTheGatesPermitFirst_AndAPermitCannotBeBuiltOutsideTheStorageAssembly: it expected 5 executor writes and found 8. The three new writes (RecompileModuleAsync, CreatePlanGuideAsync, DropPlanGuideAsync) take the permit first like the others, so the test now expects 8 writes and 9 public methods. Test change only.

[claude]

recompile_module's three pre-gate refusals drop agentTokenName, so a named agent token's refusal rows are journaled under the caller-supplied agent_id.

All three RefuseBeforeGateAsync calls in this file end at ct — line 54 (bad_module_name), line 61 (bad_database) and line 69 (postgres_target) — while the method's own agentTokenName parameter is used only once, at line 80 for ServerActionGate.AuthorizeAsync. Every other action tool passes it: DarlingMcpPlanForceTools.cs:425 / :433 (evict), :539 / :546 (kill), and the two new plan guide tools in this same PR (DarlingMcpPlanForceTools.PlanGuide.cs:62, :68, :77, :83, :311, :317, :325, :331).

RefuseBeforeGateAsync (DarlingMcpPlanForceTools.cs:929) journals agentTokenName ?? parsed.AgentId, with the comment "#6542: a named agent token journals under its own name, never the agent_id it passed." Dropping it is the exact shape the #6593 review already ruled on: AgentTokenIdentityGateLiveTests.cs:171-172 says "each kill refusal made before the gate journals the token's name, not the agent_id passed. A case fails when its call site drops agentTokenName: the row then names agent:someone-else."

Failure scenario: a client holding the named agent token triage-bot calls recompile_module with module_name = "dbo" (one part) and agent_id = "someone-else" in its agent_action. The refusal journals actor = 'agent:someone-else' — an identity the caller chose — instead of agent:triage-bot. The same call to kill_session, evict_cached_plan, create_plan_guide or drop_plan_guide journals the token's name.

Uncovered: AgentTokenIdentityGateLiveTests rosters only the kill refusals and create_mute_rule; there is no recompile_module case, and no census walks RefuseBeforeGateAsync call sites.

Suggested change

	"module_name is the two-part schema.name of a procedure, function or trigger, as get_procedure_stats returns it. Nothing was run.", ct);
	"module_name is the two-part schema.name of a procedure, function or trigger, as get_procedure_stats returns it. Nothing was run.", ct, agentTokenName);

(and the same addition at lines 61 and 69)

[claude]

The "you cannot see it" refusal fires for an OBJECT guide too, where it is not true — and it tells the operator to widen a grant the action does not need.

The branch is unconditional on guide type, but the sentence it returns is written for a SQL guide only: "it does not hold ALTER on the database {database}, which sp_control_plan_guide needs for a SQL guide, and a SQL guide is not listed for a login without it."

For an OBJECT guide the premise is false. sys.plan_guides lists an OBJECT guide to a login that holds ALTER on the referenced object, which is exactly the least-privilege configuration this PR documents (docs/darling-agent-dispatch.md grants row) and tests (PlanForceToolLiveTests.PlanGuide.cs:754 ALoginWithAlterOnTheProcedureOnly_...). In the batch, @can_db is HAS_PERMS_BY_NAME(DB_NAME(), N'DATABASE', N'ALTER') = 0 for such a login, and the SET @can_db = 1 compensation at line 206 only runs inside IF @found = 1, so a guide that is genuinely gone leaves found = 0, grantedDb = 0 and lands here.

Failure scenario: an agent creates an OBJECT guide g_p on dbo.p under a remediation login granted ALTER ON OBJECT::dbo.p and nothing else; a DBA then drops the guide by hand; a person clicks Undo in Agent changes. The undo answers refused / guide_permission_denied with "the remediation login cannot see it: it does not hold ALTER on the database", instead of guide_not_found / "it is already gone". The operator's remedy, as worded, is to grant database-wide ALTER — which docs/darling-agent-dispatch.md itself warns "also lets the login create users, change roles and schemas, and add database DDL triggers".

The information to tell the two apart is already in hand on the create-undo path: DropPlanGuideAsync receives expected, whose Type is the journaled "OBJECT". Gating this branch on expected?.Type is not "OBJECT" (and, for the drop action, keeping it as-is since the type is unknown when the guide is not found) would keep the SQL-guide case and give the OBJECT case the true answer.

Uncovered: TheUndoOfAGuideThatIsAlreadyGone_SaysSo_AndChangesNothing_... (PlanForceToolLiveTests.PlanGuide.cs:545) runs as db_owner, so grantedDb is 1 there, and ALoginWithAlterOnTheProcedureOnly_... never drops the guide out from under the undo. The PR body's round-2b note says "A login with the grant still gets guide_not_found. The same applies to 2a's undo of a create for such a login (it had no test)" — it does not say that for an OBJECT guide the sentence names a grant that is not the one sp_control_plan_guide needs.

[claude]

Non-blocking notes (not part of the verdict)

1. The three new Level 3 tools skip the #6455 switch-state sentence on a refused dry run.
recompile_module (DarlingMcpPlanForceTools.RecompileModule.cs:82), create_plan_guide (DarlingMcpPlanForceTools.PlanGuide.cs:96) and drop_plan_guide (DarlingMcpPlanForceTools.PlanGuide.cs:344) call the bare GateRefusalAnswer(gateRefusal, dry_run). The three older server action tools call GateRefusalAnswerAsync(...) (DarlingMcpPlanForceTools.cs:271, :446, :570, :726), which reads ReadSwitchStateAsync and appends one sentence saying whether that server's switch for the action is on. The #6455 comment at DarlingMcpPlanForceTools.cs:153-158 gives the reason: "so the dry run still shows whether the real call could run." An agent that dry-runs create_plan_guide without an agent_action therefore learns less than one that dry-runs evict_cached_plan. Nothing is wrong, only omitted.

2. ServerActionNames.IsValidForEngine lets the plan guide switches be turned on for an Azure SQL target the tools always refuse.
InvestigationContracts.cs:356-358 returns true for create_plan_guide and drop_plan_guide on any non-PostgreSQL server, so set_server_action and both switch screens will turn them on for a server whose sql_engine_edition is 5 or 8, where the tools refuse unconditionally as engine_not_supported (DarlingMcpPlanForceTools.PlanGuide.cs:80, :328). Darling/README.md says "A switch cannot be turned on for an action the server's engine cannot run." This matches the existing evict_plan shape (also refused on Azure SQL Database while IsValidForEngine allows it), so it is consistent with what ships, not a regression this PR introduces.

3. Two test files gained a UTF-8 byte order mark that the body does not mention.
Darling.Tests/DarlingMcpInvestigationToolsTests.cs:1 and Darling.Tests/PlanForceNoWritePathTests.cs:1 change only in that a BOM now precedes the leading /*. Harmless for the C# compiler, but it is an unexplained encoding change in two files whose substantive edits are one roster line each. Worth confirming it was intended rather than an editor artifact.

4. The V218 migration's name undersells it.
PgMigrations.cs:2784 registers it as "server-action-recompile-module", but the CHECK it writes adds recompile_module, create_plan_guide and drop_plan_guide (:2388), and both the rung guard and the viewer probe sentinel key on drop_plan_guide. The body explains why (2a and 2b joined the same rung). Cosmetic only, since the name is a label and not a key, but a reader grepping for the plan guide rung will not find it.

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED - every value reaches sp_create_plan_guide as a parameter, none spliced into batch text - SqlServerRemediationExecutor.PlanGuide.cs:250-257; only the fixed type list is interpolated (:163); test AStatementWithAQuote_GoesThroughAsAParameter_AndIsStoredExactly.
• VERIFIED - the recompile write is EXEC sys.sp_recompile @objname = @resolved; in the same batch, name as @p0 - SqlServerRemediationExecutor.RecompileModule.cs:33, :45, :81.
• VERIFIED - V218 widens ck_config_server_actions_action, guarded, inserts no row - PgMigrations.cs:2379-2390, :2784; StorageVersion.cs:19 = 218.
• VERIFIED - create_plan_guide and drop_plan_guide ride V218's CHECK, no new rung - PgMigrations.cs:2388, InvestigationContracts.cs:349.
• VERIFIED - the three agent action events are owner-only in both trigger texts - InvestigationRequestStore.AgentAction.cs:156-164, DarlingManagedRoles.cs JournalOwnerEventList, provision-roles.sql:416.
• VERIFIED - every plan tool passes the owner source - DarlingMcpPlanForceTools.cs:242, :419, :524; ...PlanGuide.cs:57, :306; ...RecompileModule.cs:48; DarlingMcpAgentActions.cs:148, DarlingMcpSignatureMergeTools.cs:91, DarlingMcpThresholdOverrideTools.cs:88.
• VERIFIED - the web plan Undo runs on the owner source while IsSignedInAdmin and CanDecide stay on the web login - DarlingWebEndpoints.cs:1787, :1792, :1906-1909.
• VERIFIED - drop_plan_guide refuses a guide its Undo could not re-create, dry run and write, after the gate - ...PlanGuide.cs:342, :376-383, :387.
• VERIFIED - the guide Undo acts only on an agent row (actor agent:, same server, level 3+) - AgentChangeUndo.PlanUndo.cs:139, :236-237.
• VERIFIED - a store with no owner source runs on its own, and the Level 2 tools then fail at action_called (stated as a tradeoff) - InvestigationRequestStore.cs:250; pinned by AgentLevel2StoreTests.cs:95-97.
• UNVERIFIED - all live-rig results and counts (123, 142, 977, 1020, 1034, 1109, 1185) and the six revert-proofs - needs a SQL Server 2019 with a remediation login plus a throwaway PostgreSQL store.
• UNVERIFIED - sp_recompile works on Azure SQL Database, not run - needs an Azure target; the code matches (...RecompileModule.cs:66 refuses only PostgreSQL, InvestigationContracts.cs:358).
• UNVERIFIED - the measured engine facts (no row from fn_validate_plan_guide when valid, message 8698, byte-identical read-back, error 10502) - engine behaviour; the batch at ...PlanGuide.cs:166-177 rests on them.
• REFUTED - "StorageVersion.SchemaVersion = 215 ... GetBoolean(190) ... 254 tools / 142 unique" (also 255/143, 256/144) - tree has 218, ViewerDataService.cs:1357 ends at GetBoolean(193), DarlingMcpInstructions.cs:58 reads 264/152. Stale draft lines the Dev merge section corrects, not a code defect.
• REFUTED - "a TEMPLATE guide is refused (guide_template, bad_guide_type)" - TEMPLATE is accepted (...PlanGuide.cs:55, :77-85), dropped and journaled (:413), and no guide_template exists in RemediationResult.cs:87-95. The "Guide forms" section says it supersedes these lines.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPlanForceTools.RecompileModule.cs:54 (also :61, :69) - the three pre-gate RefuseBeforeGateAsync calls drop agentTokenName, so a named agent token's refusal is journaled under the caller-supplied agent_id; AgentTokenIdentityGateLiveTests.cs:171-172 names this exact shape and every sibling tool passes it - UNCOVERED (that test rosters kill only; PlanForceToolLiveTests.Recompile.cs:27 passes no token name).
2. Darling/PerformanceMonitor.Darling.Remediation/SqlServerRemediationExecutor.PlanGuide.cs:403 - the !found && !grantedDb branch returns a SQL-guide-only sentence for every type, so the Undo of a gone OBJECT guide under an object-only ALTER login is told to grant database-wide ALTER instead of "already gone" - UNCOVERED (...PlanGuide.cs:545 runs as db_owner; :754 never drops the guide first).
3. Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.AgentAction.cs:186 - AgentActionSource falls back to the mcp login, which the tightened trigger now answers with a raw 42501; unlike the plan tools (...PlanGuide.cs:86-90) the Level 2 store tools have no owner is null arm - covered as a throw by AgentLevel2StoreTests.cs:95-97 and documented in the body, so pinned behaviour, not a finding.

Traps near this diff

• "Lite and Darling ship in parity ... a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit" - does not apply - server actions have no Lite twin by design (PgMigrations.cs:2407), and the one shared file that grew gates the new names on level2 (InvestigationBriefComposer.cs:232), which Lite never reaches (InvestigationTables.cs:110 pins autonomy_level = 1).
• "hours_back, limit, top, days_back are refused when out of range, never clamped" - does not apply - the new tools take no window or page argument (...PlanGuide.cs:526-543, :551-562, ...RecompileModule.cs:172-183); the caps at ...PlanGuide.cs:58-64 refuse, never truncate.
• "Truncation inferred from a full page: truncated = rows.Count >= limit" - does not apply - no paged read; the one count is cached_matches, taken with no LIMIT (...PlanGuide.cs:152-155).
• "A total_* assigned from a .Count is a count of the page unless the set is provably whole" - does not apply - cached_matches is the server's COUNT_BIG(*) filtered to st.dbid = DB_ID() (...PlanGuide.cs:152-155), and MatchWords (:302-307) says what it is "of".
• "A JSON number above 2^53 is rounded by every JavaScript client" - does not apply - the only long emitted is journal_id (...PlanGuide.cs:173, :430), a bigint identity from 1, not a tick-scale id; force, evict and kill already ship it.
• "Ruling (#5833): tests are load-bearing ... Never add one" - does not apply - no new census; existing rosters are extended (MigrationDataMovingRungCensusPins.cs:184-187, PlanForceNoWritePathTests.cs:183, :268) and the six new files are live behaviour tests. Its corollary holds above: those source lists are not coverage for either finding.
• ".gitattributes says text=auto eol=crlf ... a diff that touches every line is a line-ending conversion until proven otherwise" - does not apply - the two encoding hunks (DarlingMcpInvestigationToolsTests.cs:1, PlanForceNoWritePathTests.cs:1) touch one line each, a byte order mark before /*.

Divergence (blind read vs PR body)

1. The body presents the three new tools as one shape ("same gate as recompile_module") and devotes a whole section to making the journal name the service rather than a caller, but the diff ships recompile_module with agentTokenName dropped at all three pre-gate refusals while both plan guide tools, written later in the same PR, pass it at all eight of theirs. The body does not mention the difference, and #6593 already ruled this shape a defect.
2. Slice 2b says the not-found permission sentence "applies to 2a's undo of a create for such a login (it had no test)". Extending it there also extends it to OBJECT guides, where the sentence's own premise ("a SQL guide is not listed for a login without it") is false and the type is already known from expected. The body reads this as a coverage gap; it is also a wrong answer.
3. Unmentioned in the body: two UTF-8 byte order marks added to test files, and the three new tools using GateRefusalAnswer instead of the GateRefusalAnswerAsync the other server action tools use, which drops the #6455 switch-state sentence from a refused dry run. Both are in the non-blocking comment.

Everything else agrees: the owner-only journal move, the V218 rung and probe sentinel, the Undo provenance guard, guide_not_undoable, and the web Undo's owner source all read in the tree as described.

Reviewer checklist

• Add a recompile_module case to AgentTokenIdentityGateLiveTests and read the refusal row's actor before and after passing agentTokenName at ...RecompileModule.cs:54, :61, :69.
• On sql2019, narrow the remediation login to GRANT ALTER ON OBJECT::dbo.p, create an OBJECT guide, drop it by hand as sa, click Undo, and read which reason comes back.
• Start a service against a store provisioned by the older provision-roles.sql and confirm the journal function is replaced in the same transaction as the password rules; only a live start reaches the below-V191 arm and RecoverFromReadAsync.
• Apply V218 to a store that already holds config.config_server_actions rows and confirm the CHECK validates them without a long lock.
• Run recompile_module against an Azure SQL Database (edition 5), the one new tool that does not refuse that engine.
• Re-run the live SQL Server classes on the merged head; the Dev merge section says they were not rerun there.

[claude]

Verdict: LGTM

Claims

• VERIFIED — recompile_module's three refusals before the gate now pass agentTokenName to RefuseBeforeGateAsync, as the kill/evict/plan-guide tools do — Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPlanForceTools.RecompileModule.cs:54,61,69; test AgentTokenIdentityGateLiveTests.EachRecompileModuleRefusalBeforeTheGate_IsJournaledUnderTheTokensName.
• VERIFIED — the !found && !grantedDb branch in DropPlanGuideAsync now also requires expected?.Type not to be OBJECT, so a gone OBJECT guide answers guide_not_found instead of the database-ALTER permission sentence; a drop action (no journaled type) keeps the old answer — Darling/PerformanceMonitor.Darling.Remediation/SqlServerRemediationExecutor.PlanGuide.cs:401-407; test PlanForceToolLiveTests.ALoginWithAlterOnTheProcedureOnly_UndoingAnObjectGuideThatIsAlreadyGone_SaysItIsGone_NotThatTheGuideIsHidden.
• VERIFIED — recompile_module, create_plan_guide and drop_plan_guide now call GateRefusalAnswerAsync(...) instead of the bare sync GateRefusalAnswer, matching the three older server action tools — ...RecompileModule.cs:82, ...PlanGuide.cs:96, ...PlanGuide.cs:344.
• VERIFIED — the UTF-8 BOMs on DarlingMcpInvestigationToolsTests.cs and PlanForceNoWritePathTests.cs are gone, one line changed in each file — confirmed via git diff --stat on both commits (1 insertion/1 deletion apiece).
• VERIFIED — the CS8602 possible-null fix in CreatePlanGuideAsync (permit!.Database) matches the null-forgiving operator DropPlanGuideAsync already used — SqlServerRemediationExecutor.PlanGuide.cs:236 (commit 316841c4), compared against the unchanged :358.
• UNVERIFIED — the live sql2019 check that a login holding only GRANT ALTER ON OBJECT::dbo.p sees an OBJECT guide in sys.plan_guides while failing HAS_PERMS_BY_NAME(..., 'DATABASE', 'ALTER') — needs a live SQL Server 2019 instance with a narrowed login; the code change is consistent with the claim but the underlying engine behavior can't be checked from a read-only tree.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Remediation/SqlServerRemediationExecutor.PlanGuide.cs:405 — this is the exact line the prior review flagged as "Riskiest line #2"; wrong behavior here means an agent undoing an OBJECT-guide create under an object-only-ALTER login gets told to grant database ALTER instead of "already gone" — covered by PlanForceToolLiveTests.ALoginWithAlterOnTheProcedureOnly_UndoingAnObjectGuideThatIsAlreadyGone_SaysItIsGone_NotThatTheGuideIsHidden (live, asserts guide_not_found + "already gone" + absence of the database-ALTER sentence).
2. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPlanForceTools.RecompileModule.cs:54 (also :61, :69) — this is the prior review's "Riskiest line #1"; dropping agentTokenName here journals a named token's refusal under the caller-supplied agent_id instead — covered by AgentTokenIdentityGateLiveTests.EachRecompileModuleRefusalBeforeTheGate_IsJournaledUnderTheTokensName (three cases: bad_module_name, bad_database, postgres_target).
3. Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPlanForceTools.PlanGuide.cs:96,344 and ...RecompileModule.cs:82 — switch to GateRefusalAnswerAsync so a refused dry run also gets the switch-state sentence — UNCOVERED; the PR body itself says "No existing test pinned the old text," and no new test asserts the sentence's presence for these three tools. Non-blocking (matches the prior review's own non-blocking note #1, not a correctness risk), but worth a one-line note if a future round touches this path again.

Traps near this diff

• "Lite and Darling ship in parity ... a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit" — does not apply — server actions have no Lite twin; no ServerActionGate/AgentActionGate caller exists under any Lite path (grep -rl ServerActionGate --include=*.cs . returns no Lite files).
• "Ruling (#5833): tests are load-bearing ... never add [a test] that reads source text or asserts how code is spelled" — does not apply — the new/extended tests (AgentTokenIdentityGateLiveTests, PlanForceToolLiveTests.PlanGuide.cs) drive the real tool methods against a live server/store and assert on actual outcomes (refusal reason, journaled actor, message text from the server), not on source spelling.
• ".gitattributes says text=auto eol=crlf ... a diff that touches every line is a line-ending conversion until proven otherwise" — does not apply — the two BOM removals (DarlingMcpInvestigationToolsTests.cs:1, PlanForceNoWritePathTests.cs:1) each touch exactly one line, confirmed via git diff --stat, not a bulk line-ending rewrite.

Divergence (blind read vs PR body)

None — the body's "Round 3 (bot review) fixes" section matches the diff on every claim graded above: the journal-identity fix, the OBJECT-guide wording fix, the GateRefusalAnswerAsync wiring, the two BOM removals, and the CS8602 fix are all exactly what the tree shows, and both fixes map 1:1 onto the prior review's two confirmed "Riskiest lines" findings.

Reviewer checklist

• On sql2019, narrow the remediation login to GRANT ALTER ON OBJECT::dbo.p, create an OBJECT guide, drop it by hand as sa, then Undo, and confirm the answer is guide_not_found / "already gone" with no database-ALTER sentence (the live assertion the new test makes, can't be executed from this read-only tree).
• Run AgentTokenIdentityGateLiveTests.EachRecompileModuleRefusalBeforeTheGate_IsJournaledUnderTheTokensName against a live store and confirm all three cases journal under the token's name.

Claude posting for Erik Darling. Addressed in c9a4d34; the follow-up review 5641006373 is LGTM.