Skip to content

test(ai): pin the aiTools registry against TOOL_TIERS, both directions (#3300) - #3306

Merged
ToddHebebrand merged 1 commit into
LanternOps:mainfrom
bdunncompany:fix/3300-tool-tiers-registry-guard
Aug 10, 2026
Merged

test(ai): pin the aiTools registry against TOOL_TIERS, both directions (#3300)#3306
ToddHebebrand merged 1 commit into
LanternOps:mainfrom
bdunncompany:fix/3300-tool-tiers-registry-guard

Conversation

@bdunncompany

Copy link
Copy Markdown
Collaborator

Part of #3300the guard only, not the tier assignments.

Per the issue's own sequencing ("Step 2 is arguably worth doing first"), this adds the contract test and leaves all 86 tier decisions alone. A wrong tier on execute_containment or revoke_elevation is worse than the tool being invisible, so those want the deliberate per-tool review the issue describes.

What drifted

TOOL_TIERS gates chat visibility twice — BREEZE_MCP_TOOL_NAMES = Object.keys(TOOL_TIERS) is the allowedTools list handed to the SDK, and createSessionPreToolUse rejects !TOOL_TIERS[toolName] as "Unknown tool". Nothing asserted the two maps agree, and they drifted both ways:

Direction Count Effect
Registered, untiered 86 a working, tested tool that chat says does not exist
Tiered, unregistered 4 a name advertised as callable that fails at execution

The second direction is not in the issue. The four are get_backup_health, get_recovery_readiness, propose_action_plan, run_backup_verification. Same #2605 drift class, so the guard is bidirectional.

Measured on f400fc315: 215 registered, 133 tiered, 86 missing, 4 dead. (215/133 rather than the issue's 217/135 because #3291 merged in between; the missing count is 86 either way.)

What this adds

One test file. Three contracts:

  1. registry ⊆ TOOL_TIERS — minus KNOWN_MISSING_TOOL_TIERS, the 86 frozen.
  2. TOOL_TIERS ⊆ registry — minus KNOWN_UNREGISTERED_TOOL_TIERS, the 4 frozen.
  3. shared names carry the same tier in both maps — no allowlist, because this held for all 133.

Contract 3 is the one worth keeping past the cleanup. Presence alone is the weaker claim: an entry that is present but lower than the registry's tier has had its approval gate quietly downgraded, which is worse than invisibility. It also means the eventual step-1 work has a defensible default — the registry's own tier — rather than a blank sheet.

Both allowlists may only shrink. Each block has a second test that fails on a stale entry, so deleting a name is what un-blocks CI once a tier lands, and re-adding one cannot pass. The lists walk to empty on their own.

Controls

Every assertion was proven load-bearing by breaking it:

Perturbation Result
drop a real gap from KNOWN_MISSING registry-subset test fails
add a tiered tool to KNOWN_MISSING shrink-only test fails
drop a dead entry from KNOWN_UNREGISTERED tiers-subset test fails
add a live tool to KNOWN_UNREGISTERED shrink-only test fails
execute_command: 31 in TOOL_TIERS agreement test fails: execute_command: TOOL_TIERS=1 registry=3

The last row is the one that matters — a silent approval-gate downgrade is caught.

Verification

Test-only. No source, dependency, or behaviour changes, so the security and dependency jobs are unaffected.

  • vitest run (apps/api) — 1298 files, 20766 tests passed, 5 files / 40 tests skipped
  • tsc --noEmit --project apps/api/tsconfig.json0 errors
  • eslint on the new file — clean

LanternOps#3300)

`TOOL_TIERS` gates AI-chat visibility twice: `BREEZE_MCP_TOOL_NAMES =
Object.keys(TOOL_TIERS)` is the `allowedTools` list handed to the SDK, and
`createSessionPreToolUse` rejects `!TOOL_TIERS[toolName]` as "Unknown tool".
Nothing asserted that the two maps agree, and they have drifted in both
directions:

- 86 registered tools have no `TOOL_TIERS` entry, so a working, fully-tested
  tool is one the chat agent says does not exist. Whole feature families are
  affected — backup, MSSQL, Hyper-V, DR, incident response, PAM elevation.
- 4 `TOOL_TIERS` keys have no registered tool, so those names are advertised
  to the SDK as callable and then fail at execution rather than simply not
  being offered: get_backup_health, get_recovery_readiness,
  propose_action_plan, run_backup_verification.

This is the LanternOps#2605 drift class, and it reached 86 precisely because nothing
failed CI when it happened.

This adds only the guard, not the fix. Assigning a tier is an approval-gate
decision that wants per-tool review — a wrong tier on `execute_containment`
or `revoke_elevation` is worse than the tool being invisible — so the 86 and
the 4 go into frozen allowlists instead. Both lists may only shrink, and a
test in each block fails on a stale entry, so removing a gap is what
un-blocks CI and re-adding one cannot pass.

Measured on f400fc3: 215 registered, 133 tiered, 86 missing, 4 dead.

The suite also asserts that every shared name carries the SAME tier in both
maps. That held for all 133 with no exceptions, so it starts with no
allowlist. Presence alone would be the weaker contract: an entry that is
present but carries a lower tier than the registry assigned it has had its
approval gate silently downgraded, which is strictly worse than being
invisible.

Controls (each assertion proven load-bearing, not vacuous):
- drop a real gap from KNOWN_MISSING       -> registry-subset test fails
- add a tiered tool to KNOWN_MISSING       -> shrink-only test fails
- drop a dead entry from KNOWN_UNREGISTERED-> tiers-subset test fails
- add a live tool to KNOWN_UNREGISTERED    -> shrink-only test fails
- set execute_command 3 -> 1 in TOOL_TIERS -> agreement test fails with
  "execute_command: TOOL_TIERS=1 registry=3"

Test-only change; no source, dependency, or behaviour changes.
Local gate: apps/api vitest 1298 files / 20766 tests pass, tsc 0 errors,
eslint clean.
@bdunncompany

Copy link
Copy Markdown
Collaborator Author

Rebased onto 2cb6f7a6b (post #3308 + #3309) and re-verified — no content change, just moving off a stale base.

This PR freezes exact global counts (215 registered / 133 tiered / 86 missing / 4 dead), so a merge that adds, removes, or re-tiers a tool would break it after it lands rather than on the PR. #3308 touched services/aiToolsIntegrations.ts, so that was worth checking rather than assuming: the change is confined to the query inside the existing query_psa_status handler (dual-axis condition + ownerScope in the result) with no registration or TOOL_TIERS edit.

Re-ran the suite against the rebased tree: 5/5 pass, both allowlists still exact.

@bdunncompany
bdunncompany force-pushed the fix/3300-tool-tiers-registry-guard branch from 539d0c5 to 079f5c8 Compare August 9, 2026 12:14
@bdunncompany

Copy link
Copy Markdown
Collaborator Author

The Test Web red here is not from this PR — this diff is a single API contract test file and touches nothing under apps/web.

It's the known InvoiceWorkspace flake (#3219 / #3277): holds a header Issue click while the editor is dirty, then fires it when the save settles, 1 failed / 538 passed. Fourth sighting in eleven days; #3284 is the open fix. I've rerun the job.

@ToddHebebrand
ToddHebebrand merged commit ce22803 into LanternOps:main Aug 10, 2026
94 of 96 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants