From 8d5266c38e091d4418f548864e6d9065149b8808 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 21 Sep 2026 21:49:10 +0530 Subject: [PATCH 1/6] fix(finops): name the workspace engine tool when a FinOps tool has no local connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `finops_*` tools resolve warehouses from the local connection registry only. In a project bound to a workspace whose Snowflake credentials live in the workspace, every one of them failed with a bare `FAILED`, and the workspace's own cost skills steer the model to exactly these tools first — so a cost question began with four failures before the model fell back to raw `ACCOUNT_USAGE` SQL through `datamate_snowflake_execute_database_query`. - New `tools/finops-workspace.ts`: when the session's precedence snapshot serves `sql_execute` for a type the operation supports (reachability-filtered, the same projection the awareness section uses), append the reason and the engine tool to the failure, with the usage tables to query, and stamp `metadata.workspace_fallback` for telemetry - Every `finops_*` failure branch (dispatcher error and thrown error) goes through `withWorkspaceFallback`; the local reason is kept - Export `QUERY_HISTORY_SUPPORTED_TYPES` / `SNOWFLAKE_ONLY_TYPES` so the wrappers name the types their handler really supports - Tests: served/unserved/disabled/analyst shapes, and the three tool paths through the real handlers with an empty registry Closes #1336 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- .../altimate/native/finops/query-history.ts | 2 +- .../src/altimate/native/finops/role-access.ts | 2 +- .../altimate/tools/finops-analyze-credits.ts | 10 +- .../tools/finops-expensive-queries.ts | 10 +- .../altimate/tools/finops-query-history.ts | 10 +- .../src/altimate/tools/finops-role-access.ts | 27 +-- .../altimate/tools/finops-unused-resources.ts | 10 +- .../altimate/tools/finops-warehouse-advice.ts | 10 +- .../src/altimate/tools/finops-workspace.ts | 92 ++++++++++ .../finops-workspace-fallback.test.ts | 166 ++++++++++++++++++ 10 files changed, 305 insertions(+), 34 deletions(-) create mode 100644 packages/opencode/src/altimate/tools/finops-workspace.ts create mode 100644 packages/opencode/test/altimate/finops-workspace-fallback.test.ts diff --git a/packages/opencode/src/altimate/native/finops/query-history.ts b/packages/opencode/src/altimate/native/finops/query-history.ts index 75bac2b761..b5438bf355 100644 --- a/packages/opencode/src/altimate/native/finops/query-history.ts +++ b/packages/opencode/src/altimate/native/finops/query-history.ts @@ -9,7 +9,7 @@ import { bqRegionFor, interpolateBqRegion } from "./bq-utils" import { resolveFinopsWarehouse } from "./warehouse-resolver" import type { QueryHistoryParams, QueryHistoryResult } from "../types" -const QUERY_HISTORY_SUPPORTED_TYPES = [ +export const QUERY_HISTORY_SUPPORTED_TYPES = [ "snowflake", "postgres", "postgresql", diff --git a/packages/opencode/src/altimate/native/finops/role-access.ts b/packages/opencode/src/altimate/native/finops/role-access.ts index c10a82c05a..0252eda23a 100644 --- a/packages/opencode/src/altimate/native/finops/role-access.ts +++ b/packages/opencode/src/altimate/native/finops/role-access.ts @@ -109,7 +109,7 @@ LIMIT ? // --------------------------------------------------------------------------- const GRANTS_SUPPORTED_TYPES = DEFAULT_FINOPS_TYPES -const SNOWFLAKE_ONLY_TYPES = ["snowflake"] as const +export const SNOWFLAKE_ONLY_TYPES = ["snowflake"] as const function rowsToRecords(result: { columns: string[]; rows: any[][] }): Record[] { return result.rows.map((row) => { diff --git a/packages/opencode/src/altimate/tools/finops-analyze-credits.ts b/packages/opencode/src/altimate/tools/finops-analyze-credits.ts index a90c375ff2..9338fd19e2 100644 --- a/packages/opencode/src/altimate/tools/finops-analyze-credits.ts +++ b/packages/opencode/src/altimate/tools/finops-analyze-credits.ts @@ -1,6 +1,8 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" +import { DEFAULT_FINOPS_TYPES } from "../native/finops/warehouse-resolver" +import { withWorkspaceFallback } from "./finops-workspace" function formatCreditsAnalysis( totalCredits: number, @@ -93,11 +95,11 @@ export const FinopsAnalyzeCreditsTool = Tool.define("finops_analyze_credits", { if (!result.success) { const error = result.error ?? "Unknown error" - return { + return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { title: "Credit Analysis: FAILED", metadata: { success: false, total_credits: 0, error }, output: `Failed to analyze credits: ${error}`, - } + }) } const totalCredits = Number(result.total_credits ?? 0) @@ -114,11 +116,11 @@ export const FinopsAnalyzeCreditsTool = Tool.define("finops_analyze_credits", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return { + return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { title: "Credit Analysis: ERROR", metadata: { success: false, total_credits: 0, error: msg }, output: `Failed to analyze credits: ${msg}`, - } + }) } }, }) diff --git a/packages/opencode/src/altimate/tools/finops-expensive-queries.ts b/packages/opencode/src/altimate/tools/finops-expensive-queries.ts index 58200eed1f..03107d962f 100644 --- a/packages/opencode/src/altimate/tools/finops-expensive-queries.ts +++ b/packages/opencode/src/altimate/tools/finops-expensive-queries.ts @@ -1,6 +1,8 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" +import { DEFAULT_FINOPS_TYPES } from "../native/finops/warehouse-resolver" +import { withWorkspaceFallback } from "./finops-workspace" import { formatBytes, truncateQuery } from "./finops-formatting" function formatExpensiveQueries(queries: unknown[]): string { @@ -58,11 +60,11 @@ export const FinopsExpensiveQueriesTool = Tool.define("finops_expensive_queries" if (!result.success) { const error = result.error ?? "Unknown error" - return { + return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { title: "Expensive Queries: FAILED", metadata: { success: false, query_count: 0, error }, output: `Failed to find expensive queries: ${error}`, - } + }) } return { @@ -72,11 +74,11 @@ export const FinopsExpensiveQueriesTool = Tool.define("finops_expensive_queries" } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return { + return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { title: "Expensive Queries: ERROR", metadata: { success: false, query_count: 0, error: msg }, output: `Failed to find expensive queries: ${msg}`, - } + }) } }, }) diff --git a/packages/opencode/src/altimate/tools/finops-query-history.ts b/packages/opencode/src/altimate/tools/finops-query-history.ts index c80ed9b39d..8ab7c782fc 100644 --- a/packages/opencode/src/altimate/tools/finops-query-history.ts +++ b/packages/opencode/src/altimate/tools/finops-query-history.ts @@ -1,6 +1,8 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" +import { QUERY_HISTORY_SUPPORTED_TYPES } from "../native/finops/query-history" +import { withWorkspaceFallback } from "./finops-workspace" import { formatBytes, truncateQuery } from "./finops-formatting" function formatQueryHistory(summary: Record, queries: unknown[]): string { @@ -82,11 +84,11 @@ export const FinopsQueryHistoryTool = Tool.define("finops_query_history", { if (!result.success) { const error = result.error ?? "Unknown error" - return { + return withWorkspaceFallback(ctx.sessionID, QUERY_HISTORY_SUPPORTED_TYPES, { title: "Query History: FAILED", metadata: { success: false, query_count: 0, error }, output: `Failed to fetch query history: ${error}`, - } + }) } const summary = result.summary as Record @@ -97,11 +99,11 @@ export const FinopsQueryHistoryTool = Tool.define("finops_query_history", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return { + return withWorkspaceFallback(ctx.sessionID, QUERY_HISTORY_SUPPORTED_TYPES, { title: "Query History: ERROR", metadata: { success: false, query_count: 0, error: msg }, output: `Failed to fetch query history: ${msg}`, - } + }) } }, }) diff --git a/packages/opencode/src/altimate/tools/finops-role-access.ts b/packages/opencode/src/altimate/tools/finops-role-access.ts index c40b48df39..1fef960cad 100644 --- a/packages/opencode/src/altimate/tools/finops-role-access.ts +++ b/packages/opencode/src/altimate/tools/finops-role-access.ts @@ -1,6 +1,9 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" +import { DEFAULT_FINOPS_TYPES } from "../native/finops/warehouse-resolver" +import { SNOWFLAKE_ONLY_TYPES } from "../native/finops/role-access" +import { withWorkspaceFallback } from "./finops-workspace" function formatGrants(privilegeSummary: unknown, grants: unknown[]): string { const lines: string[] = [] @@ -121,11 +124,11 @@ export const FinopsRoleGrantsTool = Tool.define("finops_role_grants", { }) if (!result.success) { - return { + return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { title: "Role Grants: FAILED", metadata: { success: false, grant_count: 0 }, output: `Failed to query grants: ${result.error ?? "Unknown error"}`, - } + }) } return { @@ -135,11 +138,11 @@ export const FinopsRoleGrantsTool = Tool.define("finops_role_grants", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return { + return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { title: "Role Grants: ERROR", metadata: { success: false, grant_count: 0, error: msg }, output: `Failed to query grants: ${msg}`, - } + }) } }, }) @@ -159,11 +162,11 @@ export const FinopsRoleHierarchyTool = Tool.define("finops_role_hierarchy", { const result = await Dispatcher.call("finops.role_hierarchy", { warehouse: args.warehouse }) if (!result.success) { - return { + return withWorkspaceFallback(ctx.sessionID, SNOWFLAKE_ONLY_TYPES, { title: "Role Hierarchy: FAILED", metadata: { success: false, role_count: 0 }, output: `Failed to query role hierarchy: ${result.error ?? "Unknown error"}`, - } + }) } return { @@ -173,11 +176,11 @@ export const FinopsRoleHierarchyTool = Tool.define("finops_role_hierarchy", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return { + return withWorkspaceFallback(ctx.sessionID, SNOWFLAKE_ONLY_TYPES, { title: "Role Hierarchy: ERROR", metadata: { success: false, role_count: 0, error: msg }, output: `Failed to query role hierarchy: ${msg}`, - } + }) } }, }) @@ -203,11 +206,11 @@ export const FinopsUserRolesTool = Tool.define("finops_user_roles", { }) if (!result.success) { - return { + return withWorkspaceFallback(ctx.sessionID, SNOWFLAKE_ONLY_TYPES, { title: "User Roles: FAILED", metadata: { success: false, assignment_count: 0 }, output: `Failed to query user roles: ${result.error ?? "Unknown error"}`, - } + }) } return { @@ -217,11 +220,11 @@ export const FinopsUserRolesTool = Tool.define("finops_user_roles", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return { + return withWorkspaceFallback(ctx.sessionID, SNOWFLAKE_ONLY_TYPES, { title: "User Roles: ERROR", metadata: { success: false, assignment_count: 0, error: msg }, output: `Failed to query user roles: ${msg}`, - } + }) } }, }) diff --git a/packages/opencode/src/altimate/tools/finops-unused-resources.ts b/packages/opencode/src/altimate/tools/finops-unused-resources.ts index 159fa8db09..442d219f52 100644 --- a/packages/opencode/src/altimate/tools/finops-unused-resources.ts +++ b/packages/opencode/src/altimate/tools/finops-unused-resources.ts @@ -1,6 +1,8 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" +import { DEFAULT_FINOPS_TYPES } from "../native/finops/warehouse-resolver" +import { withWorkspaceFallback } from "./finops-workspace" function formatUnusedResources( summary: Record, @@ -81,11 +83,11 @@ export const FinopsUnusedResourcesTool = Tool.define("finops_unused_resources", if (!result.success) { const error = result.error ?? "Unknown error" - return { + return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { title: "Unused Resources: FAILED", metadata: { success: false, unused_count: 0, error }, output: `Failed to find unused resources: ${error}`, - } + }) } const summary = result.summary as Record @@ -98,11 +100,11 @@ export const FinopsUnusedResourcesTool = Tool.define("finops_unused_resources", } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return { + return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { title: "Unused Resources: ERROR", metadata: { success: false, unused_count: 0, error: msg }, output: `Failed to find unused resources: ${msg}`, - } + }) } }, }) diff --git a/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts b/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts index b3c9c58227..b6d1d4ed16 100644 --- a/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts +++ b/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts @@ -1,6 +1,8 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" +import { DEFAULT_FINOPS_TYPES } from "../native/finops/warehouse-resolver" +import { withWorkspaceFallback } from "./finops-workspace" function formatWarehouseAdvice( recommendations: unknown[], @@ -102,11 +104,11 @@ export const FinopsWarehouseAdviceTool = Tool.define("finops_warehouse_advice", if (!result.success) { const error = result.error ?? "Unknown error" - return { + return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { title: "Warehouse Advice: FAILED", metadata: { success: false, recommendation_count: 0, error }, output: `Failed to analyze warehouses: ${error}`, - } + }) } // Defensive null-coalesce in case the handler ever returns a partial @@ -125,11 +127,11 @@ export const FinopsWarehouseAdviceTool = Tool.define("finops_warehouse_advice", } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return { + return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { title: "Warehouse Advice: ERROR", metadata: { success: false, recommendation_count: 0, error: msg }, output: `Failed to analyze warehouses: ${msg}`, - } + }) } }, }) diff --git a/packages/opencode/src/altimate/tools/finops-workspace.ts b/packages/opencode/src/altimate/tools/finops-workspace.ts new file mode 100644 index 0000000000..e4a8b2052d --- /dev/null +++ b/packages/opencode/src/altimate/tools/finops-workspace.ts @@ -0,0 +1,92 @@ +// altimate_change - new file +// +// The FinOps tools run their SQL through connections configured on this machine — +// `~/.altimate-code/connections.json`, the project's, `ALTIMATE_CODE_CONN_*` — and +// nothing else. A project bound to a workspace whose warehouse credentials live in the +// workspace has none of those, so every `finops_*` call fails, and the workspace's own +// cost skills steer the model to exactly these tools first. A bare `FAILED` there sends +// the model through four failures before it thinks of the engine tool that works. +// +// This is the reason and the way out, appended to every FinOps failure when the +// session's bound workspace serves a type the tool supports. It is advice, not a +// redirect: the FinOps SQL is not routed through the engine (that is the option the +// issue prefers and a larger change — the engine's result shape is not a contract the +// native handlers can parse), so the model is told which engine tool to run the same +// usage-table query through instead. +import * as Precedence from "../workspace/precedence" + +/** Where each served type keeps the usage data the FinOps tools read, so the model + * can write the query the tool would have run. Keyed by canonical local driver type, + * the key `servedInventory` reports. */ +const USAGE_SOURCE: Readonly> = { + snowflake: + "the `SNOWFLAKE.ACCOUNT_USAGE` views (`QUERY_HISTORY`, `WAREHOUSE_METERING_HISTORY`, " + + "`WAREHOUSE_LOAD_HISTORY`, `GRANTS_TO_ROLES`, `GRANTS_TO_USERS`)", + bigquery: "`INFORMATION_SCHEMA.JOBS_BY_PROJECT`", + databricks: "`system.query.history` and `system.billing.usage`", + postgres: "`pg_stat_statements`", +} + +export interface WorkspaceFallback { + workspaceName: string + /** Canonical local driver type the workspace serves, e.g. `snowflake`. */ + type: string + /** The engine execute tool the model should call, e.g. + * `datamate_snowflake_execute_database_query`. */ + modelKey: string +} + +/** + * The workspace-served execute tools for the types a FinOps operation supports, if + * the session is bound to a workspace that serves any. Reads the session's precedence + * snapshot only — the same reachability-filtered projection the awareness section + * uses, so this never names a tool the caller's agent cannot call. + */ +export function workspaceFallbacks(sessionID: string, supportedTypes: readonly string[]): WorkspaceFallback[] { + const precedence = Precedence.forSession(sessionID) + if (!precedence?.enabled) return [] + return Precedence.servedInventory(precedence).flatMap(({ type, served }) => { + if (!supportedTypes.includes(type)) return [] + const execute = served.find((row) => row.capability === "sql_execute") + return execute ? [{ workspaceName: precedence.workspaceName, type, modelKey: execute.modelKey }] : [] + }) +} + +/** The sentence appended to a FinOps failure, or nothing when the workspace serves + * none of the operation's types — then the local error stands on its own. */ +export function workspaceFallbackNote(fallbacks: WorkspaceFallback[]): string | undefined { + if (fallbacks.length === 0) return undefined + const name = fallbacks[0].workspaceName + const routes = fallbacks + .map(({ type, modelKey }) => { + const source = USAGE_SOURCE[type] + return source ? `for ${type}, query ${source} with \`${modelKey}\`` : `for ${type}, use \`${modelKey}\`` + }) + .join("; ") + return ( + `This tool only uses warehouse connections configured on this machine, and workspace "${name}" ` + + `serves ${fallbacks.map((f) => f.type).join(", ")} through its integration engine instead. ` + + `Run the same analysis through the workspace: ${routes}.` + ) +} + +/** + * Attach the workspace note to a failed FinOps result. The failure text is kept — the + * local reason may still be the one to fix — and the engine tool is stamped on the + * metadata so telemetry can tell a failure the model could route around from a dead + * end. + */ +export function withWorkspaceFallback; output: string }>( + sessionID: string, + supportedTypes: readonly string[], + result: T, +): T { + const fallbacks = workspaceFallbacks(sessionID, supportedTypes) + const note = workspaceFallbackNote(fallbacks) + if (!note) return result + return { + ...result, + metadata: { ...result.metadata, workspace_fallback: fallbacks.map((f) => f.modelKey) }, + output: `${result.output}\n\n${note}`, + } +} diff --git a/packages/opencode/test/altimate/finops-workspace-fallback.test.ts b/packages/opencode/test/altimate/finops-workspace-fallback.test.ts new file mode 100644 index 0000000000..635802c47a --- /dev/null +++ b/packages/opencode/test/altimate/finops-workspace-fallback.test.ts @@ -0,0 +1,166 @@ +// altimate_change - new file +// +// #1336 — the FinOps tools resolve only local connections, so in a project bound to a +// workspace whose warehouse credentials live in the workspace they all fail, and the +// failure said nothing about the engine tool that works. These prove the failure now +// names the reason and the engine tool — and only when the workspace really serves a +// type the operation supports, for a caller who may call that tool. +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import { initTool } from "./tool-fixture" +import * as Registry from "../../src/altimate/native/connections/registry" +import { refresh, resetForTests } from "../../src/altimate/workspace/precedence" +import { + withWorkspaceFallback, + workspaceFallbackNote, + workspaceFallbacks, +} from "../../src/altimate/tools/finops-workspace" +import { registerAll as registerFinops } from "../../src/altimate/native/finops/register" +import { DEFAULT_FINOPS_TYPES } from "../../src/altimate/native/finops/warehouse-resolver" +import { ANALYST_RULESET, BIGQUERY_TOOLS, SNOWFLAKE_TOOLS, bindTo } from "./workspace/precedence-fixture" + +const SESSION = "ses_finops_fallback" +const ORIGINAL_PILOT = process.env.ALTIMATE_WORKSPACE +const ORIGINAL_INTEGRATIONS = process.env.ALTIMATE_INTEGRATIONS + +const failed = () => ({ + title: "Warehouse Advice: FAILED", + metadata: { success: false, error: "none configured" }, + output: "Failed to analyze warehouses: none configured", +}) + +beforeEach(() => { + resetForTests() + delete process.env.ALTIMATE_INTEGRATIONS + process.env.ALTIMATE_WORKSPACE = "1" + process.env.ALTIMATE_TELEMETRY_DISABLED = "true" + bindTo(42, "analytics") + // The pilot's promise: no local warehouse connection at all. + Registry.setConfigs({}) +}) + +afterEach(() => { + resetForTests() + Registry.reset() + delete process.env.ALTIMATE_TELEMETRY_DISABLED + if (ORIGINAL_PILOT === undefined) delete process.env.ALTIMATE_WORKSPACE + else process.env.ALTIMATE_WORKSPACE = ORIGINAL_PILOT + if (ORIGINAL_INTEGRATIONS === undefined) delete process.env.ALTIMATE_INTEGRATIONS + else process.env.ALTIMATE_INTEGRATIONS = ORIGINAL_INTEGRATIONS +}) + +describe("workspaceFallbacks", () => { + test("names the engine execute tool for a served type the operation supports", async () => { + await refresh(SESSION, SNOWFLAKE_TOOLS) + expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([ + { workspaceName: "analytics", type: "snowflake", modelKey: "datamate_snowflake_execute_database_query" }, + ]) + }) + + test("is empty for a session with no routing decision", () => { + expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) + }) + + test("is empty when routing is disabled for the session", async () => { + process.env.ALTIMATE_INTEGRATIONS = "local" + await refresh(SESSION, SNOWFLAKE_TOOLS) + expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) + }) + + test("ignores a served type the operation does not support", async () => { + await refresh(SESSION, BIGQUERY_TOOLS) + // The Snowflake-only operations (role hierarchy, user roles) get nothing from a + // workspace that serves only BigQuery. + expect(workspaceFallbacks(SESSION, ["snowflake"])).toEqual([]) + expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES).map((f) => f.type)).toEqual(["bigquery"]) + }) + + test("never names a tool the caller's agent cannot call", async () => { + await refresh(SESSION, SNOWFLAKE_TOOLS, ANALYST_RULESET) + expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) + }) +}) + +describe("workspaceFallbackNote", () => { + test("says why the tool cannot run here and where the same query runs", () => { + const note = workspaceFallbackNote([ + { workspaceName: "analytics", type: "snowflake", modelKey: "datamate_snowflake_execute_database_query" }, + ])! + expect(note).toContain('workspace "analytics" serves snowflake') + expect(note).toContain("configured on this machine") + expect(note).toContain("`SNOWFLAKE.ACCOUNT_USAGE`") + expect(note).toContain("`datamate_snowflake_execute_database_query`") + }) + + test("is nothing when the workspace serves none of the operation's types", () => { + expect(workspaceFallbackNote([])).toBeUndefined() + }) +}) + +describe("withWorkspaceFallback", () => { + test("keeps the local failure and appends the workspace route", async () => { + await refresh(SESSION, SNOWFLAKE_TOOLS) + const result = withWorkspaceFallback(SESSION, DEFAULT_FINOPS_TYPES, failed()) + expect(result.title).toBe("Warehouse Advice: FAILED") + expect(result.output.startsWith("Failed to analyze warehouses: none configured")).toBe(true) + expect(result.output).toContain("`datamate_snowflake_execute_database_query`") + expect(result.metadata as Record).toEqual({ + success: false, + error: "none configured", + workspace_fallback: ["datamate_snowflake_execute_database_query"], + }) + }) + + test("returns the failure untouched when there is nothing to route to", () => { + const input = failed() + const result = withWorkspaceFallback(SESSION, DEFAULT_FINOPS_TYPES, input) + expect(result).toBe(input) + }) +}) + +describe("through the tools", () => { + // The real handlers, registered here rather than through the dispatcher's lazy hook: + // another file's `Dispatcher.reset()` removes that hook for the rest of the process. + beforeEach(() => registerFinops()) + + test("finops_warehouse_advice with no local connection names the engine tool", async () => { + await refresh(SESSION, SNOWFLAKE_TOOLS) + const { FinopsWarehouseAdviceTool } = await import("../../src/altimate/tools/finops-warehouse-advice") + const tool = await initTool(FinopsWarehouseAdviceTool) + const result = await tool.execute({ warehouse: "COMPUTE_WH", days: 14 }, ctx()) + expect(result.title).toBe("Warehouse Advice: FAILED") + // The local reason survives — a missing local connection is still a fact. + expect(result.output).toContain("requires a configured warehouse") + expect(result.output).toContain("`datamate_snowflake_execute_database_query`") + expect(result.metadata.workspace_fallback).toEqual(["datamate_snowflake_execute_database_query"]) + }) + + test("finops_role_hierarchy is not pointed at a workspace that serves only BigQuery", async () => { + await refresh(SESSION, BIGQUERY_TOOLS) + const { FinopsRoleHierarchyTool } = await import("../../src/altimate/tools/finops-role-access") + const tool = await initTool(FinopsRoleHierarchyTool) + const result = await tool.execute({}, ctx()) + expect(result.title).toBe("Role Hierarchy: FAILED") + expect(result.output).not.toContain("datamate_") + expect(result.metadata.workspace_fallback).toBeUndefined() + }) + + test("an unbound project gets the plain local failure", async () => { + const { FinopsAnalyzeCreditsTool } = await import("../../src/altimate/tools/finops-analyze-credits") + const tool = await initTool(FinopsAnalyzeCreditsTool) + const result = await tool.execute({ days: 7 }, ctx()) + expect(result.title).toBe("Credit Analysis: FAILED") + expect(result.output).not.toContain("datamate_") + expect(result.output).not.toContain("workspace") + }) +}) + +function ctx(): any { + return { + sessionID: SESSION, + messageID: "msg", + agent: "build", + abort: new AbortController().signal, + messages: [], + metadata: () => {}, + } +} From 3264426a43bddea47db529e5225f02bd64c0d63b Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 21 Sep 2026 22:26:09 +0530 Subject: [PATCH 2/6] fix(finops): operation-specific tables in the workspace note, canonical identity, undetermined marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bot review on #1346. - The note names the tables the failing operation's own handler reads, per served type (grants live in `GRANTS_TO_ROLES` / `OBJECT_PRIVILEGES` / `table_privileges`, not in the usage views); a pair with no known table names the engine tool alone - The workspace is rendered with `workspaceLabel(name, id)` — quotes in a customer-authored name cannot break the sentence, and the id rides along - Routing disabled for uncertainty (unattributed, binding unreadable, derive failed) marks the failure `precedence: "undetermined"` and says the workspace could not be consulted; deliberate disablement stays a plain local failure - Test restores the prior `ALTIMATE_TELEMETRY_DISABLED` Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- .../altimate/tools/finops-analyze-credits.ts | 4 +- .../tools/finops-expensive-queries.ts | 4 +- .../altimate/tools/finops-query-history.ts | 4 +- .../src/altimate/tools/finops-role-access.ts | 12 +- .../altimate/tools/finops-unused-resources.ts | 4 +- .../altimate/tools/finops-warehouse-advice.ts | 4 +- .../src/altimate/tools/finops-workspace.ts | 108 +++++++++++++++--- .../finops-workspace-fallback.test.ts | 72 +++++++++--- 8 files changed, 163 insertions(+), 49 deletions(-) diff --git a/packages/opencode/src/altimate/tools/finops-analyze-credits.ts b/packages/opencode/src/altimate/tools/finops-analyze-credits.ts index 9338fd19e2..4f55e33447 100644 --- a/packages/opencode/src/altimate/tools/finops-analyze-credits.ts +++ b/packages/opencode/src/altimate/tools/finops-analyze-credits.ts @@ -95,7 +95,7 @@ export const FinopsAnalyzeCreditsTool = Tool.define("finops_analyze_credits", { if (!result.success) { const error = result.error ?? "Unknown error" - return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "analyze_credits", DEFAULT_FINOPS_TYPES, { title: "Credit Analysis: FAILED", metadata: { success: false, total_credits: 0, error }, output: `Failed to analyze credits: ${error}`, @@ -116,7 +116,7 @@ export const FinopsAnalyzeCreditsTool = Tool.define("finops_analyze_credits", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "analyze_credits", DEFAULT_FINOPS_TYPES, { title: "Credit Analysis: ERROR", metadata: { success: false, total_credits: 0, error: msg }, output: `Failed to analyze credits: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-expensive-queries.ts b/packages/opencode/src/altimate/tools/finops-expensive-queries.ts index 03107d962f..d5e36e5ed9 100644 --- a/packages/opencode/src/altimate/tools/finops-expensive-queries.ts +++ b/packages/opencode/src/altimate/tools/finops-expensive-queries.ts @@ -60,7 +60,7 @@ export const FinopsExpensiveQueriesTool = Tool.define("finops_expensive_queries" if (!result.success) { const error = result.error ?? "Unknown error" - return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "expensive_queries", DEFAULT_FINOPS_TYPES, { title: "Expensive Queries: FAILED", metadata: { success: false, query_count: 0, error }, output: `Failed to find expensive queries: ${error}`, @@ -74,7 +74,7 @@ export const FinopsExpensiveQueriesTool = Tool.define("finops_expensive_queries" } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "expensive_queries", DEFAULT_FINOPS_TYPES, { title: "Expensive Queries: ERROR", metadata: { success: false, query_count: 0, error: msg }, output: `Failed to find expensive queries: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-query-history.ts b/packages/opencode/src/altimate/tools/finops-query-history.ts index 8ab7c782fc..73580d92dd 100644 --- a/packages/opencode/src/altimate/tools/finops-query-history.ts +++ b/packages/opencode/src/altimate/tools/finops-query-history.ts @@ -84,7 +84,7 @@ export const FinopsQueryHistoryTool = Tool.define("finops_query_history", { if (!result.success) { const error = result.error ?? "Unknown error" - return withWorkspaceFallback(ctx.sessionID, QUERY_HISTORY_SUPPORTED_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "query_history", QUERY_HISTORY_SUPPORTED_TYPES, { title: "Query History: FAILED", metadata: { success: false, query_count: 0, error }, output: `Failed to fetch query history: ${error}`, @@ -99,7 +99,7 @@ export const FinopsQueryHistoryTool = Tool.define("finops_query_history", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, QUERY_HISTORY_SUPPORTED_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "query_history", QUERY_HISTORY_SUPPORTED_TYPES, { title: "Query History: ERROR", metadata: { success: false, query_count: 0, error: msg }, output: `Failed to fetch query history: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-role-access.ts b/packages/opencode/src/altimate/tools/finops-role-access.ts index 1fef960cad..f77fe4fc76 100644 --- a/packages/opencode/src/altimate/tools/finops-role-access.ts +++ b/packages/opencode/src/altimate/tools/finops-role-access.ts @@ -124,7 +124,7 @@ export const FinopsRoleGrantsTool = Tool.define("finops_role_grants", { }) if (!result.success) { - return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "role_grants", DEFAULT_FINOPS_TYPES, { title: "Role Grants: FAILED", metadata: { success: false, grant_count: 0 }, output: `Failed to query grants: ${result.error ?? "Unknown error"}`, @@ -138,7 +138,7 @@ export const FinopsRoleGrantsTool = Tool.define("finops_role_grants", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "role_grants", DEFAULT_FINOPS_TYPES, { title: "Role Grants: ERROR", metadata: { success: false, grant_count: 0, error: msg }, output: `Failed to query grants: ${msg}`, @@ -162,7 +162,7 @@ export const FinopsRoleHierarchyTool = Tool.define("finops_role_hierarchy", { const result = await Dispatcher.call("finops.role_hierarchy", { warehouse: args.warehouse }) if (!result.success) { - return withWorkspaceFallback(ctx.sessionID, SNOWFLAKE_ONLY_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "role_hierarchy", SNOWFLAKE_ONLY_TYPES, { title: "Role Hierarchy: FAILED", metadata: { success: false, role_count: 0 }, output: `Failed to query role hierarchy: ${result.error ?? "Unknown error"}`, @@ -176,7 +176,7 @@ export const FinopsRoleHierarchyTool = Tool.define("finops_role_hierarchy", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, SNOWFLAKE_ONLY_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "role_hierarchy", SNOWFLAKE_ONLY_TYPES, { title: "Role Hierarchy: ERROR", metadata: { success: false, role_count: 0, error: msg }, output: `Failed to query role hierarchy: ${msg}`, @@ -206,7 +206,7 @@ export const FinopsUserRolesTool = Tool.define("finops_user_roles", { }) if (!result.success) { - return withWorkspaceFallback(ctx.sessionID, SNOWFLAKE_ONLY_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "user_roles", SNOWFLAKE_ONLY_TYPES, { title: "User Roles: FAILED", metadata: { success: false, assignment_count: 0 }, output: `Failed to query user roles: ${result.error ?? "Unknown error"}`, @@ -220,7 +220,7 @@ export const FinopsUserRolesTool = Tool.define("finops_user_roles", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, SNOWFLAKE_ONLY_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "user_roles", SNOWFLAKE_ONLY_TYPES, { title: "User Roles: ERROR", metadata: { success: false, assignment_count: 0, error: msg }, output: `Failed to query user roles: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-unused-resources.ts b/packages/opencode/src/altimate/tools/finops-unused-resources.ts index 442d219f52..aef9680dd2 100644 --- a/packages/opencode/src/altimate/tools/finops-unused-resources.ts +++ b/packages/opencode/src/altimate/tools/finops-unused-resources.ts @@ -83,7 +83,7 @@ export const FinopsUnusedResourcesTool = Tool.define("finops_unused_resources", if (!result.success) { const error = result.error ?? "Unknown error" - return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "unused_resources", DEFAULT_FINOPS_TYPES, { title: "Unused Resources: FAILED", metadata: { success: false, unused_count: 0, error }, output: `Failed to find unused resources: ${error}`, @@ -100,7 +100,7 @@ export const FinopsUnusedResourcesTool = Tool.define("finops_unused_resources", } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "unused_resources", DEFAULT_FINOPS_TYPES, { title: "Unused Resources: ERROR", metadata: { success: false, unused_count: 0, error: msg }, output: `Failed to find unused resources: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts b/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts index b6d1d4ed16..3c65ab1786 100644 --- a/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts +++ b/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts @@ -104,7 +104,7 @@ export const FinopsWarehouseAdviceTool = Tool.define("finops_warehouse_advice", if (!result.success) { const error = result.error ?? "Unknown error" - return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "warehouse_advice", DEFAULT_FINOPS_TYPES, { title: "Warehouse Advice: FAILED", metadata: { success: false, recommendation_count: 0, error }, output: `Failed to analyze warehouses: ${error}`, @@ -127,7 +127,7 @@ export const FinopsWarehouseAdviceTool = Tool.define("finops_warehouse_advice", } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, DEFAULT_FINOPS_TYPES, { + return withWorkspaceFallback(ctx.sessionID, "warehouse_advice", DEFAULT_FINOPS_TYPES, { title: "Warehouse Advice: ERROR", metadata: { success: false, recommendation_count: 0, error: msg }, output: `Failed to analyze warehouses: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-workspace.ts b/packages/opencode/src/altimate/tools/finops-workspace.ts index e4a8b2052d..f094f37c59 100644 --- a/packages/opencode/src/altimate/tools/finops-workspace.ts +++ b/packages/opencode/src/altimate/tools/finops-workspace.ts @@ -14,21 +14,64 @@ // native handlers can parse), so the model is told which engine tool to run the same // usage-table query through instead. import * as Precedence from "../workspace/precedence" +import { workspaceLabel } from "../workspace/workspace-name" -/** Where each served type keeps the usage data the FinOps tools read, so the model - * can write the query the tool would have run. Keyed by canonical local driver type, - * the key `servedInventory` reports. */ -const USAGE_SOURCE: Readonly> = { - snowflake: - "the `SNOWFLAKE.ACCOUNT_USAGE` views (`QUERY_HISTORY`, `WAREHOUSE_METERING_HISTORY`, " + - "`WAREHOUSE_LOAD_HISTORY`, `GRANTS_TO_ROLES`, `GRANTS_TO_USERS`)", - bigquery: "`INFORMATION_SCHEMA.JOBS_BY_PROJECT`", - databricks: "`system.query.history` and `system.billing.usage`", - postgres: "`pg_stat_statements`", +/** The FinOps operations, named as the wrapper knows them. */ +export type FinopsOperation = + | "query_history" + | "analyze_credits" + | "expensive_queries" + | "warehouse_advice" + | "unused_resources" + | "role_grants" + | "role_hierarchy" + | "user_roles" + +/** Where each operation's data lives, per served type — the tables the native + * handler itself reads (`native/finops/*.ts`), so the model writes the query the + * tool would have run. Keyed by canonical local driver type, the key + * `servedInventory` reports; an operation/type pair with no entry gets the tool + * name alone rather than a table that holds something else. */ +const SOURCE: Readonly>>> = { + query_history: { + snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY`", + bigquery: "`INFORMATION_SCHEMA.JOBS`", + databricks: "`system.query.history`", + postgres: "`pg_stat_statements`", + }, + analyze_credits: { + snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.WAREHOUSE_METERING_HISTORY` and `QUERY_HISTORY`", + bigquery: "`INFORMATION_SCHEMA.JOBS`", + databricks: "`system.billing.usage` and `system.query.history`", + }, + expensive_queries: { + snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY`", + bigquery: "`INFORMATION_SCHEMA.JOBS`", + databricks: "`system.query.history`", + }, + warehouse_advice: { + snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.WAREHOUSE_LOAD_HISTORY` and `QUERY_HISTORY`", + bigquery: "`INFORMATION_SCHEMA.JOBS` and `JOBS_TIMELINE`", + databricks: "`system.compute.warehouse_events` and `system.query.history`", + }, + unused_resources: { + snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.TABLE_STORAGE_METRICS`, `ACCESS_HISTORY` and `WAREHOUSES`", + bigquery: "`INFORMATION_SCHEMA.TABLE_STORAGE`", + databricks: "`system.information_schema.tables`", + }, + role_grants: { + snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_ROLES`", + bigquery: "`INFORMATION_SCHEMA.OBJECT_PRIVILEGES`", + databricks: "`system.information_schema.table_privileges`", + }, + role_hierarchy: { snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_ROLES` (granted_on = 'ROLE')" }, + user_roles: { snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_USERS`" }, } export interface WorkspaceFallback { workspaceName: string + /** The bound workspace's numeric id, the stable half of its identity. */ + workspaceId?: string /** Canonical local driver type the workspace serves, e.g. `snowflake`. */ type: string /** The engine execute tool the model should call, e.g. @@ -48,28 +91,46 @@ export function workspaceFallbacks(sessionID: string, supportedTypes: readonly s return Precedence.servedInventory(precedence).flatMap(({ type, served }) => { if (!supportedTypes.includes(type)) return [] const execute = served.find((row) => row.capability === "sql_execute") - return execute ? [{ workspaceName: precedence.workspaceName, type, modelKey: execute.modelKey }] : [] + return execute + ? [{ workspaceName: precedence.workspaceName, workspaceId: precedence.workspaceId, type, modelKey: execute.modelKey }] + : [] }) } /** The sentence appended to a FinOps failure, or nothing when the workspace serves * none of the operation's types — then the local error stands on its own. */ -export function workspaceFallbackNote(fallbacks: WorkspaceFallback[]): string | undefined { +export function workspaceFallbackNote(operation: FinopsOperation, fallbacks: WorkspaceFallback[]): string | undefined { if (fallbacks.length === 0) return undefined - const name = fallbacks[0].workspaceName + // The canonical identity rendering: the name is customer-authored and is quoted + // safely, and the id — the stable half — rides along. + const label = workspaceLabel(fallbacks[0].workspaceName, fallbacks[0].workspaceId) const routes = fallbacks .map(({ type, modelKey }) => { - const source = USAGE_SOURCE[type] + const source = SOURCE[operation][type] return source ? `for ${type}, query ${source} with \`${modelKey}\`` : `for ${type}, use \`${modelKey}\`` }) .join("; ") return ( - `This tool only uses warehouse connections configured on this machine, and workspace "${name}" ` + + `This tool only uses warehouse connections configured on this machine, and workspace ${label} ` + `serves ${fallbacks.map((f) => f.type).join(", ")} through its integration engine instead. ` + `Run the same analysis through the workspace: ${routes}.` ) } +/** Why the fallback could not be decided, when routing is disabled for a reason that + * is uncertainty rather than a choice. Deliberate disablement (unbound, pilot off, + * `--integrations=local`, nothing materialised) says nothing: that is the plain local + * failure. Uncertainty must say so (the precedence module's first claim), so the + * failure is marked `undetermined` and says the workspace could not be consulted. */ +function undeterminedNote(sessionID: string): string | undefined { + const reason = Precedence.forSession(sessionID)?.disabledReason + if (reason !== "unattributed" && reason !== "binding-unreadable" && reason !== "derive-failed") return undefined + return ( + "Whether the linked workspace serves this connection type could not be determined this turn " + + "(no routing decision was available), so no workspace alternative is offered here." + ) +} + /** * Attach the workspace note to a failed FinOps result. The failure text is kept — the * local reason may still be the one to fix — and the engine tool is stamped on the @@ -78,15 +139,24 @@ export function workspaceFallbackNote(fallbacks: WorkspaceFallback[]): string | */ export function withWorkspaceFallback; output: string }>( sessionID: string, + operation: FinopsOperation, supportedTypes: readonly string[], result: T, ): T { const fallbacks = workspaceFallbacks(sessionID, supportedTypes) - const note = workspaceFallbackNote(fallbacks) - if (!note) return result + const note = workspaceFallbackNote(operation, fallbacks) + if (note) { + return { + ...result, + metadata: { ...result.metadata, workspace_fallback: fallbacks.map((f) => f.modelKey) }, + output: `${result.output}\n\n${note}`, + } + } + const undetermined = undeterminedNote(sessionID) + if (!undetermined) return result return { ...result, - metadata: { ...result.metadata, workspace_fallback: fallbacks.map((f) => f.modelKey) }, - output: `${result.output}\n\n${note}`, + metadata: { ...result.metadata, precedence: "undetermined" }, + output: `${result.output}\n\n${undetermined}`, } } diff --git a/packages/opencode/test/altimate/finops-workspace-fallback.test.ts b/packages/opencode/test/altimate/finops-workspace-fallback.test.ts index 635802c47a..d0dc984a0b 100644 --- a/packages/opencode/test/altimate/finops-workspace-fallback.test.ts +++ b/packages/opencode/test/altimate/finops-workspace-fallback.test.ts @@ -8,7 +8,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test" import { initTool } from "./tool-fixture" import * as Registry from "../../src/altimate/native/connections/registry" -import { refresh, resetForTests } from "../../src/altimate/workspace/precedence" +import { precedenceInternals, refresh, resetForTests } from "../../src/altimate/workspace/precedence" import { withWorkspaceFallback, workspaceFallbackNote, @@ -21,8 +21,9 @@ import { ANALYST_RULESET, BIGQUERY_TOOLS, SNOWFLAKE_TOOLS, bindTo } from "./work const SESSION = "ses_finops_fallback" const ORIGINAL_PILOT = process.env.ALTIMATE_WORKSPACE const ORIGINAL_INTEGRATIONS = process.env.ALTIMATE_INTEGRATIONS +const ORIGINAL_TELEMETRY = process.env.ALTIMATE_TELEMETRY_DISABLED -const failed = () => ({ +const failed = (): { title: string; metadata: Record; output: string } => ({ title: "Warehouse Advice: FAILED", metadata: { success: false, error: "none configured" }, output: "Failed to analyze warehouses: none configured", @@ -41,7 +42,8 @@ beforeEach(() => { afterEach(() => { resetForTests() Registry.reset() - delete process.env.ALTIMATE_TELEMETRY_DISABLED + if (ORIGINAL_TELEMETRY === undefined) delete process.env.ALTIMATE_TELEMETRY_DISABLED + else process.env.ALTIMATE_TELEMETRY_DISABLED = ORIGINAL_TELEMETRY if (ORIGINAL_PILOT === undefined) delete process.env.ALTIMATE_WORKSPACE else process.env.ALTIMATE_WORKSPACE = ORIGINAL_PILOT if (ORIGINAL_INTEGRATIONS === undefined) delete process.env.ALTIMATE_INTEGRATIONS @@ -52,7 +54,7 @@ describe("workspaceFallbacks", () => { test("names the engine execute tool for a served type the operation supports", async () => { await refresh(SESSION, SNOWFLAKE_TOOLS) expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([ - { workspaceName: "analytics", type: "snowflake", modelKey: "datamate_snowflake_execute_database_query" }, + { workspaceName: "analytics", workspaceId: "42", type: "snowflake", modelKey: "datamate_snowflake_execute_database_query" }, ]) }) @@ -81,29 +83,51 @@ describe("workspaceFallbacks", () => { }) describe("workspaceFallbackNote", () => { - test("says why the tool cannot run here and where the same query runs", () => { - const note = workspaceFallbackNote([ - { workspaceName: "analytics", type: "snowflake", modelKey: "datamate_snowflake_execute_database_query" }, - ])! - expect(note).toContain('workspace "analytics" serves snowflake') + const snowflake = [ + { workspaceName: "analytics", workspaceId: "42", type: "snowflake", modelKey: "datamate_snowflake_execute_database_query" }, + ] + + test("says why the tool cannot run here and where the same query runs, with the canonical identity", () => { + const note = workspaceFallbackNote("warehouse_advice", snowflake)! + expect(note).toContain('workspace "analytics" (id 42) serves snowflake') expect(note).toContain("configured on this machine") - expect(note).toContain("`SNOWFLAKE.ACCOUNT_USAGE`") + expect(note).toContain("`SNOWFLAKE.ACCOUNT_USAGE.WAREHOUSE_LOAD_HISTORY`") expect(note).toContain("`datamate_snowflake_execute_database_query`") }) + test("the tables named are the operation's own, not a generic usage view", () => { + // Grants do not live in the usage tables; a wrong table on a real failure path + // sends the model down a dead end. (bot review) + const bigquery = [{ workspaceName: "analytics", type: "bigquery", modelKey: "datamate_bigquery_execute_database_query" }] + expect(workspaceFallbackNote("role_grants", bigquery)).toContain("`INFORMATION_SCHEMA.OBJECT_PRIVILEGES`") + expect(workspaceFallbackNote("role_grants", bigquery)).not.toContain("JOBS") + expect(workspaceFallbackNote("query_history", bigquery)).toContain("`INFORMATION_SCHEMA.JOBS`") + const databricks = [{ workspaceName: "analytics", type: "databricks", modelKey: "datamate_databricks_execute_sql" }] + expect(workspaceFallbackNote("role_grants", databricks)).toContain("`system.information_schema.table_privileges`") + expect(workspaceFallbackNote("user_roles", snowflake)).toContain("`SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_USERS`") + // No table known for the pair: the tool alone, never a table that holds something else. + expect(workspaceFallbackNote("user_roles", databricks)).toContain("for databricks, use `datamate_databricks_execute_sql`") + }) + + test("a workspace name with quotes cannot break the delimiters", () => { + const note = workspaceFallbackNote("query_history", [{ ...snowflake[0], workspaceName: 'a"b\nc' }])! + expect(note).toContain('workspace "a\\"b c" (id 42)') + expect(note).not.toContain("\n") + }) + test("is nothing when the workspace serves none of the operation's types", () => { - expect(workspaceFallbackNote([])).toBeUndefined() + expect(workspaceFallbackNote("query_history", [])).toBeUndefined() }) }) describe("withWorkspaceFallback", () => { test("keeps the local failure and appends the workspace route", async () => { await refresh(SESSION, SNOWFLAKE_TOOLS) - const result = withWorkspaceFallback(SESSION, DEFAULT_FINOPS_TYPES, failed()) + const result = withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, failed()) expect(result.title).toBe("Warehouse Advice: FAILED") expect(result.output.startsWith("Failed to analyze warehouses: none configured")).toBe(true) expect(result.output).toContain("`datamate_snowflake_execute_database_query`") - expect(result.metadata as Record).toEqual({ + expect(result.metadata).toEqual({ success: false, error: "none configured", workspace_fallback: ["datamate_snowflake_execute_database_query"], @@ -112,9 +136,29 @@ describe("withWorkspaceFallback", () => { test("returns the failure untouched when there is nothing to route to", () => { const input = failed() - const result = withWorkspaceFallback(SESSION, DEFAULT_FINOPS_TYPES, input) + const result = withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, input) expect(result).toBe(input) }) + + test("routing that could not be determined is said, and marked, rather than passed off as local-only", async () => { + // The engine could not be attributed to the bound workspace this turn: the + // snapshot is disabled for uncertainty, not by choice. (bot review) + precedenceInternals.attributedTo = async () => "999" + const precedence = await refresh(SESSION, SNOWFLAKE_TOOLS) + expect(precedence.disabledReason).toBe("unattributed") + const result = withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, failed()) + expect(result.metadata.precedence).toBe("undetermined") + expect(result.output).toContain("could not be determined this turn") + expect(result.output).not.toContain("datamate_") + }) + + test("deliberate disablement stays a plain local failure", async () => { + process.env.ALTIMATE_INTEGRATIONS = "local" + const precedence = await refresh(SESSION, SNOWFLAKE_TOOLS) + expect(precedence.disabledReason).toBe("escape-hatch") + const input = failed() + expect(withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, input)).toBe(input) + }) }) describe("through the tools", () => { From 14aa1a1ef61cc9a2aae8990a4439ad98a878f870 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 21 Sep 2026 23:04:25 +0530 Subject: [PATCH 3/6] fix(finops): re-validate the snapshot, say "unknown" like check() does, runnable recipes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review of #1346 (gpt-5.6-sol). - The note trusted the cached precedence snapshot; a project re-linked while the call ran could be pointed at the workspace it had left. The helper is async now and re-validates with `snapshotState` before naming a workspace, as `check()` does before a redirect; relinked/unreadable are said and marked `undetermined` - No snapshot at all (a caller that never resolved tools, an evicted entry) was a silent local failure; it is the unknown `check()` reports - BigQuery recipes named bare `INFORMATION_SCHEMA` views, which are only reachable region-qualified (`bq-utils.ts`): they read `region-.INFORMATION_SCHEMA.…` now. The Snowflake unused-resource recipe adds `QUERY_HISTORY`; warehouse advice adds `SHOW WAREHOUSES` - Tests: every one of the eight wrappers through the real handler (unwrapping any one fails it — checked by mutation); an unbound snapshot stays plain; no snapshot is undetermined; a re-linked project gets no recommendation; every BigQuery recipe is region-qualified Not changed: a FinOps call that SUCCEEDS against an unrelated local connection while the project is bound elsewhere says nothing about it (pre-existing auto-pick behaviour), and the other registry-backed tools (`schema_index`, live PII, data_diff, warehouse_test/list) still fail bare — both are follow-ups, not this issue. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- .../altimate/tools/finops-analyze-credits.ts | 4 +- .../tools/finops-expensive-queries.ts | 4 +- .../altimate/tools/finops-query-history.ts | 4 +- .../src/altimate/tools/finops-role-access.ts | 12 +-- .../altimate/tools/finops-unused-resources.ts | 4 +- .../altimate/tools/finops-warehouse-advice.ts | 4 +- .../src/altimate/tools/finops-workspace.ts | 75 ++++++++++------ .../finops-workspace-fallback.test.ts | 86 ++++++++++++++++--- 8 files changed, 136 insertions(+), 57 deletions(-) diff --git a/packages/opencode/src/altimate/tools/finops-analyze-credits.ts b/packages/opencode/src/altimate/tools/finops-analyze-credits.ts index 4f55e33447..6155b300ef 100644 --- a/packages/opencode/src/altimate/tools/finops-analyze-credits.ts +++ b/packages/opencode/src/altimate/tools/finops-analyze-credits.ts @@ -95,7 +95,7 @@ export const FinopsAnalyzeCreditsTool = Tool.define("finops_analyze_credits", { if (!result.success) { const error = result.error ?? "Unknown error" - return withWorkspaceFallback(ctx.sessionID, "analyze_credits", DEFAULT_FINOPS_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "analyze_credits", DEFAULT_FINOPS_TYPES, { title: "Credit Analysis: FAILED", metadata: { success: false, total_credits: 0, error }, output: `Failed to analyze credits: ${error}`, @@ -116,7 +116,7 @@ export const FinopsAnalyzeCreditsTool = Tool.define("finops_analyze_credits", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, "analyze_credits", DEFAULT_FINOPS_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "analyze_credits", DEFAULT_FINOPS_TYPES, { title: "Credit Analysis: ERROR", metadata: { success: false, total_credits: 0, error: msg }, output: `Failed to analyze credits: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-expensive-queries.ts b/packages/opencode/src/altimate/tools/finops-expensive-queries.ts index d5e36e5ed9..a2707b2917 100644 --- a/packages/opencode/src/altimate/tools/finops-expensive-queries.ts +++ b/packages/opencode/src/altimate/tools/finops-expensive-queries.ts @@ -60,7 +60,7 @@ export const FinopsExpensiveQueriesTool = Tool.define("finops_expensive_queries" if (!result.success) { const error = result.error ?? "Unknown error" - return withWorkspaceFallback(ctx.sessionID, "expensive_queries", DEFAULT_FINOPS_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "expensive_queries", DEFAULT_FINOPS_TYPES, { title: "Expensive Queries: FAILED", metadata: { success: false, query_count: 0, error }, output: `Failed to find expensive queries: ${error}`, @@ -74,7 +74,7 @@ export const FinopsExpensiveQueriesTool = Tool.define("finops_expensive_queries" } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, "expensive_queries", DEFAULT_FINOPS_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "expensive_queries", DEFAULT_FINOPS_TYPES, { title: "Expensive Queries: ERROR", metadata: { success: false, query_count: 0, error: msg }, output: `Failed to find expensive queries: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-query-history.ts b/packages/opencode/src/altimate/tools/finops-query-history.ts index 73580d92dd..e7a5afec62 100644 --- a/packages/opencode/src/altimate/tools/finops-query-history.ts +++ b/packages/opencode/src/altimate/tools/finops-query-history.ts @@ -84,7 +84,7 @@ export const FinopsQueryHistoryTool = Tool.define("finops_query_history", { if (!result.success) { const error = result.error ?? "Unknown error" - return withWorkspaceFallback(ctx.sessionID, "query_history", QUERY_HISTORY_SUPPORTED_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "query_history", QUERY_HISTORY_SUPPORTED_TYPES, { title: "Query History: FAILED", metadata: { success: false, query_count: 0, error }, output: `Failed to fetch query history: ${error}`, @@ -99,7 +99,7 @@ export const FinopsQueryHistoryTool = Tool.define("finops_query_history", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, "query_history", QUERY_HISTORY_SUPPORTED_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "query_history", QUERY_HISTORY_SUPPORTED_TYPES, { title: "Query History: ERROR", metadata: { success: false, query_count: 0, error: msg }, output: `Failed to fetch query history: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-role-access.ts b/packages/opencode/src/altimate/tools/finops-role-access.ts index f77fe4fc76..3bfac2049d 100644 --- a/packages/opencode/src/altimate/tools/finops-role-access.ts +++ b/packages/opencode/src/altimate/tools/finops-role-access.ts @@ -124,7 +124,7 @@ export const FinopsRoleGrantsTool = Tool.define("finops_role_grants", { }) if (!result.success) { - return withWorkspaceFallback(ctx.sessionID, "role_grants", DEFAULT_FINOPS_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "role_grants", DEFAULT_FINOPS_TYPES, { title: "Role Grants: FAILED", metadata: { success: false, grant_count: 0 }, output: `Failed to query grants: ${result.error ?? "Unknown error"}`, @@ -138,7 +138,7 @@ export const FinopsRoleGrantsTool = Tool.define("finops_role_grants", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, "role_grants", DEFAULT_FINOPS_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "role_grants", DEFAULT_FINOPS_TYPES, { title: "Role Grants: ERROR", metadata: { success: false, grant_count: 0, error: msg }, output: `Failed to query grants: ${msg}`, @@ -162,7 +162,7 @@ export const FinopsRoleHierarchyTool = Tool.define("finops_role_hierarchy", { const result = await Dispatcher.call("finops.role_hierarchy", { warehouse: args.warehouse }) if (!result.success) { - return withWorkspaceFallback(ctx.sessionID, "role_hierarchy", SNOWFLAKE_ONLY_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "role_hierarchy", SNOWFLAKE_ONLY_TYPES, { title: "Role Hierarchy: FAILED", metadata: { success: false, role_count: 0 }, output: `Failed to query role hierarchy: ${result.error ?? "Unknown error"}`, @@ -176,7 +176,7 @@ export const FinopsRoleHierarchyTool = Tool.define("finops_role_hierarchy", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, "role_hierarchy", SNOWFLAKE_ONLY_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "role_hierarchy", SNOWFLAKE_ONLY_TYPES, { title: "Role Hierarchy: ERROR", metadata: { success: false, role_count: 0, error: msg }, output: `Failed to query role hierarchy: ${msg}`, @@ -206,7 +206,7 @@ export const FinopsUserRolesTool = Tool.define("finops_user_roles", { }) if (!result.success) { - return withWorkspaceFallback(ctx.sessionID, "user_roles", SNOWFLAKE_ONLY_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "user_roles", SNOWFLAKE_ONLY_TYPES, { title: "User Roles: FAILED", metadata: { success: false, assignment_count: 0 }, output: `Failed to query user roles: ${result.error ?? "Unknown error"}`, @@ -220,7 +220,7 @@ export const FinopsUserRolesTool = Tool.define("finops_user_roles", { } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, "user_roles", SNOWFLAKE_ONLY_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "user_roles", SNOWFLAKE_ONLY_TYPES, { title: "User Roles: ERROR", metadata: { success: false, assignment_count: 0, error: msg }, output: `Failed to query user roles: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-unused-resources.ts b/packages/opencode/src/altimate/tools/finops-unused-resources.ts index aef9680dd2..4404b77131 100644 --- a/packages/opencode/src/altimate/tools/finops-unused-resources.ts +++ b/packages/opencode/src/altimate/tools/finops-unused-resources.ts @@ -83,7 +83,7 @@ export const FinopsUnusedResourcesTool = Tool.define("finops_unused_resources", if (!result.success) { const error = result.error ?? "Unknown error" - return withWorkspaceFallback(ctx.sessionID, "unused_resources", DEFAULT_FINOPS_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "unused_resources", DEFAULT_FINOPS_TYPES, { title: "Unused Resources: FAILED", metadata: { success: false, unused_count: 0, error }, output: `Failed to find unused resources: ${error}`, @@ -100,7 +100,7 @@ export const FinopsUnusedResourcesTool = Tool.define("finops_unused_resources", } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, "unused_resources", DEFAULT_FINOPS_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "unused_resources", DEFAULT_FINOPS_TYPES, { title: "Unused Resources: ERROR", metadata: { success: false, unused_count: 0, error: msg }, output: `Failed to find unused resources: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts b/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts index 3c65ab1786..6c7bd28df3 100644 --- a/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts +++ b/packages/opencode/src/altimate/tools/finops-warehouse-advice.ts @@ -104,7 +104,7 @@ export const FinopsWarehouseAdviceTool = Tool.define("finops_warehouse_advice", if (!result.success) { const error = result.error ?? "Unknown error" - return withWorkspaceFallback(ctx.sessionID, "warehouse_advice", DEFAULT_FINOPS_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "warehouse_advice", DEFAULT_FINOPS_TYPES, { title: "Warehouse Advice: FAILED", metadata: { success: false, recommendation_count: 0, error }, output: `Failed to analyze warehouses: ${error}`, @@ -127,7 +127,7 @@ export const FinopsWarehouseAdviceTool = Tool.define("finops_warehouse_advice", } } catch (e) { const msg = e instanceof Error ? e.message : String(e) - return withWorkspaceFallback(ctx.sessionID, "warehouse_advice", DEFAULT_FINOPS_TYPES, { + return await withWorkspaceFallback(ctx.sessionID, "warehouse_advice", DEFAULT_FINOPS_TYPES, { title: "Warehouse Advice: ERROR", metadata: { success: false, recommendation_count: 0, error: msg }, output: `Failed to analyze warehouses: ${msg}`, diff --git a/packages/opencode/src/altimate/tools/finops-workspace.ts b/packages/opencode/src/altimate/tools/finops-workspace.ts index f094f37c59..c2f48277f4 100644 --- a/packages/opencode/src/altimate/tools/finops-workspace.ts +++ b/packages/opencode/src/altimate/tools/finops-workspace.ts @@ -31,37 +31,40 @@ export type FinopsOperation = * handler itself reads (`native/finops/*.ts`), so the model writes the query the * tool would have run. Keyed by canonical local driver type, the key * `servedInventory` reports; an operation/type pair with no entry gets the tool - * name alone rather than a table that holds something else. */ + * name alone rather than a table that holds something else. BigQuery's + * INFORMATION_SCHEMA views are only reachable region-qualified (`bq-utils.ts`), and + * the snapshot does not carry the integration's region, so the placeholder is + * spelled out rather than a bare name that would fail again. */ const SOURCE: Readonly>>> = { query_history: { snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY`", - bigquery: "`INFORMATION_SCHEMA.JOBS`", + bigquery: "`region-.INFORMATION_SCHEMA.JOBS`", databricks: "`system.query.history`", postgres: "`pg_stat_statements`", }, analyze_credits: { snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.WAREHOUSE_METERING_HISTORY` and `QUERY_HISTORY`", - bigquery: "`INFORMATION_SCHEMA.JOBS`", + bigquery: "`region-.INFORMATION_SCHEMA.JOBS`", databricks: "`system.billing.usage` and `system.query.history`", }, expensive_queries: { snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY`", - bigquery: "`INFORMATION_SCHEMA.JOBS`", + bigquery: "`region-.INFORMATION_SCHEMA.JOBS`", databricks: "`system.query.history`", }, warehouse_advice: { - snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.WAREHOUSE_LOAD_HISTORY` and `QUERY_HISTORY`", - bigquery: "`INFORMATION_SCHEMA.JOBS` and `JOBS_TIMELINE`", + snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.WAREHOUSE_LOAD_HISTORY` and `QUERY_HISTORY`, plus `SHOW WAREHOUSES`", + bigquery: "`region-.INFORMATION_SCHEMA.JOBS` and `JOBS_TIMELINE`", databricks: "`system.compute.warehouse_events` and `system.query.history`", }, unused_resources: { - snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.TABLE_STORAGE_METRICS`, `ACCESS_HISTORY` and `WAREHOUSES`", - bigquery: "`INFORMATION_SCHEMA.TABLE_STORAGE`", + snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.TABLE_STORAGE_METRICS`, `ACCESS_HISTORY`, `WAREHOUSES` and `QUERY_HISTORY`", + bigquery: "`region-.INFORMATION_SCHEMA.TABLE_STORAGE`", databricks: "`system.information_schema.tables`", }, role_grants: { snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_ROLES`", - bigquery: "`INFORMATION_SCHEMA.OBJECT_PRIVILEGES`", + bigquery: "`region-.INFORMATION_SCHEMA.OBJECT_PRIVILEGES`", databricks: "`system.information_schema.table_privileges`", }, role_hierarchy: { snowflake: "`SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_ROLES` (granted_on = 'ROLE')" }, @@ -85,16 +88,20 @@ export interface WorkspaceFallback { * snapshot only — the same reachability-filtered projection the awareness section * uses, so this never names a tool the caller's agent cannot call. */ -export function workspaceFallbacks(sessionID: string, supportedTypes: readonly string[]): WorkspaceFallback[] { +export async function workspaceFallbacks(sessionID: string, supportedTypes: readonly string[]): Promise { const precedence = Precedence.forSession(sessionID) if (!precedence?.enabled) return [] - return Precedence.servedInventory(precedence).flatMap(({ type, served }) => { + const served = Precedence.servedInventory(precedence).flatMap(({ type, served }) => { if (!supportedTypes.includes(type)) return [] const execute = served.find((row) => row.capability === "sql_execute") return execute ? [{ workspaceName: precedence.workspaceName, workspaceId: precedence.workspaceId, type, modelKey: execute.modelKey }] : [] }) + if (served.length === 0) return [] + // Same re-validation `check()` does before a redirect: a note naming a workspace the + // project has since left would send the model to that workspace's engine. + return (await Precedence.snapshotState(precedence)) === "current" ? served : [] } /** The sentence appended to a FinOps failure, or nothing when the workspace serves @@ -117,18 +124,32 @@ export function workspaceFallbackNote(operation: FinopsOperation, fallbacks: Wor ) } -/** Why the fallback could not be decided, when routing is disabled for a reason that - * is uncertainty rather than a choice. Deliberate disablement (unbound, pilot off, - * `--integrations=local`, nothing materialised) says nothing: that is the plain local - * failure. Uncertainty must say so (the precedence module's first claim), so the - * failure is marked `undetermined` and says the workspace could not be consulted. */ -function undeterminedNote(sessionID: string): string | undefined { - const reason = Precedence.forSession(sessionID)?.disabledReason - if (reason !== "unattributed" && reason !== "binding-unreadable" && reason !== "derive-failed") return undefined - return ( - "Whether the linked workspace serves this connection type could not be determined this turn " + - "(no routing decision was available), so no workspace alternative is offered here." - ) +/** Why the fallback could not be decided, when that is uncertainty rather than a + * choice. Deliberate disablement (unbound, pilot off, `--integrations=local`, nothing + * materialised) says nothing: that is the plain local failure. Uncertainty must say so + * (the precedence module's first claim), so the failure is marked `undetermined` and + * says the workspace could not be consulted — mirroring `check()`'s own cases. */ +async function undeterminedNote(sessionID: string): Promise { + const precedence = Precedence.forSession(sessionID) + // No snapshot at all is the same unknown `check()` reports: a caller that never + // resolved tools, or an entry evicted between resolution and this call. + if (!precedence) { + return "No routing decision was available for this call, so whether the linked workspace serves this connection type is unknown." + } + const reason = precedence.disabledReason + if (reason === "unattributed" || reason === "binding-unreadable" || reason === "derive-failed") { + return ( + "Whether the linked workspace serves this connection type could not be determined this turn " + + "(no routing decision was available), so no workspace alternative is offered here." + ) + } + if (!precedence.enabled) return undefined + // Enabled, but the link changed or could not be read while this call ran. + const state = await Precedence.snapshotState(precedence) + if (state === "current") return undefined + return state === "unreadable" + ? "The workspace link could not be read while this call ran, so whether the workspace serves this connection type is unknown." + : "The project was re-linked while this call ran, so the previous routing decision no longer applies." } /** @@ -137,13 +158,13 @@ function undeterminedNote(sessionID: string): string | undefined { * metadata so telemetry can tell a failure the model could route around from a dead * end. */ -export function withWorkspaceFallback; output: string }>( +export async function withWorkspaceFallback; output: string }>( sessionID: string, operation: FinopsOperation, supportedTypes: readonly string[], result: T, -): T { - const fallbacks = workspaceFallbacks(sessionID, supportedTypes) +): Promise { + const fallbacks = await workspaceFallbacks(sessionID, supportedTypes) const note = workspaceFallbackNote(operation, fallbacks) if (note) { return { @@ -152,7 +173,7 @@ export function withWorkspaceFallback { describe("workspaceFallbacks", () => { test("names the engine execute tool for a served type the operation supports", async () => { await refresh(SESSION, SNOWFLAKE_TOOLS) - expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([ + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([ { workspaceName: "analytics", workspaceId: "42", type: "snowflake", modelKey: "datamate_snowflake_execute_database_query" }, ]) }) - test("is empty for a session with no routing decision", () => { - expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) + test("is empty for a session with no routing decision", async () => { + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) }) test("is empty when routing is disabled for the session", async () => { process.env.ALTIMATE_INTEGRATIONS = "local" await refresh(SESSION, SNOWFLAKE_TOOLS) - expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) }) test("ignores a served type the operation does not support", async () => { await refresh(SESSION, BIGQUERY_TOOLS) // The Snowflake-only operations (role hierarchy, user roles) get nothing from a // workspace that serves only BigQuery. - expect(workspaceFallbacks(SESSION, ["snowflake"])).toEqual([]) - expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES).map((f) => f.type)).toEqual(["bigquery"]) + expect(await workspaceFallbacks(SESSION, ["snowflake"])).toEqual([]) + expect((await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).map((f) => f.type)).toEqual(["bigquery"]) }) test("never names a tool the caller's agent cannot call", async () => { await refresh(SESSION, SNOWFLAKE_TOOLS, ANALYST_RULESET) - expect(workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) }) }) @@ -99,9 +99,15 @@ describe("workspaceFallbackNote", () => { // Grants do not live in the usage tables; a wrong table on a real failure path // sends the model down a dead end. (bot review) const bigquery = [{ workspaceName: "analytics", type: "bigquery", modelKey: "datamate_bigquery_execute_database_query" }] - expect(workspaceFallbackNote("role_grants", bigquery)).toContain("`INFORMATION_SCHEMA.OBJECT_PRIVILEGES`") + expect(workspaceFallbackNote("role_grants", bigquery)).toContain("`region-.INFORMATION_SCHEMA.OBJECT_PRIVILEGES`") expect(workspaceFallbackNote("role_grants", bigquery)).not.toContain("JOBS") - expect(workspaceFallbackNote("query_history", bigquery)).toContain("`INFORMATION_SCHEMA.JOBS`") + expect(workspaceFallbackNote("query_history", bigquery)).toContain("`region-.INFORMATION_SCHEMA.JOBS`") + // Every BigQuery recipe is region-qualified: the bare view name is not runnable. + for (const op of ["query_history", "analyze_credits", "expensive_queries", "warehouse_advice", "unused_resources", "role_grants"] as const) { + expect(workspaceFallbackNote(op, bigquery)).toMatch(/region-\.INFORMATION_SCHEMA/) + } + expect(workspaceFallbackNote("unused_resources", snowflake)).toContain("`QUERY_HISTORY`") + expect(workspaceFallbackNote("warehouse_advice", snowflake)).toContain("`SHOW WAREHOUSES`") const databricks = [{ workspaceName: "analytics", type: "databricks", modelKey: "datamate_databricks_execute_sql" }] expect(workspaceFallbackNote("role_grants", databricks)).toContain("`system.information_schema.table_privileges`") expect(workspaceFallbackNote("user_roles", snowflake)).toContain("`SNOWFLAKE.ACCOUNT_USAGE.GRANTS_TO_USERS`") @@ -123,7 +129,7 @@ describe("workspaceFallbackNote", () => { describe("withWorkspaceFallback", () => { test("keeps the local failure and appends the workspace route", async () => { await refresh(SESSION, SNOWFLAKE_TOOLS) - const result = withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, failed()) + const result = await withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, failed()) expect(result.title).toBe("Warehouse Advice: FAILED") expect(result.output.startsWith("Failed to analyze warehouses: none configured")).toBe(true) expect(result.output).toContain("`datamate_snowflake_execute_database_query`") @@ -134,19 +140,41 @@ describe("withWorkspaceFallback", () => { }) }) - test("returns the failure untouched when there is nothing to route to", () => { + test("returns the failure untouched when routing is deliberately off (unbound)", async () => { + precedenceInternals.binding = async () => null + const precedence = await refresh(SESSION, SNOWFLAKE_TOOLS) + expect(precedence.disabledReason).toBe("unbound") const input = failed() - const result = withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, input) + const result = await withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, input) expect(result).toBe(input) }) + test("no snapshot at all is unknown, as check() says, not silently local", async () => { + // codex on #1346: a caller that never resolved tools, or an entry evicted between + // resolution and this call. The precedence module reports that as undetermined. + const result = await withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, failed()) + expect(result.metadata.precedence).toBe("undetermined") + expect(result.output).toContain("No routing decision was available") + }) + + test("a workspace the project has since left is not recommended (snapshot re-validated)", async () => { + await refresh(SESSION, SNOWFLAKE_TOOLS) + // Re-linked mid-call: the snapshot names 42, the link now says 43. + precedenceInternals.binding = async () => ({ datamateId: 43, datamateName: "other" }) + const result = await withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, failed()) + expect(result.output).not.toContain("datamate_") + expect(result.metadata.workspace_fallback).toBeUndefined() + expect(result.metadata.precedence).toBe("undetermined") + expect(result.output).toContain("re-linked") + }) + test("routing that could not be determined is said, and marked, rather than passed off as local-only", async () => { // The engine could not be attributed to the bound workspace this turn: the // snapshot is disabled for uncertainty, not by choice. (bot review) precedenceInternals.attributedTo = async () => "999" const precedence = await refresh(SESSION, SNOWFLAKE_TOOLS) expect(precedence.disabledReason).toBe("unattributed") - const result = withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, failed()) + const result = await withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, failed()) expect(result.metadata.precedence).toBe("undetermined") expect(result.output).toContain("could not be determined this turn") expect(result.output).not.toContain("datamate_") @@ -157,7 +185,7 @@ describe("withWorkspaceFallback", () => { const precedence = await refresh(SESSION, SNOWFLAKE_TOOLS) expect(precedence.disabledReason).toBe("escape-hatch") const input = failed() - expect(withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, input)).toBe(input) + expect(await withWorkspaceFallback(SESSION, "warehouse_advice", DEFAULT_FINOPS_TYPES, input)).toBe(input) }) }) @@ -178,6 +206,34 @@ describe("through the tools", () => { expect(result.metadata.workspace_fallback).toEqual(["datamate_snowflake_execute_database_query"]) }) + test("every finops_* wrapper routes its failure through the fallback (codex on #1346)", async () => { + await refresh(SESSION, SNOWFLAKE_TOOLS) + const mods = await Promise.all([ + import("../../src/altimate/tools/finops-query-history"), + import("../../src/altimate/tools/finops-analyze-credits"), + import("../../src/altimate/tools/finops-expensive-queries"), + import("../../src/altimate/tools/finops-unused-resources"), + import("../../src/altimate/tools/finops-role-access"), + ]) + const tools = [ + [mods[0].FinopsQueryHistoryTool, { warehouse: "COMPUTE_WH" }, "`SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY`"], + [mods[1].FinopsAnalyzeCreditsTool, { days: 7 }, "WAREHOUSE_METERING_HISTORY"], + [mods[2].FinopsExpensiveQueriesTool, {}, "`SNOWFLAKE.ACCOUNT_USAGE.QUERY_HISTORY`"], + [mods[3].FinopsUnusedResourcesTool, {}, "TABLE_STORAGE_METRICS"], + [mods[4].FinopsRoleGrantsTool, {}, "GRANTS_TO_ROLES"], + [mods[4].FinopsRoleHierarchyTool, {}, "GRANTS_TO_ROLES"], + [mods[4].FinopsUserRolesTool, {}, "GRANTS_TO_USERS"], + ] as const + for (const [def, args, table] of tools) { + const tool = await initTool(def as never) + const result = await tool.execute(args, ctx()) + expect(result.title, tool.id).toMatch(/FAILED|ERROR/) + expect(result.output, tool.id).toContain("`datamate_snowflake_execute_database_query`") + expect(result.output, tool.id).toContain(table) + expect(result.metadata.workspace_fallback, tool.id).toEqual(["datamate_snowflake_execute_database_query"]) + } + }) + test("finops_role_hierarchy is not pointed at a workspace that serves only BigQuery", async () => { await refresh(SESSION, BIGQUERY_TOOLS) const { FinopsRoleHierarchyTool } = await import("../../src/altimate/tools/finops-role-access") @@ -189,6 +245,8 @@ describe("through the tools", () => { }) test("an unbound project gets the plain local failure", async () => { + precedenceInternals.binding = async () => null + await refresh(SESSION, SNOWFLAKE_TOOLS) const { FinopsAnalyzeCreditsTool } = await import("../../src/altimate/tools/finops-analyze-credits") const tool = await initTool(FinopsAnalyzeCreditsTool) const result = await tool.execute({ days: 7 }, ctx()) From 8c0eb3095245b8525caf8d7f47da9054e857e3d2 Mon Sep 17 00:00:00 2001 From: Haider Date: Mon, 21 Sep 2026 23:27:33 +0530 Subject: [PATCH 4/6] fix(finops): one snapshot read on the failure path, explained BigQuery placeholder, neutral wording Bot review of 14aa1a1 on #1346. `workspaceFallbacks` returns the re-validation outcome so the note and the undetermined marker come from one read; the BigQuery `` placeholder is explained (the snapshot carries no location, and the engine runs what it is given); a removed binding is "the workspace binding changed", not "re-linked". Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- .../src/altimate/tools/finops-workspace.ts | 60 ++++++++++++------- .../finops-workspace-fallback.test.ts | 25 +++++--- 2 files changed, 55 insertions(+), 30 deletions(-) diff --git a/packages/opencode/src/altimate/tools/finops-workspace.ts b/packages/opencode/src/altimate/tools/finops-workspace.ts index c2f48277f4..68730840a6 100644 --- a/packages/opencode/src/altimate/tools/finops-workspace.ts +++ b/packages/opencode/src/altimate/tools/finops-workspace.ts @@ -88,20 +88,32 @@ export interface WorkspaceFallback { * snapshot only — the same reachability-filtered projection the awareness section * uses, so this never names a tool the caller's agent cannot call. */ -export async function workspaceFallbacks(sessionID: string, supportedTypes: readonly string[]): Promise { +/** What the failure path learns about the workspace, in one read. */ +export type FallbackLookup = + | { state: "current"; fallbacks: WorkspaceFallback[] } + | { state: "relinked" | "unreadable" } + +/** + * The workspace-served execute tools for the types a FinOps operation supports, if the + * session is bound to a workspace that serves any. Reads the session's precedence + * snapshot only — the same reachability-filtered projection the awareness section + * uses, so this never names a tool the caller's agent cannot call — and re-validates + * the snapshot once, as `check()` does before a redirect: a note naming a workspace + * the project has since left would send the model to that workspace's engine. + */ +export async function workspaceFallbacks(sessionID: string, supportedTypes: readonly string[]): Promise { const precedence = Precedence.forSession(sessionID) - if (!precedence?.enabled) return [] - const served = Precedence.servedInventory(precedence).flatMap(({ type, served }) => { + if (!precedence?.enabled) return { state: "current", fallbacks: [] } + const fallbacks = Precedence.servedInventory(precedence).flatMap(({ type, served }) => { if (!supportedTypes.includes(type)) return [] const execute = served.find((row) => row.capability === "sql_execute") return execute ? [{ workspaceName: precedence.workspaceName, workspaceId: precedence.workspaceId, type, modelKey: execute.modelKey }] : [] }) - if (served.length === 0) return [] - // Same re-validation `check()` does before a redirect: a note naming a workspace the - // project has since left would send the model to that workspace's engine. - return (await Precedence.snapshotState(precedence)) === "current" ? served : [] + if (fallbacks.length === 0) return { state: "current", fallbacks } + const state = await Precedence.snapshotState(precedence) + return state === "current" ? { state, fallbacks } : { state } } /** The sentence appended to a FinOps failure, or nothing when the workspace serves @@ -117,10 +129,16 @@ export function workspaceFallbackNote(operation: FinopsOperation, fallbacks: Wor return source ? `for ${type}, query ${source} with \`${modelKey}\`` : `for ${type}, use \`${modelKey}\`` }) .join("; ") + // The snapshot does not carry a BigQuery integration's location, and the engine + // runs what it is given: the placeholder has to be explained, not left to be sent. + const region = fallbacks.some((f) => f.type === "bigquery" && SOURCE[operation].bigquery) + ? " Replace `` with the BigQuery connection's location (for example `region-us`, `region-eu`); " + + "if it is unknown, ask the engine for the connection's details first — the view is not reachable unqualified." + : "" return ( `This tool only uses warehouse connections configured on this machine, and workspace ${label} ` + `serves ${fallbacks.map((f) => f.type).join(", ")} through its integration engine instead. ` + - `Run the same analysis through the workspace: ${routes}.` + `Run the same analysis through the workspace: ${routes}.${region}` ) } @@ -129,7 +147,13 @@ export function workspaceFallbackNote(operation: FinopsOperation, fallbacks: Wor * materialised) says nothing: that is the plain local failure. Uncertainty must say so * (the precedence module's first claim), so the failure is marked `undetermined` and * says the workspace could not be consulted — mirroring `check()`'s own cases. */ -async function undeterminedNote(sessionID: string): Promise { +function undeterminedNote(sessionID: string, lookup: FallbackLookup): string | undefined { + if (lookup.state === "unreadable") { + return "The workspace link could not be read while this call ran, so whether the workspace serves this connection type is unknown." + } + if (lookup.state === "relinked") { + return "The workspace binding changed while this call ran, so the previous routing decision no longer applies." + } const precedence = Precedence.forSession(sessionID) // No snapshot at all is the same unknown `check()` reports: a caller that never // resolved tools, or an entry evicted between resolution and this call. @@ -143,13 +167,7 @@ async function undeterminedNote(sessionID: string): Promise "(no routing decision was available), so no workspace alternative is offered here." ) } - if (!precedence.enabled) return undefined - // Enabled, but the link changed or could not be read while this call ran. - const state = await Precedence.snapshotState(precedence) - if (state === "current") return undefined - return state === "unreadable" - ? "The workspace link could not be read while this call ran, so whether the workspace serves this connection type is unknown." - : "The project was re-linked while this call ran, so the previous routing decision no longer applies." + return undefined } /** @@ -164,16 +182,16 @@ export async function withWorkspaceFallback { - const fallbacks = await workspaceFallbacks(sessionID, supportedTypes) - const note = workspaceFallbackNote(operation, fallbacks) - if (note) { + const lookup = await workspaceFallbacks(sessionID, supportedTypes) + const note = lookup.state === "current" ? workspaceFallbackNote(operation, lookup.fallbacks) : undefined + if (note && lookup.state === "current") { return { ...result, - metadata: { ...result.metadata, workspace_fallback: fallbacks.map((f) => f.modelKey) }, + metadata: { ...result.metadata, workspace_fallback: lookup.fallbacks.map((f) => f.modelKey) }, output: `${result.output}\n\n${note}`, } } - const undetermined = await undeterminedNote(sessionID) + const undetermined = undeterminedNote(sessionID, lookup) if (!undetermined) return result return { ...result, diff --git a/packages/opencode/test/altimate/finops-workspace-fallback.test.ts b/packages/opencode/test/altimate/finops-workspace-fallback.test.ts index c9f44bed0d..a3744de397 100644 --- a/packages/opencode/test/altimate/finops-workspace-fallback.test.ts +++ b/packages/opencode/test/altimate/finops-workspace-fallback.test.ts @@ -53,32 +53,36 @@ afterEach(() => { describe("workspaceFallbacks", () => { test("names the engine execute tool for a served type the operation supports", async () => { await refresh(SESSION, SNOWFLAKE_TOOLS) - expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([ - { workspaceName: "analytics", workspaceId: "42", type: "snowflake", modelKey: "datamate_snowflake_execute_database_query" }, - ]) + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual({ + state: "current", + fallbacks: [ + { workspaceName: "analytics", workspaceId: "42", type: "snowflake", modelKey: "datamate_snowflake_execute_database_query" }, + ], + }) }) test("is empty for a session with no routing decision", async () => { - expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual({ state: "current", fallbacks: [] }) }) test("is empty when routing is disabled for the session", async () => { process.env.ALTIMATE_INTEGRATIONS = "local" await refresh(SESSION, SNOWFLAKE_TOOLS) - expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual({ state: "current", fallbacks: [] }) }) test("ignores a served type the operation does not support", async () => { await refresh(SESSION, BIGQUERY_TOOLS) // The Snowflake-only operations (role hierarchy, user roles) get nothing from a // workspace that serves only BigQuery. - expect(await workspaceFallbacks(SESSION, ["snowflake"])).toEqual([]) - expect((await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).map((f) => f.type)).toEqual(["bigquery"]) + expect(await workspaceFallbacks(SESSION, ["snowflake"])).toEqual({ state: "current", fallbacks: [] }) + const lookup = await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES) + expect(lookup.state === "current" && lookup.fallbacks.map((f) => f.type)).toEqual(["bigquery"]) }) test("never names a tool the caller's agent cannot call", async () => { await refresh(SESSION, SNOWFLAKE_TOOLS, ANALYST_RULESET) - expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual([]) + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual({ state: "current", fallbacks: [] }) }) }) @@ -105,7 +109,10 @@ describe("workspaceFallbackNote", () => { // Every BigQuery recipe is region-qualified: the bare view name is not runnable. for (const op of ["query_history", "analyze_credits", "expensive_queries", "warehouse_advice", "unused_resources", "role_grants"] as const) { expect(workspaceFallbackNote(op, bigquery)).toMatch(/region-\.INFORMATION_SCHEMA/) + // …and the placeholder is explained, since the snapshot carries no location. + expect(workspaceFallbackNote(op, bigquery)).toContain("Replace ``") } + expect(workspaceFallbackNote("query_history", snowflake)).not.toContain("") expect(workspaceFallbackNote("unused_resources", snowflake)).toContain("`QUERY_HISTORY`") expect(workspaceFallbackNote("warehouse_advice", snowflake)).toContain("`SHOW WAREHOUSES`") const databricks = [{ workspaceName: "analytics", type: "databricks", modelKey: "datamate_databricks_execute_sql" }] @@ -165,7 +172,7 @@ describe("withWorkspaceFallback", () => { expect(result.output).not.toContain("datamate_") expect(result.metadata.workspace_fallback).toBeUndefined() expect(result.metadata.precedence).toBe("undetermined") - expect(result.output).toContain("re-linked") + expect(result.output).toContain("binding changed") }) test("routing that could not be determined is said, and marked, rather than passed off as local-only", async () => { From 31a4ad537e0ec6782d22c56cd865b802ff5fdd74 Mon Sep 17 00:00:00 2001 From: Haider Date: Tue, 22 Sep 2026 03:14:46 +0530 Subject: [PATCH 5/6] docs(finops): drop the orphaned doc block above FallbackLookup (bot review) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- packages/opencode/src/altimate/tools/finops-workspace.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/opencode/src/altimate/tools/finops-workspace.ts b/packages/opencode/src/altimate/tools/finops-workspace.ts index 68730840a6..44ed69ab4e 100644 --- a/packages/opencode/src/altimate/tools/finops-workspace.ts +++ b/packages/opencode/src/altimate/tools/finops-workspace.ts @@ -82,12 +82,6 @@ export interface WorkspaceFallback { modelKey: string } -/** - * The workspace-served execute tools for the types a FinOps operation supports, if - * the session is bound to a workspace that serves any. Reads the session's precedence - * snapshot only — the same reachability-filtered projection the awareness section - * uses, so this never names a tool the caller's agent cannot call. - */ /** What the failure path learns about the workspace, in one read. */ export type FallbackLookup = | { state: "current"; fallbacks: WorkspaceFallback[] } From e46dcba0126c39bc5553551c3742d89b2821fe7c Mon Sep 17 00:00:00 2001 From: Haider Date: Tue, 22 Sep 2026 03:31:22 +0530 Subject: [PATCH 6/6] fix(finops): unprefixed BigQuery location examples; one snapshot read for the reason; stricter tests Bot review of 8c0eb30 on #1346. The note said to replace `` with `region-us`, which yields `region-region-us`; the examples are `us`, `eu`, `us-central1`. The disabled reason is carried on `FallbackLookup` from the same read as the snapshot, so the note cannot describe a different snapshot. The wrapper test requires the real handler's FAILED branch (not a wrapper catch), and precedence announcements are stubbed in setup. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012Q51zFUmPg1WwtS5CrGJE6 --- .../src/altimate/tools/finops-workspace.ts | 30 +++++++++++-------- .../finops-workspace-fallback.test.ts | 19 ++++++++---- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/packages/opencode/src/altimate/tools/finops-workspace.ts b/packages/opencode/src/altimate/tools/finops-workspace.ts index 44ed69ab4e..ac477294af 100644 --- a/packages/opencode/src/altimate/tools/finops-workspace.ts +++ b/packages/opencode/src/altimate/tools/finops-workspace.ts @@ -84,9 +84,14 @@ export interface WorkspaceFallback { /** What the failure path learns about the workspace, in one read. */ export type FallbackLookup = - | { state: "current"; fallbacks: WorkspaceFallback[] } + | { state: "current"; fallbacks: WorkspaceFallback[]; reason: Reason } | { state: "relinked" | "unreadable" } +/** Why nothing is served, read once with the snapshot so the note describes the + * same snapshot the lookup did: `none` — no snapshot for the session; a disabled + * reason — the snapshot's own; `served` — enabled. */ +type Reason = "none" | "served" | NonNullable + /** * The workspace-served execute tools for the types a FinOps operation supports, if the * session is bound to a workspace that serves any. Reads the session's precedence @@ -97,7 +102,8 @@ export type FallbackLookup = */ export async function workspaceFallbacks(sessionID: string, supportedTypes: readonly string[]): Promise { const precedence = Precedence.forSession(sessionID) - if (!precedence?.enabled) return { state: "current", fallbacks: [] } + if (!precedence) return { state: "current", fallbacks: [], reason: "none" } + if (!precedence.enabled) return { state: "current", fallbacks: [], reason: precedence.disabledReason ?? "served" } const fallbacks = Precedence.servedInventory(precedence).flatMap(({ type, served }) => { if (!supportedTypes.includes(type)) return [] const execute = served.find((row) => row.capability === "sql_execute") @@ -105,9 +111,9 @@ export async function workspaceFallbacks(sessionID: string, supportedTypes: read ? [{ workspaceName: precedence.workspaceName, workspaceId: precedence.workspaceId, type, modelKey: execute.modelKey }] : [] }) - if (fallbacks.length === 0) return { state: "current", fallbacks } + if (fallbacks.length === 0) return { state: "current", fallbacks, reason: "served" } const state = await Precedence.snapshotState(precedence) - return state === "current" ? { state, fallbacks } : { state } + return state === "current" ? { state, fallbacks, reason: "served" } : { state } } /** The sentence appended to a FinOps failure, or nothing when the workspace serves @@ -126,8 +132,9 @@ export function workspaceFallbackNote(operation: FinopsOperation, fallbacks: Wor // The snapshot does not carry a BigQuery integration's location, and the engine // runs what it is given: the placeholder has to be explained, not left to be sent. const region = fallbacks.some((f) => f.type === "bigquery" && SOURCE[operation].bigquery) - ? " Replace `` with the BigQuery connection's location (for example `region-us`, `region-eu`); " + - "if it is unknown, ask the engine for the connection's details first — the view is not reachable unqualified." + ? " Replace `` with the BigQuery connection's location (for example `us`, `eu`, `us-central1`, " + + "giving `region-us.INFORMATION_SCHEMA…`); if it is unknown, ask the engine for the connection's details " + + "first — the view is not reachable unqualified." : "" return ( `This tool only uses warehouse connections configured on this machine, and workspace ${label} ` + @@ -141,20 +148,19 @@ export function workspaceFallbackNote(operation: FinopsOperation, fallbacks: Wor * materialised) says nothing: that is the plain local failure. Uncertainty must say so * (the precedence module's first claim), so the failure is marked `undetermined` and * says the workspace could not be consulted — mirroring `check()`'s own cases. */ -function undeterminedNote(sessionID: string, lookup: FallbackLookup): string | undefined { +function undeterminedNote(lookup: FallbackLookup): string | undefined { if (lookup.state === "unreadable") { return "The workspace link could not be read while this call ran, so whether the workspace serves this connection type is unknown." } - if (lookup.state === "relinked") { + if (lookup.state !== "current") { return "The workspace binding changed while this call ran, so the previous routing decision no longer applies." } - const precedence = Precedence.forSession(sessionID) // No snapshot at all is the same unknown `check()` reports: a caller that never // resolved tools, or an entry evicted between resolution and this call. - if (!precedence) { + if (lookup.reason === "none") { return "No routing decision was available for this call, so whether the linked workspace serves this connection type is unknown." } - const reason = precedence.disabledReason + const reason = lookup.reason if (reason === "unattributed" || reason === "binding-unreadable" || reason === "derive-failed") { return ( "Whether the linked workspace serves this connection type could not be determined this turn " + @@ -185,7 +191,7 @@ export async function withWorkspaceFallback { process.env.ALTIMATE_WORKSPACE = "1" process.env.ALTIMATE_TELEMETRY_DISABLED = "true" bindTo(42, "analytics") + // `refresh` queues an announcement; keep it off the real event bridge. (bot review) + precedenceInternals.announce = async () => {} // The pilot's promise: no local warehouse connection at all. Registry.setConfigs({}) }) @@ -55,6 +57,7 @@ describe("workspaceFallbacks", () => { await refresh(SESSION, SNOWFLAKE_TOOLS) expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual({ state: "current", + reason: "served", fallbacks: [ { workspaceName: "analytics", workspaceId: "42", type: "snowflake", modelKey: "datamate_snowflake_execute_database_query" }, ], @@ -62,27 +65,27 @@ describe("workspaceFallbacks", () => { }) test("is empty for a session with no routing decision", async () => { - expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual({ state: "current", fallbacks: [] }) + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toMatchObject({ state: "current", fallbacks: [] }) }) test("is empty when routing is disabled for the session", async () => { process.env.ALTIMATE_INTEGRATIONS = "local" await refresh(SESSION, SNOWFLAKE_TOOLS) - expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual({ state: "current", fallbacks: [] }) + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toMatchObject({ state: "current", fallbacks: [] }) }) test("ignores a served type the operation does not support", async () => { await refresh(SESSION, BIGQUERY_TOOLS) // The Snowflake-only operations (role hierarchy, user roles) get nothing from a // workspace that serves only BigQuery. - expect(await workspaceFallbacks(SESSION, ["snowflake"])).toEqual({ state: "current", fallbacks: [] }) + expect(await workspaceFallbacks(SESSION, ["snowflake"])).toMatchObject({ state: "current", fallbacks: [] }) const lookup = await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES) expect(lookup.state === "current" && lookup.fallbacks.map((f) => f.type)).toEqual(["bigquery"]) }) test("never names a tool the caller's agent cannot call", async () => { await refresh(SESSION, SNOWFLAKE_TOOLS, ANALYST_RULESET) - expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toEqual({ state: "current", fallbacks: [] }) + expect(await workspaceFallbacks(SESSION, DEFAULT_FINOPS_TYPES)).toMatchObject({ state: "current", fallbacks: [] }) }) }) @@ -110,7 +113,8 @@ describe("workspaceFallbackNote", () => { for (const op of ["query_history", "analyze_credits", "expensive_queries", "warehouse_advice", "unused_resources", "role_grants"] as const) { expect(workspaceFallbackNote(op, bigquery)).toMatch(/region-\.INFORMATION_SCHEMA/) // …and the placeholder is explained, since the snapshot carries no location. - expect(workspaceFallbackNote(op, bigquery)).toContain("Replace ``") + expect(workspaceFallbackNote(op, bigquery)).toContain("for example `us`, `eu`") + expect(workspaceFallbackNote(op, bigquery)).not.toContain("`region-us`") } expect(workspaceFallbackNote("query_history", snowflake)).not.toContain("") expect(workspaceFallbackNote("unused_resources", snowflake)).toContain("`QUERY_HISTORY`") @@ -234,7 +238,10 @@ describe("through the tools", () => { for (const [def, args, table] of tools) { const tool = await initTool(def as never) const result = await tool.execute(args, ctx()) - expect(result.title, tool.id).toMatch(/FAILED|ERROR/) + // FAILED, not ERROR: the real handler's no-connection branch, not a wrapper catching + // "No native handler". (bot review) + expect(result.title, tool.id).toMatch(/FAILED$/) + expect(result.output, tool.id).toContain("requires a configured warehouse") expect(result.output, tool.id).toContain("`datamate_snowflake_execute_database_query`") expect(result.output, tool.id).toContain(table) expect(result.metadata.workspace_fallback, tool.id).toEqual(["datamate_snowflake_execute_database_query"])