From 6b16d7f49427cb33f3485fa60b3b2581b3b1f4b9 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 12 Aug 2026 16:19:21 +0530 Subject: [PATCH 01/14] chore: upgrade `@altimateai/altimate-core` to 0.7.0 and sync consumer contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Engine upgrade 0.5.1 → 0.7.0 (through 0.6.0). Both releases are correctness releases whose output differs where the old output was wrong: more lineage edges (derived tables, CTE chains, CTAS/INSERT…SELECT), more PII exposures, stricter migration verdicts, a new `unbalanced_quote` safety rule, and validate/transpile/equivalence fixes. One type-surface addition: `PiiColumnAccess.query_targets`. Contract sync (consumers still read legacy pre-native-core shapes): - `altimate-core-migration` tool: read `findings`/`safe`/`overall_risk` (engine `MigrationResult`) instead of nonexistent `risks` — previously ANY migration, including `DROP COLUMN`, rendered "Migration: SAFE"; never render SAFE when the engine call errored; count only non-"safe" findings as risks. - `altimate_core.migration` handler: coerce empty dialect `"" → undefined` (`|| undefined`, same as the equivalence handler) so `Schema.fromDdl` does not throw on the default empty-string dialect. - `check --checks safety`: read engine `threats[]` (`rule`/`message`/`detail`) so real threats (e.g. `unbalanced_quote`) render with rule and message instead of a generic warning. - `normalizeSeverity`: map engine severities `high → error`, `medium → warning` — previously both degraded to `info`, so `--fail-on`/`--severity` silently passed high-risk injections. - `check --checks pii`: map engine `PiiColumnAccess` (`table`/`column`/ `classification`/`query_targets`/`suggested_masking`); stop assigning the column NAME to the numeric column-position field; report the exposing alias; stringify `{ Custom: string }` classifications (also in the query-pii tool renderer, which printed `[object Object]`). - `altimate-core-track-lineage` tool: collect edges from `queries[].edges` (engine `LineageResult`) instead of flat `edges` — previously always "0 edges"; render `impact_map`; format `{table, column}` refs; render ERROR instead of "0 edges" when the engine call fails. - `altimate-core-query-pii` tool: surface new 0.7.0 `query_targets` field as "Exposed via: …". - `altimate-core-check` tool: safety renderer and telemetry read `rule`/`message` with legacy fallback. Test updates for 0.7.0 behavior: - Dialect-forwarding fixture: `payload:f` now parses in the default dialect, so it no longer discriminates; switched to Snowflake time-travel `AT(OFFSET => -60)` which still does. - Grade comparison made apples-to-apples (same table, projection-only diff). - New real-engine tests: `query_targets` contract + rendering, migration destructive/safe/empty-dialect/error + tool-title regression, track-lineage edge surfacing + error rendering, check CLI safety `ThreatFinding` (incl. `high → error` normalization) and PII `PiiColumnAccess` shapes (incl. `{ Custom }` classification). - Strengthened previously vacuous migration e2e assertions. Verification: typecheck clean; test/altimate + test/cli fully green (4750 pass / 0 fail); full-suite failures (MCP/TUI-sound/subprocess) reproduced identically on 0.5.1 or shown run-to-run flaky with no engine coupling — pre-existing. Marker check: no upstream-shared files modified. Codex reviewed twice (found the legacy-shape consumers; verified all fixes non-tautological). Closes #1089 Co-Authored-By: Claude Fable 5 --- bun.lock | 14 +- packages/opencode/package.json | 2 +- .../src/altimate/native/altimate-core.ts | 6 +- .../src/altimate/tools/altimate-core-check.ts | 4 +- .../altimate/tools/altimate-core-migration.ts | 33 ++-- .../altimate/tools/altimate-core-query-pii.ts | 5 +- .../tools/altimate-core-track-lineage.ts | 35 ++++- .../opencode/src/cli/cmd/check-helpers.ts | 6 +- packages/opencode/src/cli/cmd/check.ts | 39 +++-- .../test/altimate/altimate-core-e2e.test.ts | 141 +++++++++++++++++- .../altimate/altimate-core-native.test.ts | 13 +- packages/opencode/test/cli/check-e2e.test.ts | 76 ++++++++++ 12 files changed, 319 insertions(+), 55 deletions(-) diff --git a/bun.lock b/bun.lock index b1091b850f..75cd20c40e 100644 --- a/bun.lock +++ b/bun.lock @@ -271,7 +271,7 @@ "@ai-sdk/togetherai": "2.0.41", "@ai-sdk/vercel": "2.0.39", "@ai-sdk/xai": "3.0.82", - "@altimateai/altimate-core": "0.5.1", + "@altimateai/altimate-core": "0.7.0", "@altimateai/drivers": "workspace:*", "@aws-sdk/credential-providers": "3.1057.0", "@clack/prompts": "1.0.0-alpha.1", @@ -640,17 +640,17 @@ "@altimateai/altimate-code": ["@altimateai/altimate-code@workspace:packages/opencode"], - "@altimateai/altimate-core": ["@altimateai/altimate-core@0.5.1", "", { "optionalDependencies": { "@altimateai/altimate-core-darwin-arm64": "0.5.1", "@altimateai/altimate-core-darwin-x64": "0.5.1", "@altimateai/altimate-core-linux-arm64-gnu": "0.5.1", "@altimateai/altimate-core-linux-x64-gnu": "0.5.1", "@altimateai/altimate-core-win32-x64-msvc": "0.5.1" } }, "sha512-WnGBfERvrEqcdKRmQsvm6byAchLbvaE7EWu75/nRSJLqd8S1QrStnUZg2slmtKkY9pPT/2eEMBHBBBrYbdLBcw=="], + "@altimateai/altimate-core": ["@altimateai/altimate-core@0.7.0", "", { "optionalDependencies": { "@altimateai/altimate-core-darwin-arm64": "0.7.0", "@altimateai/altimate-core-darwin-x64": "0.7.0", "@altimateai/altimate-core-linux-arm64-gnu": "0.7.0", "@altimateai/altimate-core-linux-x64-gnu": "0.7.0", "@altimateai/altimate-core-win32-x64-msvc": "0.7.0" } }, "sha512-9LqXOUHhrsuZlvdfZEoFLyMGg5Uz83cHj6TrqMb0h8lK97UdeVBvd2S/53TB7sPVlYnpjFlI++fNh8swqADsLA=="], - "@altimateai/altimate-core-darwin-arm64": ["@altimateai/altimate-core-darwin-arm64@0.5.1", "", { "os": "darwin", "cpu": "arm64" }, "sha512-HMYbas6x+zg+c9ruvRBF/Y8miLXqbyEWdxI0289eRuTWrMMti5X3HUDbNKkTPbTuYSPk5hPRlFwMvLXNr0g39Q=="], + "@altimateai/altimate-core-darwin-arm64": ["@altimateai/altimate-core-darwin-arm64@0.7.0", "", { "os": "darwin", "cpu": "arm64" }, "sha512-NBPwsfBT5Ul41u9zNiovYaOpZ+zhzHKq5+GqFWZcy4iW6k0aWUb30niTrz2Yse73/nWIt6G//pS4m4IX/nMy4g=="], - "@altimateai/altimate-core-darwin-x64": ["@altimateai/altimate-core-darwin-x64@0.5.1", "", { "os": "darwin", "cpu": "x64" }, "sha512-xVU78S9jbtYYjzeR5TJRULzSl9QZCcIZFNeAfd75t9gQoVB2QNJdiJBP5KYB6wx+FNEMYuofbDPvHyIbzNr2SA=="], + "@altimateai/altimate-core-darwin-x64": ["@altimateai/altimate-core-darwin-x64@0.7.0", "", { "os": "darwin", "cpu": "x64" }, "sha512-pLEJT2ePh247XOIicaJw8x5i2hRsCF2HqgVUy9nKJf5ebynHLGZ2JFC0fMq4oRe9ZCmywN8EzMD1UwN1Nw9AUQ=="], - "@altimateai/altimate-core-linux-arm64-gnu": ["@altimateai/altimate-core-linux-arm64-gnu@0.5.1", "", { "os": "linux", "cpu": "arm64" }, "sha512-55JRVxzE7P+kFzfhak+DJEjkSrHMnYCLuq4uX8jnAgQVj9slZyjWOjayvg0jJKycgloLxogviImR3uX4asZqtA=="], + "@altimateai/altimate-core-linux-arm64-gnu": ["@altimateai/altimate-core-linux-arm64-gnu@0.7.0", "", { "os": "linux", "cpu": "arm64" }, "sha512-co6Sm/iFIyYnsWxkhw7tUJRB9hfHH0Bjt0oa4bAysG8hVVoMhCN8ADS4fvPAg52Z8umk126mAN8k2XPfXh/ieA=="], - "@altimateai/altimate-core-linux-x64-gnu": ["@altimateai/altimate-core-linux-x64-gnu@0.5.1", "", { "os": "linux", "cpu": "x64" }, "sha512-zncu3vQN+m4/wn8BG9UXrqEMm4uICSWSyvwpQnBPe8d3YJf//IgyYIpuiCMqGgXaeH61lRDjGlHcSiYAaxN64w=="], + "@altimateai/altimate-core-linux-x64-gnu": ["@altimateai/altimate-core-linux-x64-gnu@0.7.0", "", { "os": "linux", "cpu": "x64" }, "sha512-hc9tOl8v+DQFsy8TIkFPxZGTyohPcwpfUqquLgiuKn2U6MDyJK3962Nzf/TzgfcbHj+GQE34oyPJ709iaBZnGg=="], - "@altimateai/altimate-core-win32-x64-msvc": ["@altimateai/altimate-core-win32-x64-msvc@0.5.1", "", { "os": "win32", "cpu": "x64" }, "sha512-3vcbPM+ci08dY9BJwp9XnTU4lBDiQZiQ9Qn7bV1ACfohlvNfwCUdsCCtVHqRtcxp8yBD+wvjPgRM2/HowUCTVg=="], + "@altimateai/altimate-core-win32-x64-msvc": ["@altimateai/altimate-core-win32-x64-msvc@0.7.0", "", { "os": "win32", "cpu": "x64" }, "sha512-GrykrvnilY1yZCgusNxVTvYr5ENkD1vpgjGib2qX/yvJVs0Nb59/JsSLzvA9XqAt2FVf7OcZ5ttH11OkxuFivg=="], "@altimateai/dbt-integration": ["@altimateai/dbt-integration@0.2.14", "", { "dependencies": { "@altimateai/altimate-core": "0.1.6", "node-abort-controller": "^3.1.1", "node-fetch": "^3.3.2", "python-bridge": "^1.1.0", "semver": "^7.6.3", "yaml": "^2.5.0" }, "peerDependencies": { "patch-package": "^8.0.0" } }, "sha512-44hFBx3gXss2RUlRZE9cIpAocffB3miAzpjvmma1EED0jiOhyrgmzGsw3RXEZqs73OuLYpDehixvRUt1vTfcfQ=="], diff --git a/packages/opencode/package.json b/packages/opencode/package.json index 173bd0e1be..1714dafeaf 100644 --- a/packages/opencode/package.json +++ b/packages/opencode/package.json @@ -78,7 +78,7 @@ "@ai-sdk/togetherai": "2.0.41", "@ai-sdk/vercel": "2.0.39", "@ai-sdk/xai": "3.0.82", - "@altimateai/altimate-core": "0.5.1", + "@altimateai/altimate-core": "0.7.0", "@altimateai/drivers": "workspace:*", "@aws-sdk/credential-providers": "3.1057.0", "@clack/prompts": "1.0.0-alpha.1", diff --git a/packages/opencode/src/altimate/native/altimate-core.ts b/packages/opencode/src/altimate/native/altimate-core.ts index 137e28d848..1ce271b7cd 100644 --- a/packages/opencode/src/altimate/native/altimate-core.ts +++ b/packages/opencode/src/altimate/native/altimate-core.ts @@ -337,8 +337,10 @@ export function registerAll(): void { // 12. altimate_core.migration register("altimate_core.migration", async (params) => { try { - // Build schema from old_ddl, analyze new_ddl against it - const schema = core.Schema.fromDdl(params.old_ddl, params.dialect ?? undefined) + // Build schema from old_ddl, analyze new_ddl against it. + // `|| undefined` (not `??`): "" must mean auto-detect — the engine + // throws on an unknown dialect "" (same coercion as equivalence above). + const schema = core.Schema.fromDdl(params.old_ddl, params.dialect || undefined) const raw = core.analyzeMigration(params.new_ddl, schema) const data = toData(raw) return ok(true, data) diff --git a/packages/opencode/src/altimate/tools/altimate-core-check.ts b/packages/opencode/src/altimate/tools/altimate-core-check.ts index 5d242a9dd2..6762a409a7 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-check.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-check.ts @@ -30,7 +30,7 @@ export const AltimateCoreCheckTool = Tool.define("altimate_core_check", { findings.push({ category: f.rule ?? "lint" }) } for (const t of data.safety?.threats ?? []) { - findings.push({ category: t.type ?? "safety_threat" }) + findings.push({ category: t.rule ?? t.type ?? "safety_threat" }) } for (const p of data.pii?.findings ?? []) { findings.push({ category: "pii_detected" }) @@ -93,7 +93,7 @@ export function formatCheck(data: Record): string { lines.push("Safe — no threats.") } else { for (const t of data.safety?.threats ?? []) { - lines.push(` [${t.severity ?? "warning"}] ${t.type ?? "safety"}: ${t.description ?? ""}`) + lines.push(` [${t.severity ?? "warning"}] ${t.rule ?? t.type ?? "safety"}: ${t.message ?? t.description ?? ""}`) } } diff --git a/packages/opencode/src/altimate/tools/altimate-core-migration.ts b/packages/opencode/src/altimate/tools/altimate-core-migration.ts index 694cd300c2..18c9a584a2 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-migration.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-migration.ts @@ -18,12 +18,21 @@ export const AltimateCoreMigrationTool = Tool.define("altimate_core_migration", dialect: args.dialect ?? "", }) const data = (result.data ?? {}) as Record - const riskCount = data.risks?.length ?? 0 + // Engine shape (MigrationResult): findings[], safe, overall_risk. + // Informational findings carry risk "safe" — only count real risks. + const findings = (data.findings ?? data.risks ?? []) as Array> + const riskCount = findings.filter((f) => (f.risk ?? f.severity ?? "risk") !== "safe").length const error = result.error ?? data.error + // Never render SAFE when the engine call itself failed. + const title = error + ? "Migration: ERROR" + : data.safe === false || riskCount > 0 + ? `Migration: ${(data.overall_risk ?? "risk").toString().toUpperCase()} — ${riskCount} risk(s)` + : "Migration: SAFE" return { - title: `Migration: ${riskCount === 0 ? "SAFE" : `${riskCount} risk(s)`}`, + title, metadata: { success: result.success, risk_count: riskCount, ...(error && { error }) }, - output: formatMigration(data), + output: formatMigration(data, error), } } catch (e) { const msg = e instanceof Error ? e.message : String(e) @@ -36,13 +45,17 @@ export const AltimateCoreMigrationTool = Tool.define("altimate_core_migration", }, }) -function formatMigration(data: Record): string { - if (data.error) return `Error: ${data.error}` - if (!data.risks?.length) return "Migration appears safe. No risks detected." - const lines = ["Migration risks:\n"] - for (const r of data.risks) { - lines.push(` [${r.severity ?? "warning"}] ${r.type}: ${r.message}`) - if (r.recommendation) lines.push(` Recommendation: ${r.recommendation}`) +function formatMigration(data: Record, error?: string): string { + if (error ?? data.error) return `Error: ${error ?? data.error}` + const findings = data.findings ?? data.risks ?? [] + if (!findings.length && data.safe !== false) return "Migration appears safe. No risks detected." + const lines: string[] = [] + if (data.overall_risk) lines.push(`Overall risk: ${data.overall_risk}`) + lines.push("Migration findings:\n") + for (const r of findings) { + lines.push(` [${r.risk ?? r.severity ?? "warning"}] ${r.operation ?? r.type ?? "operation"}: ${r.message ?? ""}`) + if (r.mitigation ?? r.recommendation) lines.push(` Mitigation: ${r.mitigation ?? r.recommendation}`) + if (r.rollback_sql) lines.push(` Rollback: ${r.rollback_sql}`) } return lines.join("\n") } diff --git a/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts b/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts index 463b9ba44c..1e60c2be03 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts @@ -45,10 +45,13 @@ function formatQueryPii(data: Record): string { if (data.risk_level) lines.push(`Risk level: ${data.risk_level}`) lines.push("PII exposure detected:\n") for (const e of piiCols) { - const classification = e.classification ?? e.category ?? "PII" + // Classification is a string OR { Custom: string }. + const raw = e.classification ?? e.category ?? "PII" + const classification = typeof raw === "string" ? raw : (raw?.Custom ?? "PII") const table = e.table ?? "unknown" const column = e.column ?? "unknown" lines.push(` ${table}.${column}: ${classification}`) + if (e.query_targets?.length) lines.push(` Exposed via: ${e.query_targets.join(", ")}`) if (e.suggested_masking) lines.push(` Masking: ${e.suggested_masking}`) } if (data.suggested_alternatives?.length) { diff --git a/packages/opencode/src/altimate/tools/altimate-core-track-lineage.ts b/packages/opencode/src/altimate/tools/altimate-core-track-lineage.ts index c3090c27d2..81e022cb0d 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-track-lineage.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-track-lineage.ts @@ -18,12 +18,13 @@ export const AltimateCoreTrackLineageTool = Tool.define("altimate_core_track_lin schema_context: args.schema_context, }) const data = (result.data ?? {}) as Record - const edgeCount = data.edges?.length ?? 0 + const edgeCount = collectEdges(data).length const error = result.error ?? data.error return { - title: `Track Lineage: ${edgeCount} edge(s) across ${args.queries.length} queries`, + // Never render "0 edges" when the engine call itself failed. + title: error ? "Track Lineage: ERROR" : `Track Lineage: ${edgeCount} edge(s) across ${args.queries.length} queries`, metadata: { success: result.success, edge_count: edgeCount, ...(error && { error }) }, - output: formatTrackLineage(data), + output: error ? `Error: ${error}` : formatTrackLineage(data), } } catch (e) { const msg = e instanceof Error ? e.message : String(e) @@ -36,12 +37,34 @@ export const AltimateCoreTrackLineageTool = Tool.define("altimate_core_track_lin }, }) +/** Engine shape (LineageResult): edges live under queries[].edges, not at the top level. */ +function collectEdges(data: Record): any[] { + if (Array.isArray(data.queries)) return data.queries.flatMap((q: any) => q.edges ?? []) + return data.edges ?? [] +} + +/** Column refs are { table, column } objects; legacy shape was a plain string. */ +function refToString(ref: any): string { + if (ref == null) return "?" + if (typeof ref === "string") return ref + return [ref.table, ref.column].filter(Boolean).join(".") || "?" +} + function formatTrackLineage(data: Record): string { if (data.error) return `Error: ${data.error}` - if (!data.edges?.length) return "No lineage edges found across queries." + const edges = collectEdges(data) + if (!edges.length) return "No lineage edges found across queries." const lines = ["Lineage graph:\n"] - for (const edge of data.edges) { - lines.push(` ${edge.source ?? "?"} -> ${edge.target ?? "?"}${edge.transform ? ` (${edge.transform})` : ""}`) + for (const edge of edges) { + const transform = edge.transform_type ?? edge.transform + lines.push(` ${refToString(edge.source)} -> ${refToString(edge.target)}${transform ? ` (${transform})` : ""}`) + } + if (Array.isArray(data.impact_map) && data.impact_map.length) { + lines.push("\nImpact map:") + for (const entry of data.impact_map) { + const affected = (entry.affected ?? []).map(refToString).join(", ") + lines.push(` ${refToString(entry.source)} affects: ${affected || "(none)"}`) + } } return lines.join("\n") } diff --git a/packages/opencode/src/cli/cmd/check-helpers.ts b/packages/opencode/src/cli/cmd/check-helpers.ts index a0f675f9c5..99ecf5004c 100644 --- a/packages/opencode/src/cli/cmd/check-helpers.ts +++ b/packages/opencode/src/cli/cmd/check-helpers.ts @@ -52,8 +52,10 @@ export const VALID_CHECKS = new Set(["lint", "validate", "safety", "policy", "pi export function normalizeSeverity(s?: string | unknown): Severity { if (!s || typeof s !== "string") return "warning" const lower = s.toLowerCase() - if (lower === "error" || lower === "fatal" || lower === "critical") return "error" - if (lower === "warning" || lower === "warn") return "warning" + // Engine safety severities are low|medium|high|critical — high-risk threats + // must not degrade to info or --fail-on/--severity filters silently pass. + if (lower === "error" || lower === "fatal" || lower === "critical" || lower === "high") return "error" + if (lower === "warning" || lower === "warn" || lower === "medium") return "warning" return "info" } diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index ce1027aa94..cf23573e8c 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -97,7 +97,10 @@ async function runSafety(sql: string, file: string): Promise { try { const result = await Dispatcher.call("altimate_core.safety", { sql }) if (result.success && result.data.safe !== false) return [] - const issues = (result.data.issues ?? result.data.findings ?? []) as Array> + const issues = (result.data.threats ?? + result.data.issues ?? + result.data.findings ?? + []) as Array> if (issues.length > 0) { return issues.map((f) => ({ file, @@ -107,7 +110,7 @@ async function runSafety(sql: string, file: string): Promise { rule: (f.rule ?? f.category ?? "safety") as string, severity: normalizeSeverity(f.severity as string), message: (f.message ?? f.description ?? "") as string, - suggestion: f.suggestion as string | undefined, + suggestion: (f.suggestion ?? f.detail) as string | undefined, })) } if (!result.success || result.data.safe === false) { @@ -174,17 +177,29 @@ async function runPii(sql: string, file: string, schemaPath?: string): Promise> - return piiFindings.map((f) => ({ - file, - line: f.line as number | undefined, - column: f.column as number | undefined, - code: f.code as string | undefined, - rule: (f.category ?? f.pii_type ?? "pii") as string, - severity: "warning" as const, - message: (f.message ?? f.description ?? `PII detected: ${f.column_name ?? f.name ?? "unknown"}`) as string, - suggestion: f.suggestion as string | undefined, - })) + return piiFindings.map((f) => { + const qualified = [f.table, f.column].filter(Boolean).join(".") + const targets = Array.isArray(f.query_targets) && f.query_targets.length ? ` (exposed via: ${(f.query_targets as string[]).join(", ")})` : "" + // Classification is a string OR { Custom: string }. + const classification = f.classification as string | { Custom: string } | undefined + const rule = + typeof classification === "string" + ? classification + : (classification?.Custom ?? (f.category as string) ?? (f.pii_type as string) ?? "pii") + return { + file, + line: f.line as number | undefined, + code: f.code as string | undefined, + rule, + severity: "warning" as const, + message: (f.message ?? f.description ?? `PII detected: ${qualified || "unknown"}${targets}`) as string, + suggestion: (f.suggestion ?? f.suggested_masking) as string | undefined, + } + }) } catch (e) { console.error(`[pii] error processing ${file}: ${e instanceof Error ? e.message : String(e)}`) return [dispatcherErrorFinding("pii", file, e)] diff --git a/packages/opencode/test/altimate/altimate-core-e2e.test.ts b/packages/opencode/test/altimate/altimate-core-e2e.test.ts index 4db6f27fb3..92a5522b46 100644 --- a/packages/opencode/test/altimate/altimate-core-e2e.test.ts +++ b/packages/opencode/test/altimate/altimate-core-e2e.test.ts @@ -376,7 +376,11 @@ describeIf("altimate-core E2E", () => { }) test("explicit columns grade >= SELECT *", async () => { - const r1 = await D.call("altimate_core.grade", { sql: SQL.simple, schema_context: ECOMMERCE_FLAT }) + // Same table + same rows, differing ONLY in star vs explicit projection — + // otherwise complexity subscores (filters, table count) dominate the + // style penalty for SELECT * and the comparison is meaningless. + const explicitSql = "SELECT order_id, customer_id, order_date, status, amount FROM orders" + const r1 = await D.call("altimate_core.grade", { sql: explicitSql, schema_context: ECOMMERCE_FLAT }) const r2 = await D.call("altimate_core.grade", { sql: SQL.selectStar, schema_context: ECOMMERCE_FLAT }) expect((r1.data as any).scores.overall).toBeGreaterThanOrEqual((r2.data as any).scores.overall) }) @@ -719,13 +723,62 @@ describeIf("altimate-core E2E", () => { describe("altimate_core.migration", () => { test("adding nullable column is safe", async () => { - const r = await D.call("altimate_core.migration", { old_ddl: "CREATE TABLE t (id INT);", new_ddl: "CREATE TABLE t (id INT, name VARCHAR);" }) - expect(r).toBeDefined() + const r = await D.call("altimate_core.migration", { old_ddl: "CREATE TABLE t (id INT);", new_ddl: "ALTER TABLE t ADD COLUMN name VARCHAR" }) + const d = r.data as any + expect(r.success).toBe(true) + expect(d.safe).toBe(true) + expect(d.overall_risk).toBe("safe") }) - test("dropping column detected", async () => { - const r = await D.call("altimate_core.migration", { old_ddl: "CREATE TABLE t (id INT, name VARCHAR);", new_ddl: "CREATE TABLE t (id INT);" }) - expect(r).toBeDefined() + test("dropping column detected as destructive", async () => { + const r = await D.call("altimate_core.migration", { old_ddl: "CREATE TABLE t (id INT, name VARCHAR);", new_ddl: "ALTER TABLE t DROP COLUMN name" }) + const d = r.data as any + expect(r.success).toBe(true) + expect(d.safe).toBe(false) + expect(d.overall_risk).toBe("destructive") + expect((d.findings ?? []).some((f: any) => f.risk === "destructive")).toBe(true) + }) + + test("DELETE without WHERE is destructive (core@0.6.0 behavior)", async () => { + const r = await D.call("altimate_core.migration", { old_ddl: "CREATE TABLE t (id INT);", new_ddl: "DELETE FROM t" }) + expect((r.data as any).safe).toBe(false) + }) + + test("empty dialect is coerced to auto-detect (does NOT error)", async () => { + const r = await D.call("altimate_core.migration", { old_ddl: "CREATE TABLE t (id INT);", new_ddl: "ALTER TABLE t ADD COLUMN name VARCHAR", dialect: "" }) + expect(r.success).toBe(true) + }) + + test("migration tool never renders SAFE on destructive change or engine error", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreMigrationTool } = await import("../../src/altimate/tools/altimate-core-migration") + const tool = await initTool(AltimateCoreMigrationTool) + const ctx = { + sessionID: "test", + messageID: "test", + agent: "test", + abort: new AbortController().signal, + messages: [], + metadata: () => {}, + } as any + const destructive = await tool.execute( + { old_ddl: "CREATE TABLE t (id INT, name VARCHAR);", new_ddl: "ALTER TABLE t DROP COLUMN name" }, + ctx, + ) + expect(destructive.title).not.toContain("SAFE") + expect(destructive.title).toContain("DESTRUCTIVE") + expect(destructive.output).toContain("DROP COLUMN t.name") + const safe = await tool.execute( + { old_ddl: "CREATE TABLE t (id INT);", new_ddl: "ALTER TABLE t ADD COLUMN name VARCHAR" }, + ctx, + ) + expect(safe.title).toBe("Migration: SAFE") + const errored = await tool.execute( + { old_ddl: "CREATE TABLE t (id INT);", new_ddl: "ALTER TABLE t ADD COLUMN name VARCHAR", dialect: "not-a-real-dialect" }, + ctx, + ) + expect(errored.title).toBe("Migration: ERROR") + expect(errored.output).toContain("Error") }) }) @@ -785,10 +838,41 @@ describeIf("altimate-core E2E", () => { expect((d.pii_columns ?? []).length).toBeGreaterThan(0) }) + test("query_pii reports query_targets for aliased PII output (core@0.7.0 contract)", async () => { + const r = await D.call("altimate_core.query_pii", { + sql: "SELECT email AS contact FROM customers", + schema_context: ECOMMERCE_FLAT, + }) + const d = r.data as any + expect(d.accesses_pii).toBe(true) + const emailCol = (d.pii_columns ?? []).find((c: any) => c.column === "email") + expect(emailCol).toBeDefined() + expect(emailCol.query_targets).toContain("contact") + }) + test("query_pii clean for non-PII columns", async () => { const r = await D.call("altimate_core.query_pii", { sql: "SELECT customer_id FROM customers", schema_context: ECOMMERCE_FLAT }) expect((r.data as any).accesses_pii).toBe(false) }) + + test("query_pii tool renders query_targets as 'Exposed via'", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreQueryPiiTool } = await import("../../src/altimate/tools/altimate-core-query-pii") + const tool = await initTool(AltimateCoreQueryPiiTool) + const result = await tool.execute( + { sql: "SELECT email AS contact FROM customers", schema_context: ECOMMERCE_FLAT }, + { + sessionID: "test", + messageID: "test", + agent: "test", + abort: new AbortController().signal, + messages: [], + metadata: () => {}, + } as any, + ) + expect(result.output).toContain("customers.email") + expect(result.output).toContain("Exposed via: contact") + }) }) // ========================================================================= @@ -824,6 +908,51 @@ describeIf("altimate-core E2E", () => { }) expect(r.success).toBe(true) }) + + test("track-lineage tool surfaces edges from queries[].edges (engine shape)", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreTrackLineageTool } = await import("../../src/altimate/tools/altimate-core-track-lineage") + const tool = await initTool(AltimateCoreTrackLineageTool) + const result = await tool.execute( + { + queries: [ + "CREATE TABLE staging AS SELECT customer_id, first_name FROM customers", + "CREATE TABLE summary AS SELECT customer_id, COUNT(*) AS cnt FROM staging GROUP BY customer_id", + ], + schema_context: ECOMMERCE_FLAT, + }, + { + sessionID: "test", + messageID: "test", + agent: "test", + abort: new AbortController().signal, + messages: [], + metadata: () => {}, + } as any, + ) + expect(result.metadata.edge_count).toBeGreaterThan(0) + expect(result.output).toContain("customers.customer_id") + expect(result.output).not.toContain("No lineage edges found") + }) + + test("track-lineage tool renders ERROR (not '0 edges') when the engine call fails", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreTrackLineageTool } = await import("../../src/altimate/tools/altimate-core-track-lineage") + const tool = await initTool(AltimateCoreTrackLineageTool) + const result = await tool.execute( + { queries: ["SELECT FROM"], schema_context: ECOMMERCE_FLAT }, + { + sessionID: "test", + messageID: "test", + agent: "test", + abort: new AbortController().signal, + messages: [], + metadata: () => {}, + } as any, + ) + expect(result.title).toBe("Track Lineage: ERROR") + expect(result.output).not.toContain("No lineage edges found") + }) }) // ========================================================================= diff --git a/packages/opencode/test/altimate/altimate-core-native.test.ts b/packages/opencode/test/altimate/altimate-core-native.test.ts index 78bbb507ac..eb27a34787 100644 --- a/packages/opencode/test/altimate/altimate-core-native.test.ts +++ b/packages/opencode/test/altimate/altimate-core-native.test.ts @@ -839,12 +839,13 @@ describe("core 0.5.1 dialect forwarding + decidable", () => { { name: "x", type: "INT" }, { name: "payload", type: "VARIANT" }, ] } }, } - // Snowflake semi-structured access `payload:f` is a hard PARSE error in the - // default dialect (the ':' is rejected) but parses under the snowflake dialect. - // The validation-error TEXT therefore differs by dialect — a syntax error only - // when the hint is dropped. Pre-fix (dialect not forwarded) BOTH would be syntax - // errors; this asserts they diverge, proving the arg reaches the parser. - const colonSql = "select payload:f::int as v from t" + // Snowflake time-travel `AT(OFFSET => -60)` is a hard PARSE error in the + // default dialect but parses under the snowflake dialect. (Since core@0.7.0 + // the default dialect accepts the former fixture `payload:f`, so time travel + // is the discriminating construct now.) Pre-fix (dialect not forwarded) BOTH + // would be syntax errors; this asserts they diverge, proving the arg reaches + // the parser. + const colonSql = "select x from t at(offset => -60)" const equivCall = (dialect?: string) => Dispatcher.call("altimate_core.equivalence", { sql1: colonSql, sql2: colonSql, schema_context: variantSchema, dialect }) const hasSyntaxError = (data: any) => diff --git a/packages/opencode/test/cli/check-e2e.test.ts b/packages/opencode/test/cli/check-e2e.test.ts index 25b76bfe70..517db10b3c 100644 --- a/packages/opencode/test/cli/check-e2e.test.ts +++ b/packages/opencode/test/cli/check-e2e.test.ts @@ -593,6 +593,82 @@ describe("check command E2E", () => { expect(j.results.safety.findings[0].rule).toBe("sql-injection") }) + test("safety check surfaces engine ThreatFinding shape (threats/rule/message/detail)", async () => { + // Real core@0.7.0 scanSql shape: threats[], each { rule, severity, message, detail }. + // Regression guard: the consumer previously only read issues/findings, so + // real threats (e.g. the 0.6.0 unbalanced_quote rule) rendered as a generic warning. + const file = await writeSql(tmpDir.dir, "breakout.sql", "SELECT * FROM users WHERE name = 'x' OR 1=1 --';") + setDispatcherResponse("altimate_core.safety", () => ({ + success: true, + data: { + safe: false, + risk_score: 0.9, + statement_count: 1, + statement_types: ["SELECT"], + threats: [ + { + rule: "unbalanced_quote", + severity: "high", + message: "Unbalanced quote suggests injection breakout", + detail: "Quote count is odd within a single statement", + matched_pattern: "' OR 1=1 --", + }, + ], + }, + })) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [file], checks: "safety" })) + const j = parseJson(r.stdout) + expect(j.results.safety.findings).toHaveLength(1) + expect(j.results.safety.findings[0].rule).toBe("unbalanced_quote") + expect(j.results.safety.findings[0].message).toBe("Unbalanced quote suggests injection breakout") + expect(j.results.safety.findings[0].suggestion).toBe("Quote count is odd within a single statement") + // Engine severity "high" must normalize to error, not degrade to info — + // otherwise --fail-on/--severity filters silently pass high-risk injections. + expect(j.results.safety.findings[0].severity).toBe("error") + }) + + test("pii check surfaces engine PiiColumnAccess shape (classification/query_targets/masking)", async () => { + // Real core@0.7.0 query_pii shape: pii_columns[], each + // { table, column, classification, query_targets, suggested_masking }. + const file = await writeSql(tmpDir.dir, "pii-real.sql", "SELECT email AS contact FROM customers;") + setDispatcherResponse("altimate_core.query_pii", () => ({ + success: true, + data: { + accesses_pii: true, + risk_level: "Medium", + pii_columns: [ + { + table: "customers", + column: "email", + classification: "Email", + query_targets: ["contact"], + suggested_masking: "'***MASKED***'", + }, + { + table: "customers", + column: "employee_ref", + // PiiClassification can be { Custom: string }, not just a string. + classification: { Custom: "EmployeeId" }, + query_targets: [], + suggested_masking: null, + }, + ], + }, + })) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [file], checks: "pii" })) + const j = parseJson(r.stdout) + expect(j.results.pii.findings).toHaveLength(2) + expect(j.results.pii.findings[0].rule).toBe("Email") + expect(j.results.pii.findings[0].message).toContain("customers.email") + expect(j.results.pii.findings[0].message).toContain("exposed via: contact") + expect(j.results.pii.findings[0].suggestion).toBe("'***MASKED***'") + expect(j.results.pii.findings[1].rule).toBe("EmployeeId") + }) + test("pii check reports PII columns", async () => { const file = await writeSql(tmpDir.dir, "pii.sql", "SELECT email, ssn FROM customers;") setDispatcherResponse("altimate_core.query_pii", () => ({ From e3db0a4142ebb1d3db74df10581ed8a2819b6dac Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 12 Aug 2026 16:43:31 +0530 Subject: [PATCH 02/14] fix: never emit `null` suggestion from `suggested_masking` in PII check `f.suggestion ?? f.suggested_masking` leaked `null` into the `suggestion` field when the engine emits `suggested_masking: null`. Coerce to `undefined` and lock with a test. (Kilo review follow-up on #1090.) Co-Authored-By: Claude Fable 5 --- packages/opencode/src/cli/cmd/check.ts | 3 ++- packages/opencode/test/cli/check-e2e.test.ts | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index cf23573e8c..2f12508e76 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -197,7 +197,8 @@ async function runPii(sql: string, file: string, schemaPath?: string): Promise { expect(j.results.pii.findings[0].message).toContain("exposed via: contact") expect(j.results.pii.findings[0].suggestion).toBe("'***MASKED***'") expect(j.results.pii.findings[1].rule).toBe("EmployeeId") + // suggested_masking: null must not leak into suggestion as null. + expect(j.results.pii.findings[1].suggestion).toBeUndefined() }) test("pii check reports PII columns", async () => { From 00d0a02467b3b85b4a228123e1b4c9c33fb7dc43 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 12 Aug 2026 18:36:36 +0530 Subject: [PATCH 03/14] fix: sync remaining engine-output consumers flagged by consensus review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 contract sync for PR #1090, fixing every Critical/Major finding from the 4-model consensus review (all verified against the live 0.7.0 binary; all latent since the Python-engine elimination, not 0.7.0 breakage): Dead gates (silent false-passes): - `check --checks validate`: gated on envelope `success`, which the handler sets even for invalid SQL — every file passed. Now gates on `data.valid`, maps `ValidationError` (`location.line/column`, `suggestions`) and fails closed on engine failure. - `check --checks semantic` + `altimate_core_semantics` tool: engine returns `valid:true` WITH `findings` (cartesian product) — `valid` means "plannable", not "clean". Both consumers now read `findings` and never gate on `valid`; tool formatter/title/telemetry follow the findings list. - `check --checks grade`: read `grade`/`score`/`recommendations`, none of which `evaluate()` returns — no grade or finding ever surfaced. Now reads `overall_grade`/`scores.overall`/`lint.findings` and fails closed on the failure envelope. The check-e2e mock enshrined the fictional shape — fixed. - `schema.detect_pii` (pii-detector): read `piiData.findings`; engine `PiiReport` is `{ columns, pii_count, … }` — schema PII scanning returned zero findings for every scan. Shared `piiColumnsFromReport` now filters `classification !== "None"` on both the cache and live paths. Wrong shapes / crashes: - `altimate-core-compare` tool: read `differences` (engine: `identical`/ `diff_count`/`diffs`, `DiffEntry.change_type`) — different queries rendered "Compare: IDENTICAL". Error-gated title added. - `altimate-core-policy` tool: titled on `pass` (engine: `allowed`) — clean SQL always rendered "VIOLATIONS FOUND"; `metadata.success` now reflects the envelope and error output no longer contradicts the ERROR title. - Empty-dialect coercion centralized: new `dialectHint()` in `native/engine-coerce.ts` applied to all 7 dialect-forwarding handlers (columnLineage, formatSql, extractMetadata, compareQueries, importDdl + the two already fixed) — the compare/column-lineage/extract-metadata/ import-ddl tools crashed with `unknown dialect ''` whenever `dialect` was omitted (the common invocation path). - `{ Custom: string }` PII classifications rendered `[object Object]` in classify-pii, the composite check renderer, and the review runner's signed verdict — shared `classificationToString()` used everywhere; classify-pii also no longer counts `classification: "None"` rows and error-gates its title/output; query-pii treats engine `parse_error` as an abstention (previously rendered CLEAN for unparseable SQL). - Composite `altimate_core.check` now computes query PII (fail-safe) — the tool's "=== PII ===" section previously always printed "No PII detected" because the handler never populated it. - `ThreatFinding.location` is `[byteOffset, byteLength]` — surfaced as a `(chars a-b)` range in `check --checks safety` (was dropped entirely). Tests: real-engine "consumer contract sync (round 2)" block (semantics valid-gate, compare, policy pass/violation, classify-pii None-filter, no-dialect tool invocations, query-pii/classify-pii/policy error gating, composite-check PII, grade contract); CLI-shape tests for validate/semantic/ grade incl. fail-closed cases; pii-detector unit tests against the live engine + a DuckDB-gated e2e; legacy formatter tests updated to real shapes. Also filed upstream: stale `SafetyRule` union in the engine's index.d.ts (altimate-core-internal#764). Codex-verified twice: all shape corrections match the installed 0.7.0 engine; its five review findings (parse_error abstention, grade fail-closed, byte-length location, contradictory failure outputs, untracked files) are addressed in this commit. Co-Authored-By: Claude Fable 5 --- .../src/altimate/native/altimate-core.ts | 36 ++- .../src/altimate/native/engine-coerce.ts | 23 ++ .../altimate/native/schema/pii-detector.ts | 67 +++-- .../opencode/src/altimate/review/runner.ts | 4 +- .../src/altimate/tools/altimate-core-check.ts | 15 +- .../tools/altimate-core-classify-pii.ts | 18 +- .../altimate/tools/altimate-core-compare.ts | 23 +- .../altimate/tools/altimate-core-policy.ts | 15 +- .../altimate/tools/altimate-core-query-pii.ts | 17 +- .../altimate/tools/altimate-core-semantics.ts | 20 +- packages/opencode/src/cli/cmd/check.ts | 88 ++++--- .../altimate-core-check-formatters.test.ts | 28 ++- .../test/altimate/altimate-core-e2e.test.ts | 236 ++++++++++++++++++ ...altimate-core-semantics-formatters.test.ts | 18 +- .../test/altimate/pii-detector-e2e.test.ts | 77 ++++++ packages/opencode/test/cli/check-e2e.test.ts | 112 ++++++++- 16 files changed, 680 insertions(+), 117 deletions(-) create mode 100644 packages/opencode/src/altimate/native/engine-coerce.ts create mode 100644 packages/opencode/test/altimate/pii-detector-e2e.test.ts diff --git a/packages/opencode/src/altimate/native/altimate-core.ts b/packages/opencode/src/altimate/native/altimate-core.ts index 1ce271b7cd..780ca52b94 100644 --- a/packages/opencode/src/altimate/native/altimate-core.ts +++ b/packages/opencode/src/altimate/native/altimate-core.ts @@ -9,6 +9,7 @@ */ import * as core from "@altimateai/altimate-core" +import { dialectHint } from "./engine-coerce" import { register } from "./dispatcher" import { schemaOrEmpty, resolveSchema } from "./schema-resolver" import type { AltimateCoreResult } from "./types" @@ -181,10 +182,20 @@ export function registerAll(): void { ) : core.lint(params.sql, schema) const safety = core.scanSql(params.sql) + // PII exposure for the composite check — the tool has always rendered a + // PII section; previously nothing populated it. Additive: a PII failure + // must not fail the whole composite. + let pii: Record = {} + try { + pii = toData(core.checkQueryPii(params.sql, schema)) + } catch { + // validation/lint above already report unparseable SQL + } const data: Record = { validation: toData(validation), lint: toData(lintResult), safety: toData(safety), + pii, } return ok(true, data) } catch (e) { @@ -323,10 +334,10 @@ export function registerAll(): void { // Pass the optional dialect hint so dialect-specific compiled warehouse SQL // (e.g. Snowflake semi-structured `col:field`) parses and the pair is // decidable instead of abstaining on a syntax error. Supported since - // altimate-core@0.5.1. Use `|| undefined` (not `??`) so the default empty - // string from ReviewConfig.dialect coerces to "no hint": the engine throws - // on an unknown dialect "", and "" must mean auto-detect, not a real dialect. - const raw = await core.checkEquivalence(params.sql1, params.sql2, schema, params.dialect || undefined) + // altimate-core@0.5.1. dialectHint coerces "" (the ReviewConfig default) + // to undefined: the engine throws on an unknown dialect "", and "" must + // mean auto-detect, not a real dialect. + const raw = await core.checkEquivalence(params.sql1, params.sql2, schema, dialectHint(params.dialect)) const data = toData(raw) return ok(true, data) } catch (e) { @@ -337,10 +348,9 @@ export function registerAll(): void { // 12. altimate_core.migration register("altimate_core.migration", async (params) => { try { - // Build schema from old_ddl, analyze new_ddl against it. - // `|| undefined` (not `??`): "" must mean auto-detect — the engine - // throws on an unknown dialect "" (same coercion as equivalence above). - const schema = core.Schema.fromDdl(params.old_ddl, params.dialect || undefined) + // Build schema from old_ddl, analyze new_ddl against it. dialectHint + // coerces "" to auto-detect (the engine throws on an unknown dialect ""). + const schema = core.Schema.fromDdl(params.old_ddl, dialectHint(params.dialect)) const raw = core.analyzeMigration(params.new_ddl, schema) const data = toData(raw) return ok(true, data) @@ -434,7 +444,7 @@ export function registerAll(): void { register("altimate_core.column_lineage", async (params) => { try { const schema = resolveSchema(params.schema_path, params.schema_context) - const raw = core.columnLineage(params.sql, params.dialect ?? undefined, schema ?? undefined) + const raw = core.columnLineage(params.sql, dialectHint(params.dialect), schema ?? undefined) return ok(true, toData(raw)) } catch (e) { return fail(e) @@ -455,7 +465,7 @@ export function registerAll(): void { // 22. altimate_core.format register("altimate_core.format", async (params) => { try { - const raw = core.formatSql(params.sql, params.dialect ?? undefined) + const raw = core.formatSql(params.sql, dialectHint(params.dialect)) const data = toData(raw) return ok(true, data) } catch (e) { @@ -466,7 +476,7 @@ export function registerAll(): void { // 23. altimate_core.metadata register("altimate_core.metadata", async (params) => { try { - const raw = core.extractMetadata(params.sql, params.dialect ?? undefined) + const raw = core.extractMetadata(params.sql, dialectHint(params.dialect)) return ok(true, toData(raw)) } catch (e) { return fail(e) @@ -476,7 +486,7 @@ export function registerAll(): void { // 24. altimate_core.compare register("altimate_core.compare", async (params) => { try { - const raw = core.compareQueries(params.left_sql, params.right_sql, params.dialect ?? undefined) + const raw = core.compareQueries(params.left_sql, params.right_sql, dialectHint(params.dialect)) return ok(true, toData(raw)) } catch (e) { return fail(e) @@ -530,7 +540,7 @@ export function registerAll(): void { // 29. altimate_core.import_ddl — returns Schema, must serialize register("altimate_core.import_ddl", async (params) => { try { - const schema = core.importDdl(params.ddl, params.dialect ?? undefined) + const schema = core.importDdl(params.ddl, dialectHint(params.dialect)) const jsonObj = schema.toJson() return ok(true, { success: true, schema: toData(jsonObj) }) } catch (e) { diff --git a/packages/opencode/src/altimate/native/engine-coerce.ts b/packages/opencode/src/altimate/native/engine-coerce.ts new file mode 100644 index 0000000000..a4835ed2ee --- /dev/null +++ b/packages/opencode/src/altimate/native/engine-coerce.ts @@ -0,0 +1,23 @@ +/** + * Shared coercions for altimate-core engine values. + * + * The engine's napi surface has two recurring foot-guns for consumers: + * - `PiiClassification` is a string OR `{ Custom: string }` — naive string + * interpolation renders `[object Object]`. + * - dialect parameters throw on the empty string (`unknown dialect ''`); + * `""` must mean "auto-detect" and be passed as undefined. + */ + +/** PiiClassification is 'Email' | … | { Custom: string } | 'None'. */ +export function classificationToString(c: unknown, fallback = "PII"): string { + if (typeof c === "string") return c + if (c && typeof c === "object" && typeof (c as { Custom?: unknown }).Custom === "string") { + return (c as { Custom: string }).Custom + } + return fallback +} + +/** Coerce an optional dialect to an engine-safe hint: "" and null mean auto-detect. */ +export function dialectHint(dialect: string | undefined | null): string | undefined { + return dialect || undefined +} diff --git a/packages/opencode/src/altimate/native/schema/pii-detector.ts b/packages/opencode/src/altimate/native/schema/pii-detector.ts index 0dc7358756..e9674fc704 100644 --- a/packages/opencode/src/altimate/native/schema/pii-detector.ts +++ b/packages/opencode/src/altimate/native/schema/pii-detector.ts @@ -4,6 +4,7 @@ */ import * as core from "@altimateai/altimate-core" +import { classificationToString } from "../engine-coerce" import { getCache } from "./cache" import * as Registry from "../connections/registry" import type { @@ -12,6 +13,19 @@ import type { PiiFinding, } from "../types" +/** + * Extract the real PII rows from an engine PiiReport. + * + * The engine returns `{ columns, pii_count, risk_level, total_columns }` with + * a row for EVERY column — classification "None" means not PII. (The previous + * consumer read a nonexistent `findings` field, making detection a silent + * no-op.) Exported for tests. + */ +export function piiColumnsFromReport(piiData: unknown): Array> { + const columns = ((piiData as Record)?.columns ?? []) as Array> + return columns.filter((c) => c.classification !== "None") +} + /** * Detect PII in cached schema metadata by running altimate-core's * classifyPii() on column names and types. @@ -72,19 +86,20 @@ export async function detectPii(params: PiiDetectParams): Promise 0) { - for (const finding of piiData.findings) { - findings.push({ - warehouse: col.warehouse, - schema: col.schema_name, - table: col.table, - column: col.name, - data_type: col.data_type, - pii_category: finding.category || finding.pii_type || "UNKNOWN", - confidence: finding.confidence || "medium", - }) - tablesWithPii.add(`${col.warehouse}.${col.schema_name}.${col.table}`) - } + // Engine PiiReport: { columns, pii_count, risk_level, total_columns }; + // every column gets a row — classification "None" means not PII. + const piiColumns = piiColumnsFromReport(piiData) + for (const piiCol of piiColumns) { + findings.push({ + warehouse: col.warehouse, + schema: col.schema_name, + table: col.table, + column: col.name, + data_type: col.data_type, + pii_category: classificationToString(piiCol.classification, "UNKNOWN"), + confidence: piiCol.confidence ?? "medium", + }) + tablesWithPii.add(`${col.warehouse}.${col.schema_name}.${col.table}`) } } catch { // classifyPii may not find PII — that is expected @@ -165,19 +180,19 @@ async function detectPiiLive(params: PiiDetectParams): Promise const result = core.classifyPii(schema) const piiData = JSON.parse(JSON.stringify(result)) - if (piiData?.findings) { - for (const finding of piiData.findings) { - findings.push({ - warehouse: params.warehouse!, - schema: schemaName, - table: tableInfo.name, - column: finding.column || "", - data_type: finding.data_type, - pii_category: finding.category || finding.pii_type || "UNKNOWN", - confidence: finding.confidence || "medium", - }) - tablesWithPii.add(`${params.warehouse}.${schemaName}.${tableInfo.name}`) - } + // Engine PiiReport: { columns, … } with a row per column; "None" = not PII. + const piiColumns = piiColumnsFromReport(piiData) + for (const piiCol of piiColumns) { + findings.push({ + warehouse: params.warehouse!, + schema: schemaName, + table: tableInfo.name, + column: piiCol.column || "", + data_type: columns.find((c) => c.name === piiCol.column)?.data_type, + pii_category: classificationToString(piiCol.classification, "UNKNOWN"), + confidence: piiCol.confidence ?? "medium", + }) + tablesWithPii.add(`${params.warehouse}.${schemaName}.${tableInfo.name}`) } } catch { // ignore diff --git a/packages/opencode/src/altimate/review/runner.ts b/packages/opencode/src/altimate/review/runner.ts index e4b5e8f0fd..6004233cab 100644 --- a/packages/opencode/src/altimate/review/runner.ts +++ b/packages/opencode/src/altimate/review/runner.ts @@ -1,4 +1,5 @@ import { Dispatcher } from "../native" +import { classificationToString } from "../native/engine-coerce" import { parseManifest } from "../native/dbt/manifest" import type { CheckResult, EquivalenceResult, GradeResult, ImpactResult, ReviewRunner } from "./orchestrate" import { buildReviewSchemaContext, type SchemaContext } from "./schema-context" @@ -400,7 +401,8 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun .filter((c) => c?.classification && c.classification !== "None") .map((c) => ({ column: String(c.column ?? ""), - classification: String(c.classification ?? ""), + // classification can be { Custom: string } — String() would emit "[object Object]" + classification: classificationToString(c.classification, ""), confidence: typeof c.confidence === "number" ? c.confidence : 0, masking: c.suggested_masking ?? undefined, })) diff --git a/packages/opencode/src/altimate/tools/altimate-core-check.ts b/packages/opencode/src/altimate/tools/altimate-core-check.ts index 6762a409a7..65bcf5c7c1 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-check.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-check.ts @@ -1,6 +1,7 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" +import { classificationToString } from "../native/engine-coerce" import type { Telemetry } from "../telemetry" export const AltimateCoreCheckTool = Tool.define("altimate_core_check", { @@ -32,7 +33,7 @@ export const AltimateCoreCheckTool = Tool.define("altimate_core_check", { for (const t of data.safety?.threats ?? []) { findings.push({ category: t.rule ?? t.type ?? "safety_threat" }) } - for (const p of data.pii?.findings ?? []) { + for (const p of data.pii?.pii_columns ?? data.pii?.findings ?? []) { findings.push({ category: "pii_detected" }) } // altimate_change end @@ -62,7 +63,7 @@ export function formatCheckTitle(data: Record): string { if (!data.validation?.valid) parts.push("validation errors") if (!data.lint?.clean) parts.push(`${data.lint?.findings?.length ?? 0} lint findings`) if (!data.safety?.safe) parts.push("safety threats") - if (data.pii?.findings?.length) parts.push("PII detected") + if (data.pii?.pii_columns?.length || data.pii?.findings?.length) parts.push("PII detected") return parts.length ? parts.join(", ") : "PASS" } @@ -98,11 +99,15 @@ export function formatCheck(data: Record): string { } lines.push("\n=== PII ===") - if (!data.pii?.findings?.length) { + // Engine PiiQueryResult: { accesses_pii, pii_columns, risk_level, … } + const piiCols = (data.pii?.pii_columns ?? data.pii?.findings ?? []) as any[] + if (!piiCols.length) { lines.push("No PII detected.") } else { - for (const p of data.pii?.findings ?? []) { - lines.push(` ${p.column ?? "unknown"}: ${p.category ?? "PII"} (${p.confidence ?? "unknown"} confidence)`) + for (const p of piiCols) { + const cls = classificationToString(p.classification ?? p.category) + const where = [p.table, p.column ?? "unknown"].filter(Boolean).join(".") + lines.push(` ${where}: ${cls}${p.suggested_masking ? ` (masking: ${p.suggested_masking})` : ""}`) } } diff --git a/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts b/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts index 56ba3a1304..75b0b75f89 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts @@ -1,6 +1,13 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" +import { classificationToString } from "../native/engine-coerce" + +/** Engine PiiReport lists EVERY column; classification "None" means not PII. */ +function realPiiColumns(data: Record): any[] { + const columns = (data.columns ?? data.findings ?? []) as any[] + return columns.filter((c) => c.classification !== "None") +} export const AltimateCoreClassifyPiiTool = Tool.define("altimate_core_classify_pii", { description: @@ -16,13 +23,14 @@ export const AltimateCoreClassifyPiiTool = Tool.define("altimate_core_classify_p schema_context: args.schema_context, }) const data = (result.data ?? {}) as Record - const piiColumns = data.columns ?? data.findings ?? [] + const piiColumns = realPiiColumns(data) const findingCount = piiColumns.length const error = result.error ?? data.error return { - title: `PII Classification: ${findingCount} finding(s)`, + // Never render a finding count when the engine call itself failed. + title: error ? "PII Classification: ERROR" : `PII Classification: ${findingCount} finding(s)`, metadata: { success: result.success, finding_count: findingCount, ...(error && { error }) }, - output: formatClassifyPii(data), + output: error ? `Error: ${error}` : formatClassifyPii(data), } } catch (e) { const msg = e instanceof Error ? e.message : String(e) @@ -37,7 +45,7 @@ export const AltimateCoreClassifyPiiTool = Tool.define("altimate_core_classify_p function formatClassifyPii(data: Record): string { if (data.error) return `Error: ${data.error}` - const piiColumns = data.columns ?? data.findings ?? [] + const piiColumns = realPiiColumns(data) if (!piiColumns.length) return "No PII columns detected." const lines: string[] = [] if (data.risk_level) lines.push(`Risk level: ${data.risk_level}`) @@ -45,7 +53,7 @@ function formatClassifyPii(data: Record): string { lines.push("") lines.push("PII columns found:") for (const f of piiColumns) { - const classification = f.classification ?? f.category ?? "PII" + const classification = classificationToString(f.classification ?? f.category) const confidence = f.confidence ?? "high" const table = f.table ?? "unknown" const column = f.column ?? "unknown" diff --git a/packages/opencode/src/altimate/tools/altimate-core-compare.ts b/packages/opencode/src/altimate/tools/altimate-core-compare.ts index 21c4b0bcc8..c01c7126ed 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-compare.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-compare.ts @@ -18,12 +18,21 @@ export const AltimateCoreCompareTool = Tool.define("altimate_core_compare", { dialect: args.dialect ?? "", }) const data = (result.data ?? {}) as Record - const diffCount = data.differences?.length ?? 0 + // Engine CompareResult: { identical, diff_count, diffs } — `differences` + // never existed, so every comparison used to render IDENTICAL. + const diffs = (data.diffs ?? data.differences ?? []) as any[] + const diffCount = data.diff_count ?? diffs.length const error = result.error ?? data.error + // Never render IDENTICAL when the engine call itself failed. + const title = error + ? "Compare: ERROR" + : data.identical === false || diffCount > 0 + ? `Compare: ${diffCount} difference(s)` + : "Compare: IDENTICAL" return { - title: `Compare: ${diffCount === 0 ? "IDENTICAL" : `${diffCount} difference(s)`}`, + title, metadata: { success: result.success, difference_count: diffCount, ...(error && { error }) }, - output: formatCompare(data), + output: error ? `Error: ${error}` : formatCompare(data), } } catch (e) { const msg = e instanceof Error ? e.message : String(e) @@ -38,10 +47,12 @@ export const AltimateCoreCompareTool = Tool.define("altimate_core_compare", { function formatCompare(data: Record): string { if (data.error) return `Error: ${data.error}` - if (!data.differences?.length) return "Queries are structurally identical." + // Engine DiffEntry: { change_type, description }. + const diffs = (data.diffs ?? data.differences ?? []) as any[] + if (!diffs.length) return "Queries are structurally identical." const lines = ["Structural differences:\n"] - for (const d of data.differences) { - lines.push(` [${d.type ?? "change"}] ${d.description ?? d.message ?? d}`) + for (const d of diffs) { + lines.push(` [${d.change_type ?? d.type ?? "change"}] ${d.description ?? d.message ?? d}`) } return lines.join("\n") } diff --git a/packages/opencode/src/altimate/tools/altimate-core-policy.ts b/packages/opencode/src/altimate/tools/altimate-core-policy.ts index e55c02d293..1b0eec0c98 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-policy.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-policy.ts @@ -29,16 +29,21 @@ export const AltimateCorePolicyTool = Tool.define("altimate_core_policy", { category: v.rule ?? "policy_violation", })) // altimate_change end + // Engine PolicyResult: { allowed, violations, warnings, … } — `pass` + // never existed, so every clean query used to render VIOLATIONS FOUND. + const allowed = (data.allowed ?? data.pass) as boolean | undefined return { - title: `Policy: ${data.pass ? "PASS" : "VIOLATIONS FOUND"}`, + title: error ? "Policy: ERROR" : `Policy: ${allowed ? "PASS" : "VIOLATIONS FOUND"}`, metadata: { - success: true, // engine ran — violations are findings, not failures - pass: data.pass, + // Violations are findings, not failures — but a failed engine call + // (e.g. malformed policy JSON) must not report success. + success: result.success, + pass: allowed, has_schema: hasSchema, ...(error && { error }), ...(findings.length > 0 && { findings }), }, - output: formatPolicy(data), + output: error ? `Error: ${error}` : formatPolicy(data), } } catch (e) { const msg = e instanceof Error ? e.message : String(e) @@ -53,7 +58,7 @@ export const AltimateCorePolicyTool = Tool.define("altimate_core_policy", { function formatPolicy(data: Record): string { if (data.error) return `Error: ${data.error}` - if (data.pass) return "SQL passes all policy checks." + if (data.allowed ?? data.pass) return "SQL passes all policy checks." const lines = ["Policy violations:\n"] for (const v of data.violations ?? []) { lines.push(` [${v.severity ?? "error"}] ${v.rule}: ${v.message}`) diff --git a/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts b/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts index 1e60c2be03..8eb9a7eae8 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts @@ -1,6 +1,7 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" +import { classificationToString } from "../native/engine-coerce" export const AltimateCoreQueryPiiTool = Tool.define("altimate_core_query_pii", { description: @@ -20,11 +21,17 @@ export const AltimateCoreQueryPiiTool = Tool.define("altimate_core_query_pii", { const data = (result.data ?? {}) as Record const piiCols = data.pii_columns ?? data.exposures ?? [] const exposureCount = piiCols.length - const error = result.error ?? data.error + // The engine reports unparseable SQL via data.parse_error with an empty + // pii_columns list — that is an abstention, not a CLEAN verdict. + const error = result.error ?? data.error ?? data.parse_error + // Never render CLEAN when the engine call itself failed. + const title = error + ? "Query PII: ERROR" + : `Query PII: ${exposureCount === 0 ? "CLEAN" : `${exposureCount} exposure(s)`}` return { - title: `Query PII: ${exposureCount === 0 ? "CLEAN" : `${exposureCount} exposure(s)`}`, + title, metadata: { success: result.success, exposure_count: exposureCount, ...(error && { error }) }, - output: formatQueryPii(data), + output: error ? `Error: ${error}` : formatQueryPii(data), } } catch (e) { const msg = e instanceof Error ? e.message : String(e) @@ -45,9 +52,7 @@ function formatQueryPii(data: Record): string { if (data.risk_level) lines.push(`Risk level: ${data.risk_level}`) lines.push("PII exposure detected:\n") for (const e of piiCols) { - // Classification is a string OR { Custom: string }. - const raw = e.classification ?? e.category ?? "PII" - const classification = typeof raw === "string" ? raw : (raw?.Custom ?? "PII") + const classification = classificationToString(e.classification ?? e.category) const table = e.table ?? "unknown" const column = e.column ?? "unknown" lines.push(` ${table}.${column}: ${classification}`) diff --git a/packages/opencode/src/altimate/tools/altimate-core-semantics.ts b/packages/opencode/src/altimate/tools/altimate-core-semantics.ts index f644acf2fc..f626c45ba5 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-semantics.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-semantics.ts @@ -29,17 +29,20 @@ export const AltimateCoreSemanticsTool = Tool.define("altimate_core_semantics", schema_context: args.schema_context, }) const data = (result.data ?? {}) as Record - const issueCount = data.issues?.length ?? 0 + // Engine SemanticResult reports findings under `findings`; `valid` means + // "plannable", not "clean" — the engine returns valid:true WITH findings + // (e.g. cartesian product). Never gate the count or title on `valid`. + const issues = (data.findings ?? data.issues ?? []) as any[] + const issueCount = issues.length const error = result.error ?? data.error ?? extractSemanticsErrors(data) const hasError = Boolean(error) // altimate_change start — sql quality findings for telemetry - const issues = Array.isArray(data.issues) ? data.issues : [] - const findings: Telemetry.Finding[] = issues.map(() => ({ - category: "semantic_issue", + const findings: Telemetry.Finding[] = issues.map((f: any) => ({ + category: f?.rule ?? "semantic_issue", })) // altimate_change end return { - title: hasError ? "Semantics: ERROR" : `Semantics: ${data.valid ? "VALID" : `${issueCount} issues`}`, + title: hasError ? "Semantics: ERROR" : `Semantics: ${issueCount === 0 ? "VALID" : `${issueCount} issues`}`, metadata: { success: true, // engine ran — semantic issues are findings, not failures valid: data.valid, @@ -73,9 +76,12 @@ export function extractSemanticsErrors(data: Record): string | unde export function formatSemantics(data: Record): string { if (data.error) return `Error: ${data.error}` - if (data.valid) return "No semantic issues found." + // `valid` means "plannable", not "clean" — findings must render even when + // valid is true (e.g. cartesian product returns valid:true + findings). + const issues = (data.findings ?? data.issues ?? []) as any[] + if (!issues.length) return "No semantic issues found." const lines = ["Semantic issues:\n"] - for (const issue of data.issues ?? []) { + for (const issue of issues) { lines.push(` [${issue.severity ?? "warning"}] ${issue.rule ?? issue.type}: ${issue.message}`) if (issue.suggestion) lines.push(` Fix: ${issue.suggestion}`) } diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index 2f12508e76..82329aab5c 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -63,19 +63,27 @@ async function runValidate(sql: string, file: string, schemaPath?: string): Prom sql, schema_path: schemaPath ?? "", }) - if (result.success) return [] + // The handler returns success=true even for invalid SQL — the verdict + // lives in data.valid (engine ValidationResult). Gating on success alone + // made this check pass every file. + if (result.success && result.data.valid !== false) return [] const errors = (result.data.errors ?? result.data.findings ?? []) as Array> if (errors.length > 0) { - return errors.map((f) => ({ - file, - line: f.line as number | undefined, - column: f.column as number | undefined, - code: f.code as string | undefined, - rule: "validate", - severity: normalizeSeverity(f.severity as string), - message: (f.message ?? f.description ?? "") as string, - suggestion: f.suggestion as string | undefined, - })) + return errors.map((f) => { + // Engine ValidationError: { code, kind, message, location: {line, column} | null, suggestions } + const location = f.location as { line?: number; column?: number } | null | undefined + const suggestions = f.suggestions as Array> | undefined + return { + file, + line: (location?.line ?? f.line) as number | undefined, + column: (location?.column ?? f.column) as number | undefined, + code: f.code as string | undefined, + rule: "validate", + severity: "error" as const, + message: (f.message ?? f.description ?? "") as string, + suggestion: (f.suggestion ?? suggestions?.[0]?.message) as string | undefined, + } + }) } // If no structured errors but validation failed, emit a single finding const errorMsg = result.error ?? result.data.error ?? "SQL validation failed" @@ -102,16 +110,21 @@ async function runSafety(sql: string, file: string): Promise { result.data.findings ?? []) as Array> if (issues.length > 0) { - return issues.map((f) => ({ - file, - line: f.line as number | undefined, - column: f.column as number | undefined, - code: f.code as string | undefined, - rule: (f.rule ?? f.category ?? "safety") as string, - severity: normalizeSeverity(f.severity as string), - message: (f.message ?? f.description ?? "") as string, - suggestion: (f.suggestion ?? f.detail) as string | undefined, - })) + return issues.map((f) => { + // ThreatFinding.location is [byteOffset, byteLength], not [start, end]. + const loc = f.location as [number, number] | undefined + const at = Array.isArray(loc) ? ` (chars ${loc[0]}-${loc[0] + loc[1]})` : "" + return { + file, + line: f.line as number | undefined, + column: f.column as number | undefined, + code: f.code as string | undefined, + rule: (f.rule ?? f.category ?? "safety") as string, + severity: normalizeSeverity(f.severity as string), + message: `${(f.message ?? f.description ?? "") as string}${at}`, + suggestion: (f.suggestion ?? f.detail) as string | undefined, + } + }) } if (!result.success || result.data.safe === false) { return [ @@ -213,8 +226,15 @@ async function runSemantic(sql: string, file: string, schemaPath?: string): Prom sql, schema_path: schemaPath ?? "", }) - if (result.success && result.data.valid !== false) return [] - const issues = (result.data.issues ?? result.data.findings ?? []) as Array> + // Engine SemanticResult reports findings under `findings` and can return + // valid:true WITH findings (e.g. cartesian product) — `valid` means + // "plannable", not "clean". Never gate findings on it. + const issues = (result.data.findings ?? result.data.issues ?? []) as Array> + // Fail closed on engine failure, but only when there are no structured + // findings to surface. + if (!result.success && issues.length === 0) { + return [dispatcherErrorFinding("semantic", file, result.error ?? "altimate_core.semantics failed")] + } if (issues.length > 0) { return issues.map((f) => ({ file, @@ -254,9 +274,20 @@ async function runGrade( sql, schema_path: schemaPath ?? "", }) - const issues = (result.data.issues ?? result.data.findings ?? result.data.recommendations ?? []) as Array< - Record - > + // Native handlers report failures via the envelope (success:false), not by + // throwing — fail closed instead of emitting a passing empty grade. + if (!result.success) { + return { findings: [dispatcherErrorFinding("grade", file, result.error ?? "altimate_core.grade failed")] } + } + // Engine EvalResult: { explain, lint, overall_grade, safety, scores, sql, + // total_time_ms, validation } — the grade is `overall_grade`, the numeric + // score is `scores.overall`, and actionable findings live in `lint.findings`. + const lint = result.data.lint as Record | undefined + const issues = (result.data.issues ?? + result.data.findings ?? + result.data.recommendations ?? + lint?.findings ?? + []) as Array> const findings = issues.map((f) => ({ file, line: f.line as number | undefined, @@ -268,8 +299,9 @@ async function runGrade( suggestion: f.suggestion as string | undefined, })) // Preserve the primary A-F grade value from the backend - const grade = (result.data.grade ?? result.data.letter_grade) as string | undefined - const score = (result.data.score ?? result.data.numeric_score) as number | undefined + const grade = (result.data.overall_grade ?? result.data.grade ?? result.data.letter_grade) as string | undefined + const scores = result.data.scores as Record | undefined + const score = (scores?.overall ?? result.data.score ?? result.data.numeric_score) as number | undefined return { findings, grade, score } } catch (e) { console.error(`[grade] error processing ${file}: ${e instanceof Error ? e.message : String(e)}`) diff --git a/packages/opencode/test/altimate/altimate-core-check-formatters.test.ts b/packages/opencode/test/altimate/altimate-core-check-formatters.test.ts index 4594f3e877..264d20cc0e 100644 --- a/packages/opencode/test/altimate/altimate-core-check-formatters.test.ts +++ b/packages/opencode/test/altimate/altimate-core-check-formatters.test.ts @@ -120,21 +120,37 @@ describe("formatCheck", () => { expect(output).toContain("[critical] sql_injection: Tautology detected: 1=1") }) - test("formats PII findings with column and confidence", () => { + test("formats PII exposures from the engine PiiQueryResult shape", () => { const data = { validation: { valid: true }, lint: { clean: true }, safety: { safe: true }, pii: { - findings: [ - { column: "ssn", category: "SSN", confidence: "high" }, - { column: "email", category: "EMAIL", confidence: "medium" }, + accesses_pii: true, + risk_level: "High", + pii_columns: [ + { table: "customers", column: "ssn", classification: "SSN", query_targets: [], suggested_masking: "'***'" }, + { table: "customers", column: "email", classification: { Custom: "WorkEmail" }, query_targets: ["contact"], suggested_masking: null }, ], }, } const output = formatCheck(data) - expect(output).toContain("ssn: SSN (high confidence)") - expect(output).toContain("email: EMAIL (medium confidence)") + expect(output).toContain("customers.ssn: SSN (masking: '***')") + // { Custom: string } classifications must not render as [object Object] + expect(output).toContain("customers.email: WorkEmail") + expect(output).not.toContain("[object Object]") + }) + + test("formats PII findings via the legacy findings fallback", () => { + const data = { + validation: { valid: true }, + lint: { clean: true }, + safety: { safe: true }, + pii: { findings: [{ column: "ssn", category: "SSN", confidence: "high" }] }, + } + const output = formatCheck(data) + expect(output).toContain("ssn: SSN") + expect(output).not.toContain("No PII detected") }) test("handles empty/missing sections without crashing", () => { diff --git a/packages/opencode/test/altimate/altimate-core-e2e.test.ts b/packages/opencode/test/altimate/altimate-core-e2e.test.ts index 92a5522b46..5c3c6cbab0 100644 --- a/packages/opencode/test/altimate/altimate-core-e2e.test.ts +++ b/packages/opencode/test/altimate/altimate-core-e2e.test.ts @@ -1062,3 +1062,239 @@ describeIf("altimate-core E2E", () => { }) }) }) + +// =========================================================================== +// Consumer contract sync — consensus-review round 2 regression tests. +// Every test here runs the REAL engine; each locks a consumer that previously +// read a field the engine never returns (latent since the Python-engine +// elimination) or crashed on the default empty-dialect invocation. +// =========================================================================== + +const toolCtx = () => + ({ + sessionID: "test", + messageID: "test", + agent: "test", + abort: new AbortController().signal, + messages: [], + metadata: () => {}, + }) as any + +describeIf("consumer contract sync (round 2)", () => { + const SCHEMA = { + customers: { + customer_id: "INTEGER", + email: "VARCHAR", + first_name: "VARCHAR", + }, + orders: { order_id: "INTEGER", customer_id: "INTEGER" }, + } + + beforeAll(async () => { + const { registerAll } = await import("../../src/altimate/native/altimate-core") + registerAll() + }) + + describe("semantics valid-gate", () => { + test("engine contract: cartesian join returns valid:true WITH findings", async () => { + const { Dispatcher } = await import("../../src/altimate/native") + const r = await Dispatcher.call("altimate_core.semantics", { + sql: "SELECT * FROM customers, orders", + schema_context: SCHEMA, + }) + const d = r.data as any + expect(d.valid).toBe(true) + expect((d.findings ?? []).some((f: any) => f.rule === "missing_join_condition")).toBe(true) + }) + + test("semantics tool surfaces findings despite valid:true", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreSemanticsTool } = await import("../../src/altimate/tools/altimate-core-semantics") + const tool = await initTool(AltimateCoreSemanticsTool) + const result = await tool.execute({ sql: "SELECT * FROM customers, orders", schema_context: SCHEMA }, toolCtx()) + expect(result.title).not.toContain("VALID") + expect(result.title).toMatch(/\d+ issues/) + expect(result.output).toContain("missing_join_condition") + }) + }) + + describe("compare tool (engine shape: identical/diff_count/diffs)", () => { + test("different queries do NOT render IDENTICAL (no dialect)", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreCompareTool } = await import("../../src/altimate/tools/altimate-core-compare") + const tool = await initTool(AltimateCoreCompareTool) + const result = await tool.execute( + { + left_sql: "SELECT customer_id FROM customers", + right_sql: "SELECT customer_id FROM customers WHERE customer_id = 1", + }, + toolCtx(), + ) + expect(result.title).not.toContain("IDENTICAL") + expect(result.title).toMatch(/\d+ difference/) + expect(result.metadata.difference_count).toBeGreaterThan(0) + }) + + test("identical queries render IDENTICAL", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreCompareTool } = await import("../../src/altimate/tools/altimate-core-compare") + const tool = await initTool(AltimateCoreCompareTool) + const result = await tool.execute( + { left_sql: "SELECT customer_id FROM customers", right_sql: "SELECT customer_id FROM customers" }, + toolCtx(), + ) + expect(result.title).toBe("Compare: IDENTICAL") + }) + }) + + describe("policy tool (engine shape: allowed/violations)", () => { + const POLICY = JSON.stringify({ + data_protection: { blocked_columns: [{ table: "customers", columns: ["email"] }] }, + }) + + test("clean SQL renders PASS (previously always VIOLATIONS FOUND)", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCorePolicyTool } = await import("../../src/altimate/tools/altimate-core-policy") + const tool = await initTool(AltimateCorePolicyTool) + const result = await tool.execute( + { sql: "SELECT customer_id FROM customers", policy_json: POLICY, schema_context: SCHEMA }, + toolCtx(), + ) + expect(result.title).toBe("Policy: PASS") + }) + + test("blocked column renders VIOLATIONS FOUND with the rule", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCorePolicyTool } = await import("../../src/altimate/tools/altimate-core-policy") + const tool = await initTool(AltimateCorePolicyTool) + const result = await tool.execute( + { sql: "SELECT email FROM customers", policy_json: POLICY, schema_context: SCHEMA }, + toolCtx(), + ) + expect(result.title).toBe("Policy: VIOLATIONS FOUND") + expect(result.output).toContain("blocked_columns") + }) + }) + + describe("classify-pii tool (None rows excluded)", () => { + test("finding count excludes classification:'None' columns", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreClassifyPiiTool } = await import("../../src/altimate/tools/altimate-core-classify-pii") + const tool = await initTool(AltimateCoreClassifyPiiTool) + // 3 customers columns: email is PII; customer_id/first_name may or may not + // be — but order_id definitely is not. Assert count matches engine pii_count. + const result = await tool.execute({ schema_context: SCHEMA }, toolCtx()) + const { Dispatcher } = await import("../../src/altimate/native") + const raw = await Dispatcher.call("altimate_core.classify_pii", { schema_context: SCHEMA }) + const piiCount = (raw.data as any).pii_count + expect(result.metadata.finding_count).toBe(piiCount) + expect(result.output).not.toContain(": None") + }) + }) + + describe("no-dialect tool invocation (empty-string coercion in handlers)", () => { + test("column-lineage tool works without dialect", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreColumnLineageTool } = await import("../../src/altimate/tools/altimate-core-column-lineage") + const tool = await initTool(AltimateCoreColumnLineageTool) + const result = await tool.execute({ sql: "SELECT customer_id FROM customers", schema_context: SCHEMA }, toolCtx()) + expect(result.metadata.error ?? "").not.toContain("unknown dialect") + }) + + test("extract-metadata tool works without dialect", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreExtractMetadataTool } = await import("../../src/altimate/tools/altimate-core-extract-metadata") + const tool = await initTool(AltimateCoreExtractMetadataTool) + const result = await tool.execute({ sql: "SELECT customer_id FROM customers" }, toolCtx()) + expect(result.metadata.error ?? "").not.toContain("unknown dialect") + expect(result.output).toContain("customers") + }) + + test("import-ddl tool works without dialect", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreImportDdlTool } = await import("../../src/altimate/tools/altimate-core-import-ddl") + const tool = await initTool(AltimateCoreImportDdlTool) + const result = await tool.execute({ ddl: "CREATE TABLE t (id INT, email VARCHAR);" }, toolCtx()) + expect(result.metadata.error ?? "").not.toContain("unknown dialect") + }) + + test("compare tool works without dialect (was: unknown dialect '')", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreCompareTool } = await import("../../src/altimate/tools/altimate-core-compare") + const tool = await initTool(AltimateCoreCompareTool) + const result = await tool.execute({ left_sql: "SELECT 1", right_sql: "SELECT 1" }, toolCtx()) + expect(result.metadata.error ?? "").not.toContain("unknown dialect") + }) + }) + + describe("query-pii error gating", () => { + test("unparseable SQL renders ERROR, not CLEAN (engine abstains via parse_error)", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreQueryPiiTool } = await import("../../src/altimate/tools/altimate-core-query-pii") + const tool = await initTool(AltimateCoreQueryPiiTool) + // Live engine returns { parse_error: "…", pii_columns: [] } for this — + // an abstention that previously rendered "Query PII: CLEAN". + const result = await tool.execute({ sql: "SELECT FROM", schema_context: SCHEMA }, toolCtx()) + expect(result.title).toBe("Query PII: ERROR") + expect(result.metadata.error).toBeDefined() + expect(result.output).toContain("Error") + }) + }) + + describe("failure output consistency (title and body must agree)", () => { + test("classify-pii engine error renders Error output, not 'No PII columns detected'", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreClassifyPiiTool } = await import("../../src/altimate/tools/altimate-core-classify-pii") + const tool = await initTool(AltimateCoreClassifyPiiTool) + // Nonexistent schema_path → engine failure envelope + const result = await tool.execute({ schema_path: "/nonexistent/schema.json" }, toolCtx()) + if (result.title === "PII Classification: ERROR") { + expect(result.output).toContain("Error") + expect(result.output).not.toContain("No PII columns detected") + } + }) + + test("policy tool malformed policy JSON renders ERROR with failing metadata", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCorePolicyTool } = await import("../../src/altimate/tools/altimate-core-policy") + const tool = await initTool(AltimateCorePolicyTool) + const result = await tool.execute( + { sql: "SELECT 1", policy_json: "not-valid-json{", schema_context: SCHEMA }, + toolCtx(), + ) + expect(result.title).toBe("Policy: ERROR") + expect(result.metadata.success).toBe(false) + expect(result.output).toContain("Error") + }) + }) + + describe("composite check PII section (now wired)", () => { + test("check tool reports PII exposure for a PII query", async () => { + const { initTool } = await import("./tool-fixture") + const { AltimateCoreCheckTool } = await import("../../src/altimate/tools/altimate-core-check") + const tool = await initTool(AltimateCoreCheckTool) + const result = await tool.execute( + { sql: "SELECT email AS contact FROM customers", schema_context: SCHEMA }, + toolCtx(), + ) + expect(result.output).toContain("=== PII ===") + expect(result.output).toContain("customers.email") + expect(result.output).not.toContain("No PII detected") + }) + }) + + describe("grade CLI mapping (engine shape: overall_grade/scores.overall)", () => { + test("engine contract: evaluate has overall_grade and scores.overall, no grade/score", async () => { + const { Dispatcher } = await import("../../src/altimate/native") + const r = await Dispatcher.call("altimate_core.grade", { + sql: "SELECT customer_id FROM customers", + schema_context: SCHEMA, + }) + const d = r.data as any + expect(d.overall_grade).toBeDefined() + expect(d.scores?.overall).toBeDefined() + expect(d.grade).toBeUndefined() + expect(d.score).toBeUndefined() + }) + }) +}) diff --git a/packages/opencode/test/altimate/altimate-core-semantics-formatters.test.ts b/packages/opencode/test/altimate/altimate-core-semantics-formatters.test.ts index 6a271120ce..8a5fbbfb51 100644 --- a/packages/opencode/test/altimate/altimate-core-semantics-formatters.test.ts +++ b/packages/opencode/test/altimate/altimate-core-semantics-formatters.test.ts @@ -43,12 +43,20 @@ describe("formatSemantics", () => { expect(formatSemantics(data)).toBe("No semantic issues found.") }) - test("shows issues header when data.valid is false even with empty issues", () => { - // This tests the degenerate case: valid=false but no issues array - const data = { valid: false } + test("findings-driven: no findings renders clean regardless of valid flag", () => { + // `valid` means "plannable", not "clean" — rendering is driven by the + // findings list. valid:false with no findings surfaces its detail via + // validation_errors (the error path), not a bare issues header. + expect(formatSemantics({ valid: false })).toBe("No semantic issues found.") + }) + + test("findings render even when valid is true (cartesian returns valid:true + findings)", () => { + const data = { + valid: true, + findings: [{ severity: "error", rule: "missing_join_condition", message: "Cartesian product detected" }], + } const output = formatSemantics(data) - expect(output).toContain("Semantic issues:") - // No actual issue lines since data.issues is undefined + expect(output).toContain("[error] missing_join_condition: Cartesian product detected") }) test("lists issues with severity and rule", () => { diff --git a/packages/opencode/test/altimate/pii-detector-e2e.test.ts b/packages/opencode/test/altimate/pii-detector-e2e.test.ts new file mode 100644 index 0000000000..47ab033f50 --- /dev/null +++ b/packages/opencode/test/altimate/pii-detector-e2e.test.ts @@ -0,0 +1,77 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test" + +import { detectPii, piiColumnsFromReport } from "../../src/altimate/native/schema/pii-detector" +import * as Registry from "../../src/altimate/native/connections/registry" + +const RUN = process.env.ALTIMATE_RUN_WAREHOUSE_E2E === "1" +const e2eTest = RUN ? test : test.skip + +describe("piiColumnsFromReport (real engine PiiReport shape)", () => { + test("extracts PII rows and drops 'None' rows from a live classifyPii result", () => { + const core = require("@altimateai/altimate-core") + const schema = core.Schema.fromJson( + JSON.stringify({ + tables: { + users: { + columns: [ + { name: "id", type: "INTEGER" }, + { name: "email", type: "VARCHAR" }, + { name: "note", type: "VARCHAR" }, + ], + }, + }, + }), + ) + const report = JSON.parse(JSON.stringify(core.classifyPii(schema))) + // Sanity: the engine returns a row per column under `columns` (not `findings`). + expect(report.findings).toBeUndefined() + expect(report.columns.length).toBe(3) + + const pii = piiColumnsFromReport(report) + expect(pii.length).toBe(report.pii_count) + expect(pii.some((c) => c.column === "email" && c.classification === "Email")).toBe(true) + expect(pii.some((c) => c.classification === "None")).toBe(false) + }) + + test("returns empty for empty/malformed reports", () => { + expect(piiColumnsFromReport(undefined)).toEqual([]) + expect(piiColumnsFromReport({})).toEqual([]) + expect(piiColumnsFromReport({ findings: [{ category: "email" }] })).toEqual([]) + }) +}) + +/** + * Regression: pii-detector read `piiData.findings` + `finding.category`, but the + * engine's classifyPii returns PiiReport `{ columns, pii_count, … }` with + * `classification` — so schema-level PII detection silently returned zero + * findings for every scan (latent since the Python-engine elimination). + */ +describe("schema.detect_pii DuckDB e2e", () => { + beforeAll(() => { + process.env.ALTIMATE_TELEMETRY_DISABLED = "true" + }) + + afterAll(() => { + delete process.env.ALTIMATE_TELEMETRY_DISABLED + Registry.reset() + }) + + e2eTest("detects PII columns through a real DuckDB warehouse (live path)", async () => { + Registry.reset() + Registry.setConfigs({ duck_pii_e2e: { type: "duckdb", path: ":memory:" } }) + const conn = await Registry.get("duck_pii_e2e") + + await conn.execute("CREATE TABLE users (id INTEGER, email VARCHAR, note VARCHAR)") + + const result = await detectPii({ warehouse: "duck_pii_e2e" }) + + expect(result.success).toBe(true) + expect(result.finding_count).toBeGreaterThan(0) + const emailFinding = result.findings.find((f) => f.column === "email") + expect(emailFinding).toBeDefined() + expect(emailFinding!.pii_category).toBe("Email") + // Non-PII columns (classification "None") must NOT be reported. + expect(result.findings.some((f) => f.pii_category === "None")).toBe(false) + expect(result.findings.some((f) => f.pii_category === "UNKNOWN")).toBe(false) + }) +}) diff --git a/packages/opencode/test/cli/check-e2e.test.ts b/packages/opencode/test/cli/check-e2e.test.ts index 1d91ef23aa..0961d83e1f 100644 --- a/packages/opencode/test/cli/check-e2e.test.ts +++ b/packages/opencode/test/cli/check-e2e.test.ts @@ -611,6 +611,8 @@ describe("check command E2E", () => { severity: "high", message: "Unbalanced quote suggests injection breakout", detail: "Quote count is odd within a single statement", + // Real engine semantics: [byteOffset, byteLength] — "OR 1=1 " at 37. + location: [37, 7], matched_pattern: "' OR 1=1 --", }, ], @@ -622,7 +624,8 @@ describe("check command E2E", () => { const j = parseJson(r.stdout) expect(j.results.safety.findings).toHaveLength(1) expect(j.results.safety.findings[0].rule).toBe("unbalanced_quote") - expect(j.results.safety.findings[0].message).toBe("Unbalanced quote suggests injection breakout") + // ThreatFinding.location is [byteOffset, byteLength] — rendered as an offset range. + expect(j.results.safety.findings[0].message).toBe("Unbalanced quote suggests injection breakout (chars 37-44)") expect(j.results.safety.findings[0].suggestion).toBe("Quote count is odd within a single statement") // Engine severity "high" must normalize to error, not degrade to info — // otherwise --fail-on/--severity filters silently pass high-risk injections. @@ -707,13 +710,22 @@ describe("check command E2E", () => { expect(j.results.semantic.findings[0].rule).toBe("cartesian-join") }) - test("grade check returns recommendations", async () => { + test("grade check maps the real EvalResult shape (overall_grade/scores.overall/lint.findings)", async () => { + // Real core@0.7.0 evaluate() shape — the previous mock used grade/recommendations, + // fields the engine never returns, which enshrined a dead consumer. const file = await writeSql(tmpDir.dir, "grade.sql", "SELECT * FROM big_table;") setDispatcherResponse("altimate_core.grade", () => ({ success: true, data: { - grade: "C", - recommendations: [{ rule: "selectivity", severity: "info", message: "Add WHERE clause" }], + overall_grade: "C", + scores: { overall: 0.72, complexity: 0.9, safety: 1, style: 0.6, syntax: 1 }, + lint: { + clean: false, + findings: [{ rule: "select-star", severity: "info", message: "Add WHERE clause or explicit columns" }], + }, + explain: {}, + safety: { safe: true }, + validation: { valid: true }, }, })) installDispatcherMocks() @@ -722,6 +734,98 @@ describe("check command E2E", () => { const j = parseJson(r.stdout) expect(j.results.grade.findings).toHaveLength(1) expect(j.results.grade.findings[0].message).toContain("WHERE clause") + expect(j.results.grade.grade).toBe("C") + expect(j.results.grade.score).toBe(0.72) + }) + + test("grade check fails closed on engine failure envelope", async () => { + // Native handlers report failures via {success:false}, not by throwing — + // a failed grade run must not pass silently with zero findings. + const file = await writeSql(tmpDir.dir, "grade-fail.sql", "SELECT 1;") + setDispatcherResponse("altimate_core.grade", () => ({ + success: false, + data: {}, + error: "Failed to parse JSON schema", + })) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [file], checks: "grade" })) + const j = parseJson(r.stdout) + expect(j.results.grade.findings).toHaveLength(1) + expect(j.results.grade.findings[0].severity).toBe("error") + expect(j.results.grade.findings[0].message).toContain("Failed to parse JSON schema") + }) + + test("validate check fails invalid SQL despite handler success (dead-gate regression)", async () => { + // The native handler returns success=true even for invalid SQL — the + // verdict is data.valid. Gating on success alone made validate a no-op. + const file = await writeSql(tmpDir.dir, "invalid.sql", "SELECT zzz FROM t;") + setDispatcherResponse("altimate_core.validate", () => ({ + success: true, + data: { + valid: false, + errors: [ + { + code: "E002", + kind: { type: "ColumnNotFound", column: "zzz", table: null }, + message: "Column 'zzz' not found", + location: { line: 1, column: 8 }, + suggestions: [], + }, + ], + warnings: [], + }, + })) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [file], checks: "validate" })) + const j = parseJson(r.stdout) + expect(j.results.validate.findings).toHaveLength(1) + expect(j.results.validate.findings[0].severity).toBe("error") + expect(j.results.validate.findings[0].message).toBe("Column 'zzz' not found") + expect(j.results.validate.findings[0].line).toBe(1) + }) + + test("validate check passes valid SQL", async () => { + const file = await writeSql(tmpDir.dir, "valid.sql", "SELECT id FROM t;") + setDispatcherResponse("altimate_core.validate", () => ({ + success: true, + data: { valid: true, errors: [], warnings: [] }, + })) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [file], checks: "validate" })) + const j = parseJson(r.stdout) + expect(j.results.validate.findings).toHaveLength(0) + }) + + test("semantic check surfaces findings when valid:true (valid means plannable, not clean)", async () => { + const file = await writeSql(tmpDir.dir, "cartesian.sql", "SELECT * FROM a, b;") + setDispatcherResponse("altimate_core.semantics", () => ({ + success: true, + data: { + valid: true, + semantic_score: 0.5, + findings: [ + { + rule: "missing_join_condition", + severity: "error", + message: "Cartesian product detected between 'a' and 'b'", + explanation: "…", + confidence: 0.95, + }, + ], + passed_checks: [], + validation_errors: [], + }, + })) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [file], checks: "semantic" })) + const j = parseJson(r.stdout) + expect(j.results.semantic.findings).toHaveLength(1) + expect(j.results.semantic.findings[0].rule).toBe("missing_join_condition") + expect(j.results.semantic.findings[0].severity).toBe("error") }) // --- Schema resolution --- From a03901b84141674afa40efb96bd3d7147fb84fbd Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 12 Aug 2026 19:02:50 +0530 Subject: [PATCH 04/14] =?UTF-8?q?fix:=20address=20cubic=20round-2=20review?= =?UTF-8?q?=20=E2=80=94=20runner=20PII=20extraction=20+=20consumer=20polis?= =?UTF-8?q?h?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `review/runner.ts`: the composite check's `data.pii` is now the engine `PiiQueryResult` object — extract columns from `pii_columns` (legacy array shape kept as fallback) so check-derived PII columns reach the signed review verdict. - Composite check renderer: surface `query_targets` ("exposed via: …") and honor the engine's `parse_error` abstention ("PII check skipped: …") instead of rendering "No PII detected" for unparseable SQL. - `pii-detector`: band numeric engine confidence (0..1) to `high`/`medium`/`low` via shared `bandConfidence` — `PiiFinding.confidence` is a string field. - `check --checks validate`: normalize string-shaped `suggestions` entries as well as `Suggestion` objects. - Safety location label: `bytes a-b` (engine offsets are byte-based and diverge from char indexes on multibyte SQL). - Semantics tool metadata reports `result.success` instead of hardcoding true (consistent with the policy/compare tools). - `classify-pii` tool reuses `piiColumnsFromReport` instead of a second None-filter implementation. - Tests: classify-pii error case made deterministic (malformed schema file — previously guarded by an `if`, so it could pass vacuously); composite-check alias + parse-error abstention assertions added; bytes label updated. Co-Authored-By: Claude Fable 5 --- .../src/altimate/native/engine-coerce.ts | 12 +++++++ .../altimate/native/schema/pii-detector.ts | 6 ++-- .../opencode/src/altimate/review/runner.ts | 4 ++- .../src/altimate/tools/altimate-core-check.ts | 10 ++++-- .../tools/altimate-core-classify-pii.ts | 6 ++-- .../altimate/tools/altimate-core-semantics.ts | 4 ++- packages/opencode/src/cli/cmd/check.ts | 10 +++--- .../test/altimate/altimate-core-e2e.test.ts | 31 ++++++++++++++++--- packages/opencode/test/cli/check-e2e.test.ts | 4 +-- 9 files changed, 67 insertions(+), 20 deletions(-) diff --git a/packages/opencode/src/altimate/native/engine-coerce.ts b/packages/opencode/src/altimate/native/engine-coerce.ts index a4835ed2ee..7090b882c8 100644 --- a/packages/opencode/src/altimate/native/engine-coerce.ts +++ b/packages/opencode/src/altimate/native/engine-coerce.ts @@ -21,3 +21,15 @@ export function classificationToString(c: unknown, fallback = "PII"): string { export function dialectHint(dialect: string | undefined | null): string | undefined { return dialect || undefined } + +/** Map the engine's numeric confidence (0..1) to a string band. */ +export function bandConfidence(c: unknown): "high" | "medium" | "low" { + if (typeof c === "string") { + const s = c.toLowerCase() + if (s === "high" || s === "medium" || s === "low") return s + } + const n = typeof c === "number" ? c : NaN + if (n >= 0.8) return "high" + if (n >= 0.5) return "medium" + return "low" +} diff --git a/packages/opencode/src/altimate/native/schema/pii-detector.ts b/packages/opencode/src/altimate/native/schema/pii-detector.ts index e9674fc704..79ab15bbe2 100644 --- a/packages/opencode/src/altimate/native/schema/pii-detector.ts +++ b/packages/opencode/src/altimate/native/schema/pii-detector.ts @@ -4,7 +4,7 @@ */ import * as core from "@altimateai/altimate-core" -import { classificationToString } from "../engine-coerce" +import { bandConfidence, classificationToString } from "../engine-coerce" import { getCache } from "./cache" import * as Registry from "../connections/registry" import type { @@ -97,7 +97,7 @@ export async function detectPii(params: PiiDetectParams): Promise column: piiCol.column || "", data_type: columns.find((c) => c.name === piiCol.column)?.data_type, pii_category: classificationToString(piiCol.classification, "UNKNOWN"), - confidence: piiCol.confidence ?? "medium", + confidence: bandConfidence(piiCol.confidence), }) tablesWithPii.add(`${params.warehouse}.${schemaName}.${tableInfo.name}`) } diff --git a/packages/opencode/src/altimate/review/runner.ts b/packages/opencode/src/altimate/review/runner.ts index 6004233cab..1838e8793f 100644 --- a/packages/opencode/src/altimate/review/runner.ts +++ b/packages/opencode/src/altimate/review/runner.ts @@ -228,7 +228,9 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun // RAW issues (which still carry column/target/name) — the normalized // `issues` drop those fields, so mapping over them would always miss. const piiColumns = [ - ...asArray(data.pii).map(piiColumnOf), + // data.pii is the engine PiiQueryResult ({ pii_columns, … }); the + // legacy array shape is kept as a fallback. + ...asArray((data.pii as any)?.pii_columns ?? data.pii).map(piiColumnOf), ...rawIssues .filter((i: any) => /pii|sensitive/i.test(String(i.category ?? i.rule ?? i.code ?? i.kind ?? ""))) .map(piiColumnOf), diff --git a/packages/opencode/src/altimate/tools/altimate-core-check.ts b/packages/opencode/src/altimate/tools/altimate-core-check.ts index 65bcf5c7c1..6f431e1125 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-check.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-check.ts @@ -99,15 +99,19 @@ export function formatCheck(data: Record): string { } lines.push("\n=== PII ===") - // Engine PiiQueryResult: { accesses_pii, pii_columns, risk_level, … } + // Engine PiiQueryResult: { accesses_pii, pii_columns, risk_level, parse_error? } const piiCols = (data.pii?.pii_columns ?? data.pii?.findings ?? []) as any[] - if (!piiCols.length) { + if (data.pii?.parse_error) { + // Abstention, not a clean verdict — the engine could not parse the query. + lines.push(`PII check skipped: ${data.pii.parse_error}`) + } else if (!piiCols.length) { lines.push("No PII detected.") } else { for (const p of piiCols) { const cls = classificationToString(p.classification ?? p.category) const where = [p.table, p.column ?? "unknown"].filter(Boolean).join(".") - lines.push(` ${where}: ${cls}${p.suggested_masking ? ` (masking: ${p.suggested_masking})` : ""}`) + const via = Array.isArray(p.query_targets) && p.query_targets.length ? ` exposed via: ${p.query_targets.join(", ")}` : "" + lines.push(` ${where}: ${cls}${via}${p.suggested_masking ? ` (masking: ${p.suggested_masking})` : ""}`) } } diff --git a/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts b/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts index 75b0b75f89..bedc1a31d2 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts @@ -2,11 +2,13 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" import { classificationToString } from "../native/engine-coerce" +import { piiColumnsFromReport } from "../native/schema/pii-detector" /** Engine PiiReport lists EVERY column; classification "None" means not PII. */ function realPiiColumns(data: Record): any[] { - const columns = (data.columns ?? data.findings ?? []) as any[] - return columns.filter((c) => c.classification !== "None") + if (Array.isArray(data.columns)) return piiColumnsFromReport(data) + // Legacy fallback shape. + return ((data.findings ?? []) as any[]).filter((c) => c.classification !== "None") } export const AltimateCoreClassifyPiiTool = Tool.define("altimate_core_classify_pii", { diff --git a/packages/opencode/src/altimate/tools/altimate-core-semantics.ts b/packages/opencode/src/altimate/tools/altimate-core-semantics.ts index f626c45ba5..302e22c749 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-semantics.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-semantics.ts @@ -44,7 +44,9 @@ export const AltimateCoreSemanticsTool = Tool.define("altimate_core_semantics", return { title: hasError ? "Semantics: ERROR" : `Semantics: ${issueCount === 0 ? "VALID" : `${issueCount} issues`}`, metadata: { - success: true, // engine ran — semantic issues are findings, not failures + // Semantic issues are findings, not failures — but a failed engine + // call must not report success. + success: result.success, valid: data.valid, issue_count: issueCount, has_schema: hasSchema, diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index 82329aab5c..9e388fbaae 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -72,7 +72,8 @@ async function runValidate(sql: string, file: string, schemaPath?: string): Prom return errors.map((f) => { // Engine ValidationError: { code, kind, message, location: {line, column} | null, suggestions } const location = f.location as { line?: number; column?: number } | null | undefined - const suggestions = f.suggestions as Array> | undefined + const s0 = (f.suggestions as unknown[] | undefined)?.[0] + const engineSuggestion = typeof s0 === "string" ? s0 : ((s0 as Record | undefined)?.message as string | undefined) return { file, line: (location?.line ?? f.line) as number | undefined, @@ -81,7 +82,7 @@ async function runValidate(sql: string, file: string, schemaPath?: string): Prom rule: "validate", severity: "error" as const, message: (f.message ?? f.description ?? "") as string, - suggestion: (f.suggestion ?? suggestions?.[0]?.message) as string | undefined, + suggestion: (f.suggestion ?? engineSuggestion) as string | undefined, } }) } @@ -111,9 +112,10 @@ async function runSafety(sql: string, file: string): Promise { []) as Array> if (issues.length > 0) { return issues.map((f) => { - // ThreatFinding.location is [byteOffset, byteLength], not [start, end]. + // ThreatFinding.location is [byteOffset, byteLength] — label as bytes, + // since byte offsets diverge from character indexes on multibyte SQL. const loc = f.location as [number, number] | undefined - const at = Array.isArray(loc) ? ` (chars ${loc[0]}-${loc[0] + loc[1]})` : "" + const at = Array.isArray(loc) ? ` (bytes ${loc[0]}-${loc[0] + loc[1]})` : "" return { file, line: f.line as number | undefined, diff --git a/packages/opencode/test/altimate/altimate-core-e2e.test.ts b/packages/opencode/test/altimate/altimate-core-e2e.test.ts index 5c3c6cbab0..a01ca0417f 100644 --- a/packages/opencode/test/altimate/altimate-core-e2e.test.ts +++ b/packages/opencode/test/altimate/altimate-core-e2e.test.ts @@ -1242,15 +1242,25 @@ describeIf("consumer contract sync (round 2)", () => { }) describe("failure output consistency (title and body must agree)", () => { - test("classify-pii engine error renders Error output, not 'No PII columns detected'", async () => { + test("classify-pii engine error renders ERROR title and Error output", async () => { const { initTool } = await import("./tool-fixture") const { AltimateCoreClassifyPiiTool } = await import("../../src/altimate/tools/altimate-core-classify-pii") const tool = await initTool(AltimateCoreClassifyPiiTool) - // Nonexistent schema_path → engine failure envelope - const result = await tool.execute({ schema_path: "/nonexistent/schema.json" }, toolCtx()) - if (result.title === "PII Classification: ERROR") { + // Malformed schema file → deterministic engine failure envelope + // (a nonexistent path would silently fall back to an empty schema). + const fs = await import("fs/promises") + const os = await import("os") + const path = await import("path") + const badSchema = path.join(await fs.mkdtemp(path.join(os.tmpdir(), "pii-err-")), "schema.json") + await fs.writeFile(badSchema, "not json{{{") + try { + const result = await tool.execute({ schema_path: badSchema }, toolCtx()) + expect(result.title).toBe("PII Classification: ERROR") + expect(result.metadata.success).toBe(false) expect(result.output).toContain("Error") expect(result.output).not.toContain("No PII columns detected") + } finally { + await fs.rm(path.dirname(badSchema), { recursive: true, force: true }) } }) @@ -1279,8 +1289,21 @@ describeIf("consumer contract sync (round 2)", () => { ) expect(result.output).toContain("=== PII ===") expect(result.output).toContain("customers.email") + expect(result.output).toContain("exposed via: contact") expect(result.output).not.toContain("No PII detected") }) + + test("check tool PII section reports abstention for unparseable SQL, not a clean verdict", async () => { + const { formatCheck } = await import("../../src/altimate/tools/altimate-core-check") + const output = formatCheck({ + validation: { valid: false, errors: [{ message: "syntax" }] }, + lint: { clean: true }, + safety: { safe: true }, + pii: { accesses_pii: false, pii_columns: [], parse_error: "Syntax error: Expected: identifier" }, + }) + expect(output).toContain("PII check skipped") + expect(output).not.toContain("No PII detected") + }) }) describe("grade CLI mapping (engine shape: overall_grade/scores.overall)", () => { diff --git a/packages/opencode/test/cli/check-e2e.test.ts b/packages/opencode/test/cli/check-e2e.test.ts index 0961d83e1f..e16c76ef7f 100644 --- a/packages/opencode/test/cli/check-e2e.test.ts +++ b/packages/opencode/test/cli/check-e2e.test.ts @@ -624,8 +624,8 @@ describe("check command E2E", () => { const j = parseJson(r.stdout) expect(j.results.safety.findings).toHaveLength(1) expect(j.results.safety.findings[0].rule).toBe("unbalanced_quote") - // ThreatFinding.location is [byteOffset, byteLength] — rendered as an offset range. - expect(j.results.safety.findings[0].message).toBe("Unbalanced quote suggests injection breakout (chars 37-44)") + // ThreatFinding.location is [byteOffset, byteLength] — rendered as a byte range. + expect(j.results.safety.findings[0].message).toBe("Unbalanced quote suggests injection breakout (bytes 37-44)") expect(j.results.safety.findings[0].suggestion).toBe("Quote count is odd within a single statement") // Engine severity "high" must normalize to error, not degrade to info — // otherwise --fail-on/--severity filters silently pass high-risk injections. From a00fb3866811dd88c2bf80214d64d0b2ccb4045f Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 12 Aug 2026 19:27:06 +0530 Subject: [PATCH 05/14] =?UTF-8?q?fix:=20cubic=20round-3=20polish=20?= =?UTF-8?q?=E2=80=94=20no=20eager=20NAPI=20load=20from=20tool=20imports?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move `piiColumnsFromReport` into `native/engine-coerce.ts` (re-exported from `pii-detector` for compatibility) so `altimate-core-classify-pii` no longer pulls the native NAPI binding eagerly at tool-registry load time. - `bandConfidence`: missing confidence maps to "medium" (unknown), restoring the previous default instead of degrading to "low". - classify-pii error test: temp dir created outside `try` but cleaned in `finally` regardless of setup failure. Co-Authored-By: Claude Fable 5 --- .../src/altimate/native/engine-coerce.ts | 15 +++++++++++++++ .../src/altimate/native/schema/pii-detector.ts | 17 +++-------------- .../tools/altimate-core-classify-pii.ts | 3 +-- .../test/altimate/altimate-core-e2e.test.ts | 7 ++++--- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/packages/opencode/src/altimate/native/engine-coerce.ts b/packages/opencode/src/altimate/native/engine-coerce.ts index 7090b882c8..7bd8d33eae 100644 --- a/packages/opencode/src/altimate/native/engine-coerce.ts +++ b/packages/opencode/src/altimate/native/engine-coerce.ts @@ -24,6 +24,8 @@ export function dialectHint(dialect: string | undefined | null): string | undefi /** Map the engine's numeric confidence (0..1) to a string band. */ export function bandConfidence(c: unknown): "high" | "medium" | "low" { + // Missing confidence is unknown, not low. + if (c == null) return "medium" if (typeof c === "string") { const s = c.toLowerCase() if (s === "high" || s === "medium" || s === "low") return s @@ -33,3 +35,16 @@ export function bandConfidence(c: unknown): "high" | "medium" | "low" { if (n >= 0.5) return "medium" return "low" } + +/** + * Extract the real PII rows from an engine PiiReport. + * + * The engine returns `{ columns, pii_count, risk_level, total_columns }` with + * a row for EVERY column — classification "None" means not PII. Lives here + * (not in pii-detector) so tool modules can import it without eagerly loading + * the native NAPI binding at registry-load time. + */ +export function piiColumnsFromReport(piiData: unknown): Array> { + const columns = ((piiData as Record)?.columns ?? []) as Array> + return columns.filter((c) => c.classification !== "None") +} diff --git a/packages/opencode/src/altimate/native/schema/pii-detector.ts b/packages/opencode/src/altimate/native/schema/pii-detector.ts index 79ab15bbe2..16b4e80ec8 100644 --- a/packages/opencode/src/altimate/native/schema/pii-detector.ts +++ b/packages/opencode/src/altimate/native/schema/pii-detector.ts @@ -4,8 +4,10 @@ */ import * as core from "@altimateai/altimate-core" -import { bandConfidence, classificationToString } from "../engine-coerce" +import { bandConfidence, classificationToString, piiColumnsFromReport } from "../engine-coerce" import { getCache } from "./cache" + +export { piiColumnsFromReport } import * as Registry from "../connections/registry" import type { PiiDetectParams, @@ -13,19 +15,6 @@ import type { PiiFinding, } from "../types" -/** - * Extract the real PII rows from an engine PiiReport. - * - * The engine returns `{ columns, pii_count, risk_level, total_columns }` with - * a row for EVERY column — classification "None" means not PII. (The previous - * consumer read a nonexistent `findings` field, making detection a silent - * no-op.) Exported for tests. - */ -export function piiColumnsFromReport(piiData: unknown): Array> { - const columns = ((piiData as Record)?.columns ?? []) as Array> - return columns.filter((c) => c.classification !== "None") -} - /** * Detect PII in cached schema metadata by running altimate-core's * classifyPii() on column names and types. diff --git a/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts b/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts index bedc1a31d2..de02516e10 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts @@ -1,8 +1,7 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" -import { classificationToString } from "../native/engine-coerce" -import { piiColumnsFromReport } from "../native/schema/pii-detector" +import { classificationToString, piiColumnsFromReport } from "../native/engine-coerce" /** Engine PiiReport lists EVERY column; classification "None" means not PII. */ function realPiiColumns(data: Record): any[] { diff --git a/packages/opencode/test/altimate/altimate-core-e2e.test.ts b/packages/opencode/test/altimate/altimate-core-e2e.test.ts index a01ca0417f..ccbae134a2 100644 --- a/packages/opencode/test/altimate/altimate-core-e2e.test.ts +++ b/packages/opencode/test/altimate/altimate-core-e2e.test.ts @@ -1251,16 +1251,17 @@ describeIf("consumer contract sync (round 2)", () => { const fs = await import("fs/promises") const os = await import("os") const path = await import("path") - const badSchema = path.join(await fs.mkdtemp(path.join(os.tmpdir(), "pii-err-")), "schema.json") - await fs.writeFile(badSchema, "not json{{{") + const dir = await fs.mkdtemp(path.join(os.tmpdir(), "pii-err-")) try { + const badSchema = path.join(dir, "schema.json") + await fs.writeFile(badSchema, "not json{{{") const result = await tool.execute({ schema_path: badSchema }, toolCtx()) expect(result.title).toBe("PII Classification: ERROR") expect(result.metadata.success).toBe(false) expect(result.output).toContain("Error") expect(result.output).not.toContain("No PII columns detected") } finally { - await fs.rm(path.dirname(badSchema), { recursive: true, force: true }) + await fs.rm(dir, { recursive: true, force: true }) } }) From 960229d4ac75aab1ff7a0a83c1a752c861589628 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 12 Aug 2026 20:43:55 +0530 Subject: [PATCH 06/14] fix: address round-4 review comments (Codex connector, CodeRabbit, Kilo) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Abstention and fail-closed hardening: - `check --checks pii`: engine `parse_error` abstentions now produce an error finding ("PII analysis skipped: …") instead of an empty pass — `--fail-on` no longer PASSes files whose PII analysis never ran. - Composite `altimate_core.check`: a thrown `checkQueryPii` now marks the PII section as an abstention (`parse_error`) instead of leaving `{}`, which rendered a false-clean "No PII detected"; `formatCheckTitle` reports "PII check skipped" instead of PASS for abstained-but-clean runs. - `schema.detect_pii`: per-column classify failures are counted and flip `success` to false (fail closed) instead of being silently swallowed; `piiColumnsFromReport` now throws on malformed reports (missing/non-array `columns`) rather than yielding zero findings. - `altimate_core_query_pii` tool: abstentions set `metadata.success: false` so telemetry's soft-failure classification records them. Correctness: - `check --checks grade`: per-file grades (`results.grade.grades`) — the shared `gradeValue`/`gradeScore` raced across concurrent batch promises, keeping whichever file finished last; flat `grade`/`score` retained for single-file runs only. Module shape / dedup: - `engine-coerce.ts` gets the AGENTS.md-prescribed `EngineCoerce` self-reexport; all consumers import the namespace projection. - `review/runner.ts` drops its local `bandConfidence` for the shared one; shared version now bands missing/non-numeric confidence as "medium" (matching the runner's previous behavior). Tests: CLI PII-abstention and multi-file grade regression tests; "PII check skipped" title test; strict malformed-PiiReport assertions; classify-pii e2e asserts a positive `pii_count` lower bound; DuckDB e2e closes its connector and restores the telemetry env var on teardown. Co-Authored-By: Claude Fable 5 --- .../src/altimate/native/altimate-core.ts | 24 +++++----- .../src/altimate/native/engine-coerce.ts | 32 +++++++++----- .../altimate/native/schema/pii-detector.ts | 33 ++++++++------ .../opencode/src/altimate/review/runner.ts | 16 ++----- .../src/altimate/tools/altimate-core-check.ts | 7 +-- .../tools/altimate-core-classify-pii.ts | 6 +-- .../altimate/tools/altimate-core-query-pii.ts | 8 ++-- packages/opencode/src/cli/cmd/check.ts | 26 ++++++++--- .../altimate-core-check-formatters.test.ts | 12 +++++ .../test/altimate/altimate-core-e2e.test.ts | 3 ++ .../test/altimate/pii-detector-e2e.test.ts | 26 ++++++++--- packages/opencode/test/cli/check-e2e.test.ts | 44 +++++++++++++++++++ 12 files changed, 169 insertions(+), 68 deletions(-) diff --git a/packages/opencode/src/altimate/native/altimate-core.ts b/packages/opencode/src/altimate/native/altimate-core.ts index 780ca52b94..92a42e54ed 100644 --- a/packages/opencode/src/altimate/native/altimate-core.ts +++ b/packages/opencode/src/altimate/native/altimate-core.ts @@ -9,7 +9,7 @@ */ import * as core from "@altimateai/altimate-core" -import { dialectHint } from "./engine-coerce" +import { EngineCoerce } from "./engine-coerce" import { register } from "./dispatcher" import { schemaOrEmpty, resolveSchema } from "./schema-resolver" import type { AltimateCoreResult } from "./types" @@ -185,11 +185,13 @@ export function registerAll(): void { // PII exposure for the composite check — the tool has always rendered a // PII section; previously nothing populated it. Additive: a PII failure // must not fail the whole composite. - let pii: Record = {} + let pii: Record try { pii = toData(core.checkQueryPii(params.sql, schema)) - } catch { - // validation/lint above already report unparseable SQL + } catch (e) { + // Mark as an abstention — an empty object would render "No PII + // detected", a false-clean verdict. + pii = { parse_error: String(e) } } const data: Record = { validation: toData(validation), @@ -337,7 +339,7 @@ export function registerAll(): void { // altimate-core@0.5.1. dialectHint coerces "" (the ReviewConfig default) // to undefined: the engine throws on an unknown dialect "", and "" must // mean auto-detect, not a real dialect. - const raw = await core.checkEquivalence(params.sql1, params.sql2, schema, dialectHint(params.dialect)) + const raw = await core.checkEquivalence(params.sql1, params.sql2, schema, EngineCoerce.dialectHint(params.dialect)) const data = toData(raw) return ok(true, data) } catch (e) { @@ -350,7 +352,7 @@ export function registerAll(): void { try { // Build schema from old_ddl, analyze new_ddl against it. dialectHint // coerces "" to auto-detect (the engine throws on an unknown dialect ""). - const schema = core.Schema.fromDdl(params.old_ddl, dialectHint(params.dialect)) + const schema = core.Schema.fromDdl(params.old_ddl, EngineCoerce.dialectHint(params.dialect)) const raw = core.analyzeMigration(params.new_ddl, schema) const data = toData(raw) return ok(true, data) @@ -444,7 +446,7 @@ export function registerAll(): void { register("altimate_core.column_lineage", async (params) => { try { const schema = resolveSchema(params.schema_path, params.schema_context) - const raw = core.columnLineage(params.sql, dialectHint(params.dialect), schema ?? undefined) + const raw = core.columnLineage(params.sql, EngineCoerce.dialectHint(params.dialect), schema ?? undefined) return ok(true, toData(raw)) } catch (e) { return fail(e) @@ -465,7 +467,7 @@ export function registerAll(): void { // 22. altimate_core.format register("altimate_core.format", async (params) => { try { - const raw = core.formatSql(params.sql, dialectHint(params.dialect)) + const raw = core.formatSql(params.sql, EngineCoerce.dialectHint(params.dialect)) const data = toData(raw) return ok(true, data) } catch (e) { @@ -476,7 +478,7 @@ export function registerAll(): void { // 23. altimate_core.metadata register("altimate_core.metadata", async (params) => { try { - const raw = core.extractMetadata(params.sql, dialectHint(params.dialect)) + const raw = core.extractMetadata(params.sql, EngineCoerce.dialectHint(params.dialect)) return ok(true, toData(raw)) } catch (e) { return fail(e) @@ -486,7 +488,7 @@ export function registerAll(): void { // 24. altimate_core.compare register("altimate_core.compare", async (params) => { try { - const raw = core.compareQueries(params.left_sql, params.right_sql, dialectHint(params.dialect)) + const raw = core.compareQueries(params.left_sql, params.right_sql, EngineCoerce.dialectHint(params.dialect)) return ok(true, toData(raw)) } catch (e) { return fail(e) @@ -540,7 +542,7 @@ export function registerAll(): void { // 29. altimate_core.import_ddl — returns Schema, must serialize register("altimate_core.import_ddl", async (params) => { try { - const schema = core.importDdl(params.ddl, dialectHint(params.dialect)) + const schema = core.importDdl(params.ddl, EngineCoerce.dialectHint(params.dialect)) const jsonObj = schema.toJson() return ok(true, { success: true, schema: toData(jsonObj) }) } catch (e) { diff --git a/packages/opencode/src/altimate/native/engine-coerce.ts b/packages/opencode/src/altimate/native/engine-coerce.ts index 7bd8d33eae..ec9de5e31b 100644 --- a/packages/opencode/src/altimate/native/engine-coerce.ts +++ b/packages/opencode/src/altimate/native/engine-coerce.ts @@ -1,11 +1,15 @@ /** * Shared coercions for altimate-core engine values. * - * The engine's napi surface has two recurring foot-guns for consumers: + * The engine's napi surface has recurring foot-guns for consumers: * - `PiiClassification` is a string OR `{ Custom: string }` — naive string * interpolation renders `[object Object]`. * - dialect parameters throw on the empty string (`unknown dialect ''`); * `""` must mean "auto-detect" and be passed as undefined. + * - confidence is numeric (0..1) while several consumers declare string bands. + * + * This module must stay free of `@altimateai/altimate-core` imports so tool + * modules can use it without eagerly loading the native NAPI binding. */ /** PiiClassification is 'Email' | … | { Custom: string } | 'None'. */ @@ -22,15 +26,16 @@ export function dialectHint(dialect: string | undefined | null): string | undefi return dialect || undefined } -/** Map the engine's numeric confidence (0..1) to a string band. */ +/** + * Map the engine's numeric confidence (0..1) to a string band. + * Missing or non-numeric confidence is unknown, not low — band it "medium". + */ export function bandConfidence(c: unknown): "high" | "medium" | "low" { - // Missing confidence is unknown, not low. - if (c == null) return "medium" if (typeof c === "string") { const s = c.toLowerCase() if (s === "high" || s === "medium" || s === "low") return s } - const n = typeof c === "number" ? c : NaN + const n = typeof c === "number" && Number.isFinite(c) ? c : 0.5 if (n >= 0.8) return "high" if (n >= 0.5) return "medium" return "low" @@ -40,11 +45,18 @@ export function bandConfidence(c: unknown): "high" | "medium" | "low" { * Extract the real PII rows from an engine PiiReport. * * The engine returns `{ columns, pii_count, risk_level, total_columns }` with - * a row for EVERY column — classification "None" means not PII. Lives here - * (not in pii-detector) so tool modules can import it without eagerly loading - * the native NAPI binding at registry-load time. + * a row for EVERY column — classification "None" means not PII. + * + * Fails closed: a report without an array `columns` field is malformed (the + * engine always emits one) and throws instead of silently yielding zero + * findings — silent-empty output is exactly the bug class this fixes. */ export function piiColumnsFromReport(piiData: unknown): Array> { - const columns = ((piiData as Record)?.columns ?? []) as Array> - return columns.filter((c) => c.classification !== "None") + const columns = (piiData as Record | null | undefined)?.columns + if (!Array.isArray(columns)) { + throw new TypeError("malformed PiiReport: missing columns array") + } + return columns.filter((c) => c && c.classification !== "None") } + +export * as EngineCoerce from "./engine-coerce" diff --git a/packages/opencode/src/altimate/native/schema/pii-detector.ts b/packages/opencode/src/altimate/native/schema/pii-detector.ts index 16b4e80ec8..cd311a714d 100644 --- a/packages/opencode/src/altimate/native/schema/pii-detector.ts +++ b/packages/opencode/src/altimate/native/schema/pii-detector.ts @@ -4,10 +4,8 @@ */ import * as core from "@altimateai/altimate-core" -import { bandConfidence, classificationToString, piiColumnsFromReport } from "../engine-coerce" +import { EngineCoerce } from "../engine-coerce" import { getCache } from "./cache" - -export { piiColumnsFromReport } import * as Registry from "../connections/registry" import type { PiiDetectParams, @@ -15,6 +13,9 @@ import type { PiiFinding, } from "../types" +/** Re-exported for tests and legacy importers; lives in engine-coerce. */ +export const piiColumnsFromReport = EngineCoerce.piiColumnsFromReport + /** * Detect PII in cached schema metadata by running altimate-core's * classifyPii() on column names and types. @@ -46,6 +47,7 @@ export async function detectPii(params: PiiDetectParams): Promise() for (const wh of targetWarehouses) { @@ -77,7 +79,7 @@ export async function detectPii(params: PiiDetectParams): Promise const findings: PiiFinding[] = [] let columnsScanned = 0 + let scanErrors = 0 const tablesWithPii = new Set() for (const schemaName of schemas) { @@ -170,7 +175,7 @@ async function detectPiiLive(params: PiiDetectParams): Promise const piiData = JSON.parse(JSON.stringify(result)) // Engine PiiReport: { columns, … } with a row per column; "None" = not PII. - const piiColumns = piiColumnsFromReport(piiData) + const piiColumns = EngineCoerce.piiColumnsFromReport(piiData) for (const piiCol of piiColumns) { findings.push({ warehouse: params.warehouse!, @@ -178,13 +183,15 @@ async function detectPiiLive(params: PiiDetectParams): Promise table: tableInfo.name, column: piiCol.column || "", data_type: columns.find((c) => c.name === piiCol.column)?.data_type, - pii_category: classificationToString(piiCol.classification, "UNKNOWN"), - confidence: bandConfidence(piiCol.confidence), + pii_category: EngineCoerce.classificationToString(piiCol.classification, "UNKNOWN"), + confidence: EngineCoerce.bandConfidence(piiCol.confidence), }) tablesWithPii.add(`${params.warehouse}.${schemaName}.${tableInfo.name}`) } } catch { - // ignore + // classifyPii threw or returned a malformed report — record it so + // the scan fails closed instead of silently reporting fewer findings. + scanErrors++ } } } @@ -195,7 +202,7 @@ async function detectPiiLive(params: PiiDetectParams): Promise } return { - success: true, + success: scanErrors === 0, findings, finding_count: findings.length, columns_scanned: columnsScanned, diff --git a/packages/opencode/src/altimate/review/runner.ts b/packages/opencode/src/altimate/review/runner.ts index 1838e8793f..0f6cf53c9c 100644 --- a/packages/opencode/src/altimate/review/runner.ts +++ b/packages/opencode/src/altimate/review/runner.ts @@ -1,5 +1,5 @@ import { Dispatcher } from "../native" -import { classificationToString } from "../native/engine-coerce" +import { EngineCoerce } from "../native/engine-coerce" import { parseManifest } from "../native/dbt/manifest" import type { CheckResult, EquivalenceResult, GradeResult, ImpactResult, ReviewRunner } from "./orchestrate" import { buildReviewSchemaContext, type SchemaContext } from "./schema-context" @@ -58,16 +58,6 @@ function piiColumnOf(issue: any): string | undefined { return issue?.column ?? issue?.target ?? issue?.name ?? undefined } -/** The engine returns a numeric confidence (0..1); map it to a band. */ -function bandConfidence(c: unknown): "high" | "medium" | "low" { - if (typeof c === "string") { - const s = c.toLowerCase() - if (s === "high" || s === "medium" || s === "low") return s - } - const n = typeof c === "number" ? c : 0.5 - return n >= 0.8 ? "high" : n >= 0.5 ? "medium" : "low" -} - /** * Map a core lint finding (by rule name / L0xx code) to a review category. * Core's `LintFinding` carries no category, so without this every AST finding @@ -404,7 +394,7 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun .map((c) => ({ column: String(c.column ?? ""), // classification can be { Custom: string } — String() would emit "[object Object]" - classification: classificationToString(c.classification, ""), + classification: EngineCoerce.classificationToString(c.classification, ""), confidence: typeof c.confidence === "number" ? c.confidence : 0, masking: c.suggested_masking ?? undefined, })) @@ -512,7 +502,7 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun decided: true, equivalent, differences: diffs.map((d) => d?.description ?? String(d)), - confidence: bandConfidence(data.confidence), + confidence: EngineCoerce.bandConfidence(data.confidence), } } catch { return { decided: false } diff --git a/packages/opencode/src/altimate/tools/altimate-core-check.ts b/packages/opencode/src/altimate/tools/altimate-core-check.ts index 6f431e1125..1b9e67c279 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-check.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-check.ts @@ -1,7 +1,7 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" -import { classificationToString } from "../native/engine-coerce" +import { EngineCoerce } from "../native/engine-coerce" import type { Telemetry } from "../telemetry" export const AltimateCoreCheckTool = Tool.define("altimate_core_check", { @@ -63,7 +63,8 @@ export function formatCheckTitle(data: Record): string { if (!data.validation?.valid) parts.push("validation errors") if (!data.lint?.clean) parts.push(`${data.lint?.findings?.length ?? 0} lint findings`) if (!data.safety?.safe) parts.push("safety threats") - if (data.pii?.pii_columns?.length || data.pii?.findings?.length) parts.push("PII detected") + if (data.pii?.parse_error) parts.push("PII check skipped") + else if (data.pii?.pii_columns?.length || data.pii?.findings?.length) parts.push("PII detected") return parts.length ? parts.join(", ") : "PASS" } @@ -108,7 +109,7 @@ export function formatCheck(data: Record): string { lines.push("No PII detected.") } else { for (const p of piiCols) { - const cls = classificationToString(p.classification ?? p.category) + const cls = EngineCoerce.classificationToString(p.classification ?? p.category) const where = [p.table, p.column ?? "unknown"].filter(Boolean).join(".") const via = Array.isArray(p.query_targets) && p.query_targets.length ? ` exposed via: ${p.query_targets.join(", ")}` : "" lines.push(` ${where}: ${cls}${via}${p.suggested_masking ? ` (masking: ${p.suggested_masking})` : ""}`) diff --git a/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts b/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts index de02516e10..5225f5bf72 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-classify-pii.ts @@ -1,11 +1,11 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" -import { classificationToString, piiColumnsFromReport } from "../native/engine-coerce" +import { EngineCoerce } from "../native/engine-coerce" /** Engine PiiReport lists EVERY column; classification "None" means not PII. */ function realPiiColumns(data: Record): any[] { - if (Array.isArray(data.columns)) return piiColumnsFromReport(data) + if (Array.isArray(data.columns)) return EngineCoerce.piiColumnsFromReport(data) // Legacy fallback shape. return ((data.findings ?? []) as any[]).filter((c) => c.classification !== "None") } @@ -54,7 +54,7 @@ function formatClassifyPii(data: Record): string { lines.push("") lines.push("PII columns found:") for (const f of piiColumns) { - const classification = classificationToString(f.classification ?? f.category) + const classification = EngineCoerce.classificationToString(f.classification ?? f.category) const confidence = f.confidence ?? "high" const table = f.table ?? "unknown" const column = f.column ?? "unknown" diff --git a/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts b/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts index 8eb9a7eae8..bf54be3553 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-query-pii.ts @@ -1,7 +1,7 @@ import z from "zod" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" -import { classificationToString } from "../native/engine-coerce" +import { EngineCoerce } from "../native/engine-coerce" export const AltimateCoreQueryPiiTool = Tool.define("altimate_core_query_pii", { description: @@ -30,7 +30,9 @@ export const AltimateCoreQueryPiiTool = Tool.define("altimate_core_query_pii", { : `Query PII: ${exposureCount === 0 ? "CLEAN" : `${exposureCount} exposure(s)`}` return { title, - metadata: { success: result.success, exposure_count: exposureCount, ...(error && { error }) }, + // An abstention (parse_error) is a soft failure — telemetry classifies + // on metadata.success === false, so it must not report success. + metadata: { success: result.success && !error, exposure_count: exposureCount, ...(error && { error }) }, output: error ? `Error: ${error}` : formatQueryPii(data), } } catch (e) { @@ -52,7 +54,7 @@ function formatQueryPii(data: Record): string { if (data.risk_level) lines.push(`Risk level: ${data.risk_level}`) lines.push("PII exposure detected:\n") for (const e of piiCols) { - const classification = classificationToString(e.classification ?? e.category) + const classification = EngineCoerce.classificationToString(e.classification ?? e.category) const table = e.table ?? "unknown" const column = e.column ?? "unknown" lines.push(` ${table}.${column}: ${classification}`) diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index 9e388fbaae..d1f1d2a806 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -192,6 +192,12 @@ async function runPii(sql: string, file: string, schemaPath?: string): Promise = {} - let gradeValue: string | undefined - let gradeScore: number | undefined + // Per-file grades — concurrent batch promises must not race on a single + // shared grade (multi-file runs used to keep whichever file finished last). + const gradesByFile: Record = {} for (const check of checks) { allResults[check] = [] } @@ -486,8 +493,9 @@ export const CheckCommand = cmd({ case "grade": { const gradeResult = await runGrade(sql, relFile, schemaPath) findings = gradeResult.findings - if (gradeResult.grade) gradeValue = gradeResult.grade - if (gradeResult.score != null) gradeScore = gradeResult.score + if (gradeResult.grade || gradeResult.score != null) { + gradesByFile[relFile] = { grade: gradeResult.grade, score: gradeResult.score } + } break } } @@ -516,8 +524,14 @@ export const CheckCommand = cmd({ // 8. Attach grade metadata if available if (results.grade) { - if (gradeValue) results.grade.grade = gradeValue - if (gradeScore != null) results.grade.score = gradeScore + const graded = Object.entries(gradesByFile) + if (graded.length) results.grade.grades = gradesByFile + // Keep the flat grade/score fields for the common single-file invocation. + if (graded.length === 1) { + const [, only] = graded[0] + if (only.grade) results.grade.grade = only.grade + if (only.score != null) results.grade.score = only.score + } } // 9. Build output using the helper diff --git a/packages/opencode/test/altimate/altimate-core-check-formatters.test.ts b/packages/opencode/test/altimate/altimate-core-check-formatters.test.ts index 264d20cc0e..2451ee0b83 100644 --- a/packages/opencode/test/altimate/altimate-core-check-formatters.test.ts +++ b/packages/opencode/test/altimate/altimate-core-check-formatters.test.ts @@ -26,6 +26,18 @@ describe("formatCheckTitle", () => { expect(result).toContain("PII detected") }) + test("reports 'PII check skipped' (not PASS) when the PII check abstained", () => { + const data = { + validation: { valid: true }, + lint: { clean: true, findings: [] }, + safety: { safe: true }, + pii: { accesses_pii: false, pii_columns: [], parse_error: "Syntax error" }, + } + const result = formatCheckTitle(data) + expect(result).toContain("PII check skipped") + expect(result).not.toBe("PASS") + }) + test("treats missing sections as failures (undefined is falsy)", () => { // When data is empty, !undefined is true, so each section looks like a failure const data = {} as Record diff --git a/packages/opencode/test/altimate/altimate-core-e2e.test.ts b/packages/opencode/test/altimate/altimate-core-e2e.test.ts index ccbae134a2..68efd1ffd9 100644 --- a/packages/opencode/test/altimate/altimate-core-e2e.test.ts +++ b/packages/opencode/test/altimate/altimate-core-e2e.test.ts @@ -1187,6 +1187,9 @@ describeIf("consumer contract sync (round 2)", () => { const { Dispatcher } = await import("../../src/altimate/native") const raw = await Dispatcher.call("altimate_core.classify_pii", { schema_context: SCHEMA }) const piiCount = (raw.data as any).pii_count + // customers.email must classify as PII — a zero count would make this + // test pass without exercising the None-filter it guards. + expect(piiCount).toBeGreaterThan(0) expect(result.metadata.finding_count).toBe(piiCount) expect(result.output).not.toContain(": None") }) diff --git a/packages/opencode/test/altimate/pii-detector-e2e.test.ts b/packages/opencode/test/altimate/pii-detector-e2e.test.ts index 47ab033f50..574d9acf3a 100644 --- a/packages/opencode/test/altimate/pii-detector-e2e.test.ts +++ b/packages/opencode/test/altimate/pii-detector-e2e.test.ts @@ -33,10 +33,14 @@ describe("piiColumnsFromReport (real engine PiiReport shape)", () => { expect(pii.some((c) => c.classification === "None")).toBe(false) }) - test("returns empty for empty/malformed reports", () => { - expect(piiColumnsFromReport(undefined)).toEqual([]) - expect(piiColumnsFromReport({})).toEqual([]) - expect(piiColumnsFromReport({ findings: [{ category: "email" }] })).toEqual([]) + test("fails closed on malformed reports (missing/non-array columns)", () => { + // A PiiReport always carries a columns array — anything else is malformed + // and must throw rather than silently yield zero findings. + expect(() => piiColumnsFromReport(undefined)).toThrow("malformed PiiReport") + expect(() => piiColumnsFromReport({})).toThrow("malformed PiiReport") + expect(() => piiColumnsFromReport({ columns: "not-an-array" })).toThrow("malformed PiiReport") + expect(() => piiColumnsFromReport({ findings: [{ category: "email" }] })).toThrow("malformed PiiReport") + expect(piiColumnsFromReport({ columns: [] })).toEqual([]) }) }) @@ -47,12 +51,16 @@ describe("piiColumnsFromReport (real engine PiiReport shape)", () => { * findings for every scan (latent since the Python-engine elimination). */ describe("schema.detect_pii DuckDB e2e", () => { + let priorTelemetry: string | undefined + beforeAll(() => { + priorTelemetry = process.env.ALTIMATE_TELEMETRY_DISABLED process.env.ALTIMATE_TELEMETRY_DISABLED = "true" }) afterAll(() => { - delete process.env.ALTIMATE_TELEMETRY_DISABLED + if (priorTelemetry === undefined) delete process.env.ALTIMATE_TELEMETRY_DISABLED + else process.env.ALTIMATE_TELEMETRY_DISABLED = priorTelemetry Registry.reset() }) @@ -63,7 +71,13 @@ describe("schema.detect_pii DuckDB e2e", () => { await conn.execute("CREATE TABLE users (id INTEGER, email VARCHAR, note VARCHAR)") - const result = await detectPii({ warehouse: "duck_pii_e2e" }) + let result: Awaited> + try { + result = await detectPii({ warehouse: "duck_pii_e2e" }) + } finally { + // Registry.reset() clears the cache without closing connectors. + await conn.close() + } expect(result.success).toBe(true) expect(result.finding_count).toBeGreaterThan(0) diff --git a/packages/opencode/test/cli/check-e2e.test.ts b/packages/opencode/test/cli/check-e2e.test.ts index e16c76ef7f..230ef9e4c3 100644 --- a/packages/opencode/test/cli/check-e2e.test.ts +++ b/packages/opencode/test/cli/check-e2e.test.ts @@ -674,6 +674,50 @@ describe("check command E2E", () => { expect(j.results.pii.findings[1].suggestion).toBeUndefined() }) + test("pii check fails when the engine abstains via parse_error", async () => { + // Unparseable SQL: engine returns success + parse_error + empty pii_columns. + // No findings would let --fail-on PASS a file whose PII analysis never ran. + const file = await writeSql(tmpDir.dir, "pii-abstain.sql", "SELECT FROM;") + setDispatcherResponse("altimate_core.query_pii", () => ({ + success: true, + data: { accesses_pii: false, parse_error: "Syntax error: Expected: identifier", pii_columns: [] }, + })) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [file], checks: "pii" })) + const j = parseJson(r.stdout) + expect(j.results.pii.findings).toHaveLength(1) + expect(j.results.pii.findings[0].severity).toBe("error") + expect(j.results.pii.findings[0].message).toContain("PII analysis skipped") + }) + + test("grade check keeps per-file grades on multi-file runs", async () => { + const fileA = await writeSql(tmpDir.dir, "grade-a.sql", "SELECT 1;") + const fileB = await writeSql(tmpDir.dir, "grade-b.sql", "SELECT * FROM t;") + let call = 0 + setDispatcherResponse("altimate_core.grade", () => { + call++ + return { + success: true, + data: { + overall_grade: call === 1 ? "A" : "C", + scores: { overall: call === 1 ? 0.95 : 0.7 }, + lint: { clean: true, findings: [] }, + }, + } + }) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [fileA, fileB], checks: "grade" })) + const j = parseJson(r.stdout) + const grades = j.results.grade.grades as Record + expect(Object.keys(grades)).toHaveLength(2) + expect(new Set(Object.values(grades).map((g) => g.grade))).toEqual(new Set(["A", "C"])) + // Flat grade/score only meaningful for single-file runs — must not pick a + // racy winner across files. + expect(j.results.grade.grade).toBeUndefined() + }) + test("pii check reports PII columns", async () => { const file = await writeSql(tmpDir.dir, "pii.sql", "SELECT email, ssn FROM customers;") setDispatcherResponse("altimate_core.query_pii", () => ({ From 3974e35c93ae8ead61a39f3b701a5cff9faf5344 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 12 Aug 2026 21:49:42 +0530 Subject: [PATCH 07/14] =?UTF-8?q?fix:=20round-5=20review=20follow-through?= =?UTF-8?q?=20=E2=80=94=20failed=20scans=20and=20warnings=20must=20surface?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `schema_detect_pii` tool: honor `success: false` from `detectPii` — a scan that failed for any column now renders "PII Scan: ERROR" (with any partial findings attached) instead of a clean "no findings" verdict. The previous round made the detector fail closed but left its only user-facing consumer branching on `finding_count` alone. (cursor High / cubic P1 / Codex P2) - `check --checks grade`: flat `grade`/`score` gated on the invocation file count, not surviving-grade count — with several files where all but one grade call failed, the flat fields would have misattributed the survivor. (CodeRabbit Major) - `altimate_core_policy` tool: `allowed: true` with non-empty `warnings` now renders the warnings with the pass instead of hiding them behind the early "passes all policy checks" return. (Codex P2) - `review/runner.ts`: PII column extraction includes `query_targets` output aliases (e.g. `SELECT email AS contact` exposes `contact`), not just source columns. (Codex P2) - Tests: fail-closed detect-pii tool cases (zero and partial findings), policy warnings-on-pass case. Co-Authored-By: Claude Fable 5 --- .../opencode/src/altimate/review/runner.ts | 11 ++- .../altimate/tools/altimate-core-policy.ts | 11 ++- .../src/altimate/tools/schema-detect-pii.ts | 15 ++++ packages/opencode/src/cli/cmd/check.ts | 5 +- .../altimate/tool-error-propagation.test.ts | 77 +++++++++++++++++++ 5 files changed, 113 insertions(+), 6 deletions(-) diff --git a/packages/opencode/src/altimate/review/runner.ts b/packages/opencode/src/altimate/review/runner.ts index 0f6cf53c9c..010c62fbdb 100644 --- a/packages/opencode/src/altimate/review/runner.ts +++ b/packages/opencode/src/altimate/review/runner.ts @@ -219,12 +219,17 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun // `issues` drop those fields, so mapping over them would always miss. const piiColumns = [ // data.pii is the engine PiiQueryResult ({ pii_columns, … }); the - // legacy array shape is kept as a fallback. - ...asArray((data.pii as any)?.pii_columns ?? data.pii).map(piiColumnOf), + // legacy array shape is kept as a fallback. Each entry names the + // SOURCE column; query_targets carries the exposed OUTPUT aliases + // (e.g. `SELECT email AS contact` → column "email", targets ["contact"]). + ...asArray((data.pii as any)?.pii_columns ?? data.pii).flatMap((p: any) => [ + piiColumnOf(p), + ...(Array.isArray(p?.query_targets) ? p.query_targets : []), + ]), ...rawIssues .filter((i: any) => /pii|sensitive/i.test(String(i.category ?? i.rule ?? i.code ?? i.kind ?? ""))) .map(piiColumnOf), - ].filter((c): c is string => !!c) + ].filter((c): c is string => typeof c === "string" && !!c) // ran=true: the core parsed and analyzed the SQL (even if zero issues). // This lets the orchestrator defer structural checks to the AST lint. out = { issues, piiColumns: [...new Set(piiColumns)], ran: true } diff --git a/packages/opencode/src/altimate/tools/altimate-core-policy.ts b/packages/opencode/src/altimate/tools/altimate-core-policy.ts index 1b0eec0c98..f4eb97c760 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-policy.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-policy.ts @@ -58,7 +58,16 @@ export const AltimateCorePolicyTool = Tool.define("altimate_core_policy", { function formatPolicy(data: Record): string { if (data.error) return `Error: ${data.error}` - if (data.allowed ?? data.pass) return "SQL passes all policy checks." + if (data.allowed ?? data.pass) { + // allowed:true can still carry warnings — surface them with the pass. + const warnings = (data.warnings ?? []) as any[] + if (!warnings.length) return "SQL passes all policy checks." + const lines = ["SQL passes all policy checks, with warnings:\n"] + for (const w of warnings) { + lines.push(` [warning] ${w.rule ?? "policy"}: ${w.message ?? ""}`) + } + return lines.join("\n") + } const lines = ["Policy violations:\n"] for (const v of data.violations ?? []) { lines.push(` [${v.severity ?? "error"}] ${v.rule}: ${v.message}`) diff --git a/packages/opencode/src/altimate/tools/schema-detect-pii.ts b/packages/opencode/src/altimate/tools/schema-detect-pii.ts index f1134b859a..c4a13cba03 100644 --- a/packages/opencode/src/altimate/tools/schema-detect-pii.ts +++ b/packages/opencode/src/altimate/tools/schema-detect-pii.ts @@ -19,6 +19,21 @@ export const SchemaDetectPiiTool = Tool.define("schema_detect_pii", { table: args.table, }) + // Fail closed: detectPii reports success:false when any column scan + // failed — never render "no findings" for a scan that didn't complete. + if (result.success === false) { + return { + title: "PII Scan: ERROR", + metadata: { + success: false, + finding_count: result.finding_count, + columns_scanned: result.columns_scanned, + error: "PII scan failed for one or more columns — results are incomplete", + }, + output: `PII scan failed for one or more columns — results are incomplete (${result.finding_count} finding(s) before failure).${result.finding_count ? `\n\n${formatPii(result)}` : ""}`, + } + } + if (result.finding_count === 0) { return { title: `PII Scan: no findings (${result.columns_scanned} columns)`, diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index d1f1d2a806..fe023e12bc 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -526,8 +526,9 @@ export const CheckCommand = cmd({ if (results.grade) { const graded = Object.entries(gradesByFile) if (graded.length) results.grade.grades = gradesByFile - // Keep the flat grade/score fields for the common single-file invocation. - if (graded.length === 1) { + // Flat grade/score only for single-file INVOCATIONS — gating on how many + // grades survived would misattribute when one of several files errored. + if (files.length === 1 && graded.length === 1) { const [, only] = graded[0] if (only.grade) results.grade.grade = only.grade if (only.score != null) results.grade.score = only.score diff --git a/packages/opencode/test/altimate/tool-error-propagation.test.ts b/packages/opencode/test/altimate/tool-error-propagation.test.ts index 31ec98a400..0040c2639a 100644 --- a/packages/opencode/test/altimate/tool-error-propagation.test.ts +++ b/packages/opencode/test/altimate/tool-error-propagation.test.ts @@ -498,3 +498,80 @@ describe("extractors handle empty message fields", () => { } }) }) + +// --------------------------------------------------------------------------- +// schema_detect_pii — fail-closed on incomplete scans (round-4 review) +// --------------------------------------------------------------------------- +describe("schema_detect_pii fail-closed", () => { + beforeEach(() => Dispatcher.reset()) + + test("success:false scan renders ERROR, never 'no findings'", async () => { + // detectPii reports success:false when any column classification failed. + // The tool previously branched only on finding_count, so a failed scan + // with zero findings rendered as a clean "no findings" verdict. + Dispatcher.register("schema.detect_pii" as any, async () => ({ + success: false, + findings: [], + finding_count: 0, + columns_scanned: 42, + by_category: {}, + tables_with_pii: 0, + })) + + const { SchemaDetectPiiTool } = await import("../../src/altimate/tools/schema-detect-pii") + const tool = await initTool(SchemaDetectPiiTool) + const result = await tool.execute({}, stubCtx()) + + expect(result.title).toBe("PII Scan: ERROR") + expect(result.metadata.success).toBe(false) + expect(String(result.output)).toContain("incomplete") + expect(String(result.output)).not.toContain("No PII detected") + }) + + test("partial findings from a failed scan are still shown alongside the error", async () => { + Dispatcher.register("schema.detect_pii" as any, async () => ({ + success: false, + findings: [ + { warehouse: "wh", schema: "s", table: "users", column: "email", pii_category: "Email", confidence: "high" }, + ], + finding_count: 1, + columns_scanned: 42, + by_category: { Email: 1 }, + tables_with_pii: 1, + })) + + const { SchemaDetectPiiTool } = await import("../../src/altimate/tools/schema-detect-pii") + const tool = await initTool(SchemaDetectPiiTool) + const result = await tool.execute({}, stubCtx()) + + expect(result.title).toBe("PII Scan: ERROR") + expect(String(result.output)).toContain("wh.s.users.email") + }) +}) + +// --------------------------------------------------------------------------- +// altimate_core_policy — warnings surface with a pass (round-4 review) +// --------------------------------------------------------------------------- +describe("altimate_core_policy warnings on pass", () => { + beforeEach(() => Dispatcher.reset()) + + test("allowed:true with warnings renders the warnings, not a bare pass", async () => { + Dispatcher.register("altimate_core.policy" as any, async () => ({ + success: true, + data: { + allowed: true, + violations: [], + warnings: [{ rule: "row_estimate", category: "cost_control", message: "Query may scan a large table" }], + policies_evaluated: 2, + }, + })) + + const { AltimateCorePolicyTool } = await import("../../src/altimate/tools/altimate-core-policy") + const tool = await initTool(AltimateCorePolicyTool) + const result = await tool.execute({ sql: "SELECT 1", policy_json: "{}" }, stubCtx()) + + expect(result.title).toBe("Policy: PASS") + expect(String(result.output)).toContain("row_estimate") + expect(String(result.output)).toContain("Query may scan a large table") + }) +}) From 846cf24bc8a14425e629c0b1589c2390eabe0094 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 12 Aug 2026 22:08:13 +0530 Subject: [PATCH 08/14] =?UTF-8?q?fix:=20round-6=20review=20polish=20?= =?UTF-8?q?=E2=80=94=20alias=20preference=20+=20grade=20nested=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `review/runner.ts`: when a PII column has `query_targets`, report ONLY the output aliases — the source column name is not present in the model output, so including it flagged a column that isn't there (`SELECT email AS contact` now reports `contact`, not `email` + `contact`). - `check --checks grade`: findings now come from all nested EvalResult sections (lint.findings + validation.errors + safety.threats) — a failing grade with clean lint no longer yields an empty, passing finding list. - Regression test for the nested-findings case. Co-Authored-By: Claude Fable 5 --- .../opencode/src/altimate/review/runner.ts | 9 ++++--- packages/opencode/src/cli/cmd/check.ts | 20 +++++++++++++-- packages/opencode/test/cli/check-e2e.test.ts | 25 +++++++++++++++++++ 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/packages/opencode/src/altimate/review/runner.ts b/packages/opencode/src/altimate/review/runner.ts index 010c62fbdb..2c8eb45e3e 100644 --- a/packages/opencode/src/altimate/review/runner.ts +++ b/packages/opencode/src/altimate/review/runner.ts @@ -222,10 +222,11 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun // legacy array shape is kept as a fallback. Each entry names the // SOURCE column; query_targets carries the exposed OUTPUT aliases // (e.g. `SELECT email AS contact` → column "email", targets ["contact"]). - ...asArray((data.pii as any)?.pii_columns ?? data.pii).flatMap((p: any) => [ - piiColumnOf(p), - ...(Array.isArray(p?.query_targets) ? p.query_targets : []), - ]), + // Prefer the aliases when present — the source name is not in the + // model output, so reporting it would flag a column that isn't there. + ...asArray((data.pii as any)?.pii_columns ?? data.pii).flatMap((p: any) => + Array.isArray(p?.query_targets) && p.query_targets.length ? p.query_targets : [piiColumnOf(p)], + ), ...rawIssues .filter((i: any) => /pii|sensitive/i.test(String(i.category ?? i.rule ?? i.code ?? i.kind ?? ""))) .map(piiColumnOf), diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index fe023e12bc..499a7c83dd 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -289,12 +289,28 @@ async function runGrade( } // Engine EvalResult: { explain, lint, overall_grade, safety, scores, sql, // total_time_ms, validation } — the grade is `overall_grade`, the numeric - // score is `scores.overall`, and actionable findings live in `lint.findings`. + // score is `scores.overall`. Findings come from ALL nested sections: + // lint.findings, validation.errors, and safety.threats — a failing grade + // with clean lint must not produce an empty (passing) finding list. const lint = result.data.lint as Record | undefined + const validation = result.data.validation as Record | undefined + const safety = result.data.safety as Record | undefined + const nested = [ + ...((lint?.findings as Array> | undefined) ?? []), + ...(((validation?.errors as Array> | undefined) ?? []).map((e) => ({ + ...e, + rule: "validate", + severity: "error", + })) as Array>), + ...(((safety?.threats as Array> | undefined) ?? []).map((t) => ({ + ...t, + rule: (t.rule as string) ?? "safety", + })) as Array>), + ] const issues = (result.data.issues ?? result.data.findings ?? result.data.recommendations ?? - lint?.findings ?? + (nested.length ? nested : undefined) ?? []) as Array> const findings = issues.map((f) => ({ file, diff --git a/packages/opencode/test/cli/check-e2e.test.ts b/packages/opencode/test/cli/check-e2e.test.ts index 230ef9e4c3..dc4547b968 100644 --- a/packages/opencode/test/cli/check-e2e.test.ts +++ b/packages/opencode/test/cli/check-e2e.test.ts @@ -782,6 +782,31 @@ describe("check command E2E", () => { expect(j.results.grade.score).toBe(0.72) }) + test("grade check surfaces validation/safety findings when lint is clean", async () => { + // EvalResult can carry validation errors or safety threats with zero lint + // findings — the grade check must not present an empty (passing) list. + const file = await writeSql(tmpDir.dir, "grade-nested.sql", "SELECT zzz FROM t;") + setDispatcherResponse("altimate_core.grade", () => ({ + success: true, + data: { + overall_grade: "D", + scores: { overall: 0.4 }, + lint: { clean: true, findings: [] }, + validation: { valid: false, errors: [{ code: "E002", message: "Column 'zzz' not found" }] }, + safety: { safe: false, threats: [{ rule: "tautology_attack", severity: "high", message: "OR 1=1 detected" }] }, + }, + })) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [file], checks: "grade" })) + const j = parseJson(r.stdout) + expect(j.results.grade.findings.length).toBe(2) + const rules = j.results.grade.findings.map((f: any) => f.rule) + expect(rules).toContain("validate") + expect(rules).toContain("tautology_attack") + expect(j.results.grade.findings.some((f: any) => f.severity === "error")).toBe(true) + }) + test("grade check fails closed on engine failure envelope", async () => { // Native handlers report failures via {success:false}, not by throwing — // a failed grade run must not pass silently with zero findings. From 4358bb4d46ddbf1be25b27d78c6badd93ab7cab1 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 12 Aug 2026 22:42:43 +0530 Subject: [PATCH 09/14] fix: preserve locations/suggestions in nested grade findings `runGrade`'s nested `validation.errors`/`safety.threats` went through the shared flat mapper, dropping the line numbers and fixes that `runValidate`/ `runSafety` already surface. Normalize each shape first: `ValidationError` hoists `location.{line,column}` and the first `suggestions[]` entry; `ThreatFinding` renders its `[byteOffset, byteLength]` range into the message and maps `detail` to the suggestion. Regression assertions added. Co-Authored-By: Claude Fable 5 --- packages/opencode/src/cli/cmd/check.ts | 37 ++++++++++++++----- packages/opencode/test/cli/check-e2e.test.ts | 38 ++++++++++++++++---- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index 499a7c83dd..6915f5f93b 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -295,17 +295,36 @@ async function runGrade( const lint = result.data.lint as Record | undefined const validation = result.data.validation as Record | undefined const safety = result.data.safety as Record | undefined + // Normalize each section's location/suggestion shape to the flat fields + // the shared mapper below reads — otherwise nested findings drop the line + // numbers and fixes that runValidate/runSafety already surface. const nested = [ ...((lint?.findings as Array> | undefined) ?? []), - ...(((validation?.errors as Array> | undefined) ?? []).map((e) => ({ - ...e, - rule: "validate", - severity: "error", - })) as Array>), - ...(((safety?.threats as Array> | undefined) ?? []).map((t) => ({ - ...t, - rule: (t.rule as string) ?? "safety", - })) as Array>), + ...(((validation?.errors as Array> | undefined) ?? []).map((e) => { + // ValidationError: location {line, column} | null, suggestions[] + const location = e.location as { line?: number; column?: number } | null | undefined + const s0 = (e.suggestions as unknown[] | undefined)?.[0] + return { + ...e, + rule: "validate", + severity: "error", + line: location?.line ?? e.line, + column: location?.column ?? e.column, + suggestion: e.suggestion ?? (typeof s0 === "string" ? s0 : (s0 as Record | undefined)?.message), + } + }) as Array>), + ...(((safety?.threats as Array> | undefined) ?? []).map((t) => { + // ThreatFinding: location is [byteOffset, byteLength]; detail is the fix hint + const loc = t.location as [number, number] | undefined + const at = Array.isArray(loc) ? ` (bytes ${loc[0]}-${loc[0] + loc[1]})` : "" + return { + ...t, + rule: (t.rule as string) ?? "safety", + message: `${(t.message ?? "") as string}${at}`, + location: undefined, + suggestion: t.suggestion ?? t.detail, + } + }) as Array>), ] const issues = (result.data.issues ?? result.data.findings ?? diff --git a/packages/opencode/test/cli/check-e2e.test.ts b/packages/opencode/test/cli/check-e2e.test.ts index dc4547b968..f173ef81f0 100644 --- a/packages/opencode/test/cli/check-e2e.test.ts +++ b/packages/opencode/test/cli/check-e2e.test.ts @@ -792,8 +792,29 @@ describe("check command E2E", () => { overall_grade: "D", scores: { overall: 0.4 }, lint: { clean: true, findings: [] }, - validation: { valid: false, errors: [{ code: "E002", message: "Column 'zzz' not found" }] }, - safety: { safe: false, threats: [{ rule: "tautology_attack", severity: "high", message: "OR 1=1 detected" }] }, + validation: { + valid: false, + errors: [ + { + code: "E002", + message: "Column 'zzz' not found", + location: { line: 1, column: 8 }, + suggestions: [{ kind: "column", message: "Did you mean 'id'?", confidence: 0.9 }], + }, + ], + }, + safety: { + safe: false, + threats: [ + { + rule: "tautology_attack", + severity: "high", + message: "OR 1=1 detected", + detail: "Remove the always-true predicate", + location: [10, 7], + }, + ], + }, }, })) installDispatcherMocks() @@ -801,10 +822,15 @@ describe("check command E2E", () => { const r = await runHandler(baseArgs({ files: [file], checks: "grade" })) const j = parseJson(r.stdout) expect(j.results.grade.findings.length).toBe(2) - const rules = j.results.grade.findings.map((f: any) => f.rule) - expect(rules).toContain("validate") - expect(rules).toContain("tautology_attack") - expect(j.results.grade.findings.some((f: any) => f.severity === "error")).toBe(true) + const byRule = Object.fromEntries(j.results.grade.findings.map((f: any) => [f.rule, f])) + // ValidationError location/suggestions must survive the flat mapper. + expect(byRule.validate.severity).toBe("error") + expect(byRule.validate.line).toBe(1) + expect(byRule.validate.column).toBe(8) + expect(byRule.validate.suggestion).toBe("Did you mean 'id'?") + // ThreatFinding byte-range location and detail must survive too. + expect(byRule.tautology_attack.message).toBe("OR 1=1 detected (bytes 10-17)") + expect(byRule.tautology_attack.suggestion).toBe("Remove the always-true predicate") }) test("grade check fails closed on engine failure envelope", async () => { From 494407e9adefbe9198707fe92f26ad1b448c03ee Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Thu, 13 Aug 2026 17:44:19 +0530 Subject: [PATCH 10/14] fix: address post-approval review follow-ups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All items from the approving human review, except where noted in the PR reply: Introduced by this PR: - `classificationToString` default fallback unified to "UNKNOWN" across all call sites (was "PII"/"UNKNOWN"/"" depending on caller — an unrecognized classification no longer renders as a positive PII assertion). - `pii-detector-e2e.test.ts` gets the same NAPI-availability guard as `altimate-core-e2e.test.ts` (skip, not crash, when the binary is absent). - The `unbalanced_quote` check-e2e fixture SQL is now engine-faithful (a dangling quote, which the live 0.7.0 runtime genuinely flags with that rule — only the stale `SafetyRule` union omits it; upstream issue #764). Adjacent pre-existing: - `check --checks policy`: violations map `remediation` into the suggestion. - `sql.format` / `sql.column_lineage` handlers: last two raw dialect forwards coerced via `EngineCoerce.dialectHint`. - Review runner: composite `validation.errors` and `safety.threats` now flow into review issues alongside lint findings. - `runSafety` dispatcher-failure fallback fails closed (error severity via `dispatcherErrorFinding`, matching every other check). - Runner grade reads `overall_grade` before the legacy `grade`. Nits: - `runSemantic`'s valid:false fallback surfaces `validation_errors` detail. - Dead `line`/`column`/`code` reads dropped from the threats mapper. - Byte ranges render the INCLUSIVE end (`[37,7]` → `bytes 37-43`) in both places; tests updated. - `normalizeSeverity` documents the intentional low→info mapping. - PII findings carry a machine-readable `columnName` field. Co-Authored-By: Claude Fable 5 --- .../src/altimate/native/engine-coerce.ts | 2 +- .../src/altimate/native/sql/register.ts | 5 +-- .../opencode/src/altimate/review/runner.ts | 6 ++-- .../opencode/src/cli/cmd/check-helpers.ts | 4 +++ packages/opencode/src/cli/cmd/check.ts | 33 +++++++++++++------ .../test/altimate/pii-detector-e2e.test.ts | 13 ++++++-- packages/opencode/test/cli/check-e2e.test.ts | 14 +++++--- 7 files changed, 55 insertions(+), 22 deletions(-) diff --git a/packages/opencode/src/altimate/native/engine-coerce.ts b/packages/opencode/src/altimate/native/engine-coerce.ts index ec9de5e31b..b72ffae04a 100644 --- a/packages/opencode/src/altimate/native/engine-coerce.ts +++ b/packages/opencode/src/altimate/native/engine-coerce.ts @@ -13,7 +13,7 @@ */ /** PiiClassification is 'Email' | … | { Custom: string } | 'None'. */ -export function classificationToString(c: unknown, fallback = "PII"): string { +export function classificationToString(c: unknown, fallback = "UNKNOWN"): string { if (typeof c === "string") return c if (c && typeof c === "object" && typeof (c as { Custom?: unknown }).Custom === "string") { return (c as { Custom: string }).Custom diff --git a/packages/opencode/src/altimate/native/sql/register.ts b/packages/opencode/src/altimate/native/sql/register.ts index 60b565289f..443d54ddf4 100644 --- a/packages/opencode/src/altimate/native/sql/register.ts +++ b/packages/opencode/src/altimate/native/sql/register.ts @@ -10,6 +10,7 @@ import * as core from "@altimateai/altimate-core" import { register } from "../dispatcher" import { schemaOrEmpty, resolveSchema } from "../schema-resolver" import { preprocessIff, postprocessQualify } from "../altimate-core" +import { EngineCoerce } from "../engine-coerce" import type { SqlAnalyzeResult, SqlAnalyzeIssue, @@ -183,7 +184,7 @@ export function registerAllSql(): void { // --------------------------------------------------------------------------- register("sql.format", async (params) => { try { - const raw = core.formatSql(params.sql, params.dialect) + const raw = core.formatSql(params.sql, EngineCoerce.dialectHint(params.dialect)) const result = JSON.parse(JSON.stringify(raw)) return { success: result.success ?? true, @@ -462,7 +463,7 @@ export function registerAllSql(): void { register("lineage.check", async (params) => { try { const schema = params.schema_context ? (resolveSchema(undefined, params.schema_context) ?? undefined) : undefined - const raw = core.columnLineage(params.sql, params.dialect ?? undefined, schema ?? undefined) + const raw = core.columnLineage(params.sql, EngineCoerce.dialectHint(params.dialect), schema ?? undefined) const result = JSON.parse(JSON.stringify(raw)) return { success: true, diff --git a/packages/opencode/src/altimate/review/runner.ts b/packages/opencode/src/altimate/review/runner.ts index 2c8eb45e3e..84dfff79fe 100644 --- a/packages/opencode/src/altimate/review/runner.ts +++ b/packages/opencode/src/altimate/review/runner.ts @@ -204,6 +204,8 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun // failures surface as data.validation.errors. We also keep the legacy // top-level keys as a fallback for older core builds. const rawIssues = asArray(data.lint?.findings) + .concat(asArray(data.validation?.errors).map((e: any) => ({ ...e, rule: "validate", severity: "error" }))) + .concat(asArray(data.safety?.threats)) .concat(asArray(data.issues)) .concat(asArray(data.violations)) .concat(asArray(data.findings)) @@ -290,7 +292,7 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun try { const res = await Dispatcher.call("altimate_core.grade", { sql, schema_context: await resolveSchema() }) const data = (res.data ?? {}) as Record - return { grade: data.grade ?? data.overall_grade } + return { grade: data.overall_grade ?? data.grade } } catch { return {} } @@ -400,7 +402,7 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun .map((c) => ({ column: String(c.column ?? ""), // classification can be { Custom: string } — String() would emit "[object Object]" - classification: EngineCoerce.classificationToString(c.classification, ""), + classification: EngineCoerce.classificationToString(c.classification), confidence: typeof c.confidence === "number" ? c.confidence : 0, masking: c.suggested_masking ?? undefined, })) diff --git a/packages/opencode/src/cli/cmd/check-helpers.ts b/packages/opencode/src/cli/cmd/check-helpers.ts index 99ecf5004c..0334a03020 100644 --- a/packages/opencode/src/cli/cmd/check-helpers.ts +++ b/packages/opencode/src/cli/cmd/check-helpers.ts @@ -15,6 +15,8 @@ export interface Finding { severity: "error" | "warning" | "info" message: string suggestion?: string + /** Machine-readable column name for PII findings (not a position). */ + columnName?: string } export interface CheckCategoryResult { @@ -56,6 +58,8 @@ export function normalizeSeverity(s?: string | unknown): Severity { // must not degrade to info or --fail-on/--severity filters silently pass. if (lower === "error" || lower === "fatal" || lower === "critical" || lower === "high") return "error" if (lower === "warning" || lower === "warn" || lower === "medium") return "warning" + // Everything else (incl. engine "low") is info by design: low-severity + // threats inform but do not trip --fail-on warning. return "info" } diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index 6915f5f93b..3459459aba 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -112,15 +112,13 @@ async function runSafety(sql: string, file: string): Promise { []) as Array> if (issues.length > 0) { return issues.map((f) => { - // ThreatFinding.location is [byteOffset, byteLength] — label as bytes, - // since byte offsets diverge from character indexes on multibyte SQL. + // ThreatFinding.location is [byteOffset, byteLength] — label as bytes + // (offsets diverge from char indexes on multibyte SQL) and render the + // INCLUSIVE end. ThreatFinding carries no line/column/code fields. const loc = f.location as [number, number] | undefined - const at = Array.isArray(loc) ? ` (bytes ${loc[0]}-${loc[0] + loc[1]})` : "" + const at = Array.isArray(loc) ? ` (bytes ${loc[0]}-${loc[0] + Math.max(loc[1] - 1, 0)})` : "" return { file, - line: f.line as number | undefined, - column: f.column as number | undefined, - code: f.code as string | undefined, rule: (f.rule ?? f.category ?? "safety") as string, severity: normalizeSeverity(f.severity as string), message: `${(f.message ?? f.description ?? "") as string}${at}`, @@ -128,7 +126,12 @@ async function runSafety(sql: string, file: string): Promise { } }) } - if (!result.success || result.data.safe === false) { + if (!result.success) { + // Fail closed like every other check — a crashed safety engine must not + // pass --fail-on error. + return [dispatcherErrorFinding("safety", file, result.error ?? "altimate_core.safety failed")] + } + if (result.data.safe === false) { return [ { file, @@ -163,7 +166,8 @@ async function runPolicy(sql: string, file: string, policyJson: string, schemaPa rule: (f.rule ?? f.policy ?? "policy") as string, severity: normalizeSeverity(f.severity as string), message: (f.message ?? f.description ?? "") as string, - suggestion: f.suggestion as string | undefined, + // PolicyViolation carries its fix hint as `remediation`. + suggestion: (f.suggestion ?? f.remediation) as string | undefined, })) } if (result.data.allowed === false) { @@ -217,6 +221,9 @@ async function runPii(sql: string, file: string, schemaPath?: string): Promise (typeof e === "string" ? e : ((e as Record)?.message ?? String(e)))) + .filter(Boolean) + .join("; ") return [ { file, rule: "semantic", severity: "warning", - message: result.error ?? "Semantic check found issues", + message: (result.error ?? detail) || "Semantic check found issues", }, ] } @@ -316,7 +329,7 @@ async function runGrade( ...(((safety?.threats as Array> | undefined) ?? []).map((t) => { // ThreatFinding: location is [byteOffset, byteLength]; detail is the fix hint const loc = t.location as [number, number] | undefined - const at = Array.isArray(loc) ? ` (bytes ${loc[0]}-${loc[0] + loc[1]})` : "" + const at = Array.isArray(loc) ? ` (bytes ${loc[0]}-${loc[0] + Math.max(loc[1] - 1, 0)})` : "" return { ...t, rule: (t.rule as string) ?? "safety", diff --git a/packages/opencode/test/altimate/pii-detector-e2e.test.ts b/packages/opencode/test/altimate/pii-detector-e2e.test.ts index 574d9acf3a..06f3d53649 100644 --- a/packages/opencode/test/altimate/pii-detector-e2e.test.ts +++ b/packages/opencode/test/altimate/pii-detector-e2e.test.ts @@ -3,10 +3,19 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test" import { detectPii, piiColumnsFromReport } from "../../src/altimate/native/schema/pii-detector" import * as Registry from "../../src/altimate/native/connections/registry" -const RUN = process.env.ALTIMATE_RUN_WAREHOUSE_E2E === "1" +// Skip (don't crash) when the NAPI binary is absent — same guard as +// altimate-core-e2e.test.ts; this file imports the engine at module scope. +let coreAvailable = false +try { + require.resolve("@altimateai/altimate-core") + coreAvailable = true +} catch {} +const describeIf = coreAvailable ? describe : describe.skip + +const RUN = coreAvailable && process.env.ALTIMATE_RUN_WAREHOUSE_E2E === "1" const e2eTest = RUN ? test : test.skip -describe("piiColumnsFromReport (real engine PiiReport shape)", () => { +describeIf("piiColumnsFromReport (real engine PiiReport shape)", () => { test("extracts PII rows and drops 'None' rows from a live classifyPii result", () => { const core = require("@altimateai/altimate-core") const schema = core.Schema.fromJson( diff --git a/packages/opencode/test/cli/check-e2e.test.ts b/packages/opencode/test/cli/check-e2e.test.ts index f173ef81f0..dfb417ac8a 100644 --- a/packages/opencode/test/cli/check-e2e.test.ts +++ b/packages/opencode/test/cli/check-e2e.test.ts @@ -597,7 +597,11 @@ describe("check command E2E", () => { // Real core@0.7.0 scanSql shape: threats[], each { rule, severity, message, detail }. // Regression guard: the consumer previously only read issues/findings, so // real threats (e.g. the 0.6.0 unbalanced_quote rule) rendered as a generic warning. - const file = await writeSql(tmpDir.dir, "breakout.sql", "SELECT * FROM users WHERE name = 'x' OR 1=1 --';") + // Engine-faithful fixture: a dangling quote genuinely emits the + // unbalanced_quote rule at runtime (verified against the live 0.7.0 + // binary; the rule is only missing from the stale SafetyRule union in + // index.d.ts — filed upstream as altimate-core-internal#764). + const file = await writeSql(tmpDir.dir, "breakout.sql", "SELECT * FROM users WHERE name = 'x'';") setDispatcherResponse("altimate_core.safety", () => ({ success: true, data: { @@ -612,7 +616,7 @@ describe("check command E2E", () => { message: "Unbalanced quote suggests injection breakout", detail: "Quote count is odd within a single statement", // Real engine semantics: [byteOffset, byteLength] — "OR 1=1 " at 37. - location: [37, 7], + location: [33, 3], matched_pattern: "' OR 1=1 --", }, ], @@ -624,8 +628,8 @@ describe("check command E2E", () => { const j = parseJson(r.stdout) expect(j.results.safety.findings).toHaveLength(1) expect(j.results.safety.findings[0].rule).toBe("unbalanced_quote") - // ThreatFinding.location is [byteOffset, byteLength] — rendered as a byte range. - expect(j.results.safety.findings[0].message).toBe("Unbalanced quote suggests injection breakout (bytes 37-44)") + // ThreatFinding.location is [byteOffset, byteLength] — rendered as an INCLUSIVE byte range. + expect(j.results.safety.findings[0].message).toBe("Unbalanced quote suggests injection breakout (bytes 33-35)") expect(j.results.safety.findings[0].suggestion).toBe("Quote count is odd within a single statement") // Engine severity "high" must normalize to error, not degrade to info — // otherwise --fail-on/--severity filters silently pass high-risk injections. @@ -829,7 +833,7 @@ describe("check command E2E", () => { expect(byRule.validate.column).toBe(8) expect(byRule.validate.suggestion).toBe("Did you mean 'id'?") // ThreatFinding byte-range location and detail must survive too. - expect(byRule.tautology_attack.message).toBe("OR 1=1 detected (bytes 10-17)") + expect(byRule.tautology_attack.message).toBe("OR 1=1 detected (bytes 10-16)") expect(byRule.tautology_attack.suggestion).toBe("Remove the always-true predicate") }) From d4b12827d1db4faa5f875c1b60f93a61eedccab2 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Thu, 13 Aug 2026 18:04:09 +0530 Subject: [PATCH 11/14] =?UTF-8?q?fix:=20round-7=20review=20polish=20?= =?UTF-8?q?=E2=80=94=20threat=20normalization=20in=20review=20lane=20+=20f?= =?UTF-8?q?ail-closed=20partials?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Review runner: composite `safety.threats` are normalized before joining review issues — engine severities (`critical`/`high` → error, `medium` → warning, `low` → info) and the byte-tuple `location` dropped, so the quality lane classifies threats as errors instead of suggestions. - `check --checks safety`: a `success:false` envelope now appends the fail-closed error finding even when partial sub-error threats were returned — `--fail-on error` can no longer pass a crashed scanner. - Semantics tool: abstentions (`validation_errors`) set `metadata.success: false` so telemetry records the soft failure (consistent with query-pii); contract test updated accordingly. - Shared `byteRange()` helper replaces the duplicated inclusive-range formula in `runSafety`/`runGrade`. - `pii-detector-e2e.test.ts` defers the pii-detector import until after the NAPI-availability guard (static import defeated the guard). - Fixture cleanup: stale `matched_pattern`/comment from the old OR 1=1 payload removed; new fail-closed partial-threats test. Co-Authored-By: Claude Fable 5 --- .../opencode/src/altimate/review/runner.ts | 11 ++++- .../altimate/tools/altimate-core-semantics.ts | 5 ++- packages/opencode/src/cli/cmd/check.ts | 45 ++++++++++--------- .../test/altimate/pii-detector-e2e.test.ts | 8 +++- .../altimate/tool-error-propagation.test.ts | 8 +++- packages/opencode/test/cli/check-e2e.test.ts | 30 +++++++++++-- 6 files changed, 76 insertions(+), 31 deletions(-) diff --git a/packages/opencode/src/altimate/review/runner.ts b/packages/opencode/src/altimate/review/runner.ts index 84dfff79fe..061b7bc997 100644 --- a/packages/opencode/src/altimate/review/runner.ts +++ b/packages/opencode/src/altimate/review/runner.ts @@ -205,7 +205,16 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun // top-level keys as a fallback for older core builds. const rawIssues = asArray(data.lint?.findings) .concat(asArray(data.validation?.errors).map((e: any) => ({ ...e, rule: "validate", severity: "error" }))) - .concat(asArray(data.safety?.threats)) + .concat( + // ThreatFinding severity is low|medium|high|critical and its location + // is a byte tuple — normalize both so downstream lanes (which only + // recognize error/warning and location.line) classify them correctly. + asArray(data.safety?.threats).map((t: any) => ({ + ...t, + severity: t.severity === "critical" || t.severity === "high" ? "error" : t.severity === "medium" ? "warning" : "info", + location: undefined, + })), + ) .concat(asArray(data.issues)) .concat(asArray(data.violations)) .concat(asArray(data.findings)) diff --git a/packages/opencode/src/altimate/tools/altimate-core-semantics.ts b/packages/opencode/src/altimate/tools/altimate-core-semantics.ts index 302e22c749..d4691012aa 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-semantics.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-semantics.ts @@ -45,8 +45,9 @@ export const AltimateCoreSemanticsTool = Tool.define("altimate_core_semantics", title: hasError ? "Semantics: ERROR" : `Semantics: ${issueCount === 0 ? "VALID" : `${issueCount} issues`}`, metadata: { // Semantic issues are findings, not failures — but a failed engine - // call must not report success. - success: result.success, + // call or an abstention (validation_errors) must not report success: + // telemetry classifies soft failures on metadata.success === false. + success: result.success && !hasError, valid: data.valid, issue_count: issueCount, has_schema: hasSchema, diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index 3459459aba..d73e302057 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -22,6 +22,13 @@ import { // On Dispatcher failure, emit an error-severity finding so CI doesn't false-pass. // --------------------------------------------------------------------------- +/** Render an engine [byteOffset, byteLength] tuple as an inclusive byte range. */ +function byteRange(loc: unknown): string { + if (!Array.isArray(loc) || loc.length < 2) return "" + const [start, len] = loc as [number, number] + return ` (bytes ${start}-${start + Math.max(len - 1, 0)})` +} + function dispatcherErrorFinding(check: string, file: string, e: unknown): Finding { return { file, @@ -110,26 +117,24 @@ async function runSafety(sql: string, file: string): Promise { result.data.issues ?? result.data.findings ?? []) as Array> - if (issues.length > 0) { - return issues.map((f) => { - // ThreatFinding.location is [byteOffset, byteLength] — label as bytes - // (offsets diverge from char indexes on multibyte SQL) and render the - // INCLUSIVE end. ThreatFinding carries no line/column/code fields. - const loc = f.location as [number, number] | undefined - const at = Array.isArray(loc) ? ` (bytes ${loc[0]}-${loc[0] + Math.max(loc[1] - 1, 0)})` : "" - return { - file, - rule: (f.rule ?? f.category ?? "safety") as string, - severity: normalizeSeverity(f.severity as string), - message: `${(f.message ?? f.description ?? "") as string}${at}`, - suggestion: (f.suggestion ?? f.detail) as string | undefined, - } - }) - } + const mapped = issues.map((f) => ({ + file, + rule: (f.rule ?? f.category ?? "safety") as string, + severity: normalizeSeverity(f.severity as string), + // ThreatFinding.location is [byteOffset, byteLength] — labeled as bytes + // (offsets diverge from char indexes on multibyte SQL), inclusive end. + // ThreatFinding carries no line/column/code fields. + message: `${(f.message ?? f.description ?? "") as string}${byteRange(f.location)}`, + suggestion: (f.suggestion ?? f.detail) as string | undefined, + })) if (!result.success) { // Fail closed like every other check — a crashed safety engine must not - // pass --fail-on error. - return [dispatcherErrorFinding("safety", file, result.error ?? "altimate_core.safety failed")] + // pass --fail-on error, even when partial threats were returned and none + // of them is error-severity. + return [...mapped, dispatcherErrorFinding("safety", file, result.error ?? "altimate_core.safety failed")] + } + if (mapped.length > 0) { + return mapped } if (result.data.safe === false) { return [ @@ -328,12 +333,10 @@ async function runGrade( }) as Array>), ...(((safety?.threats as Array> | undefined) ?? []).map((t) => { // ThreatFinding: location is [byteOffset, byteLength]; detail is the fix hint - const loc = t.location as [number, number] | undefined - const at = Array.isArray(loc) ? ` (bytes ${loc[0]}-${loc[0] + Math.max(loc[1] - 1, 0)})` : "" return { ...t, rule: (t.rule as string) ?? "safety", - message: `${(t.message ?? "") as string}${at}`, + message: `${(t.message ?? "") as string}${byteRange(t.location)}`, location: undefined, suggestion: t.suggestion ?? t.detail, } diff --git a/packages/opencode/test/altimate/pii-detector-e2e.test.ts b/packages/opencode/test/altimate/pii-detector-e2e.test.ts index 06f3d53649..f57918774e 100644 --- a/packages/opencode/test/altimate/pii-detector-e2e.test.ts +++ b/packages/opencode/test/altimate/pii-detector-e2e.test.ts @@ -1,10 +1,10 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test" -import { detectPii, piiColumnsFromReport } from "../../src/altimate/native/schema/pii-detector" import * as Registry from "../../src/altimate/native/connections/registry" // Skip (don't crash) when the NAPI binary is absent — same guard as -// altimate-core-e2e.test.ts; this file imports the engine at module scope. +// altimate-core-e2e.test.ts. pii-detector imports the engine at module scope, +// so it must be loaded AFTER (and only if) the guard passes. let coreAvailable = false try { require.resolve("@altimateai/altimate-core") @@ -12,6 +12,10 @@ try { } catch {} const describeIf = coreAvailable ? describe : describe.skip +const { detectPii, piiColumnsFromReport } = coreAvailable + ? await import("../../src/altimate/native/schema/pii-detector") + : ({} as typeof import("../../src/altimate/native/schema/pii-detector")) + const RUN = coreAvailable && process.env.ALTIMATE_RUN_WAREHOUSE_E2E === "1" const e2eTest = RUN ? test : test.skip diff --git a/packages/opencode/test/altimate/tool-error-propagation.test.ts b/packages/opencode/test/altimate/tool-error-propagation.test.ts index 0040c2639a..73b70ac1d4 100644 --- a/packages/opencode/test/altimate/tool-error-propagation.test.ts +++ b/packages/opencode/test/altimate/tool-error-propagation.test.ts @@ -147,8 +147,12 @@ describe("altimate_core_semantics error propagation", () => { stubCtx(), ) - // Handler completed (success=true), but validation_errors are surfaced in metadata.error - expect(result.metadata.success).toBe(true) + // The handler completed, but validation_errors mean the semantic check + // ABSTAINED — a soft failure: telemetry classifies on + // metadata.success === false, so an ERROR-titled abstention must not + // report success. + expect(result.metadata.success).toBe(false) + expect(result.title).toBe("Semantics: ERROR") expect(result.metadata.error).toContain("Failed to resolve table") expect(telemetryWouldExtract(result.metadata)).not.toBe("unknown error") }) diff --git a/packages/opencode/test/cli/check-e2e.test.ts b/packages/opencode/test/cli/check-e2e.test.ts index dfb417ac8a..3898b602e3 100644 --- a/packages/opencode/test/cli/check-e2e.test.ts +++ b/packages/opencode/test/cli/check-e2e.test.ts @@ -589,8 +589,32 @@ describe("check command E2E", () => { const r = await runHandler(baseArgs({ files: [file], checks: "safety" })) const j = parseJson(r.stdout) - expect(j.results.safety.findings).toHaveLength(1) + // success:false envelope adds a fail-closed error finding alongside the threat. + expect(j.results.safety.findings).toHaveLength(2) expect(j.results.safety.findings[0].rule).toBe("sql-injection") + expect(j.results.safety.findings[1].rule).toBe("safety-error") + expect(j.results.safety.findings[1].severity).toBe("error") + }) + + test("safety envelope failure with only sub-error threats still fails closed", async () => { + // success:false + partial warning-severity threats must not let + // --fail-on=error pass — an error-severity envelope finding is appended. + const file = await writeSql(tmpDir.dir, "partial.sql", "SELECT 1;") + setDispatcherResponse("altimate_core.safety", () => ({ + success: false, + error: "scanner crashed midway", + data: { + safe: false, + threats: [{ rule: "multi_statement", severity: "medium", message: "Multiple statements" }], + }, + })) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [file], checks: "safety", "fail-on": "error", failOn: "error" })) + const j = parseJson(r.stdout) + expect(j.results.safety.findings).toHaveLength(2) + expect(j.results.safety.findings.some((f: any) => f.severity === "error")).toBe(true) + expect(j.summary.pass).toBe(false) }) test("safety check surfaces engine ThreatFinding shape (threats/rule/message/detail)", async () => { @@ -615,9 +639,9 @@ describe("check command E2E", () => { severity: "high", message: "Unbalanced quote suggests injection breakout", detail: "Quote count is odd within a single statement", - // Real engine semantics: [byteOffset, byteLength] — "OR 1=1 " at 37. + // Real engine semantics: [byteOffset, byteLength] — the 'x' literal. location: [33, 3], - matched_pattern: "' OR 1=1 --", + matched_pattern: "'", }, ], }, From 27624151ebb9b4c13d5b41c836e7fe0fd4e953ee Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Thu, 13 Aug 2026 18:26:31 +0530 Subject: [PATCH 12/14] =?UTF-8?q?fix:=20round-8=20review=20polish=20?= =?UTF-8?q?=E2=80=94=20lint-only=20validation=20gating,=20diff-scoped=20sa?= =?UTF-8?q?fety,=20CLI=20policy=20warnings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Review runner: validation errors from the composite check are surfaced ONLY when a real schema exists — in lint-only mode the throwaway `_altimate_lint_` schema marks every real table unknown and would flood the review (restores the documented intent of that mode). - Composite `altimate_core.check`: safety threats are diff-scoped like lint when `base_sql` is supplied — threats already present in the base (matched by rule + matched_pattern) are pre-existing, not introduced by the change. Real-engine regression test added. - `check --checks policy`: advisory `warnings[]` on an allowed result now surface as info findings (they do not fail the check). Test added. Co-Authored-By: Claude Fable 5 --- .../src/altimate/native/altimate-core.ts | 18 +++++++++++++- .../opencode/src/altimate/review/runner.ts | 12 ++++++++-- packages/opencode/src/cli/cmd/check.ts | 9 ++++++- .../test/altimate/altimate-core-e2e.test.ts | 17 +++++++++++++ packages/opencode/test/cli/check-e2e.test.ts | 24 +++++++++++++++++++ 5 files changed, 76 insertions(+), 4 deletions(-) diff --git a/packages/opencode/src/altimate/native/altimate-core.ts b/packages/opencode/src/altimate/native/altimate-core.ts index 92a42e54ed..32129f015f 100644 --- a/packages/opencode/src/altimate/native/altimate-core.ts +++ b/packages/opencode/src/altimate/native/altimate-core.ts @@ -181,7 +181,23 @@ export function registerAll(): void { params.schema_context ? JSON.stringify(params.schema_context) : undefined, ) : core.lint(params.sql, schema) - const safety = core.scanSql(params.sql) + // Diff-scope safety like lint: threats present in the base SQL are + // pre-existing, not introduced by this change — subtract them by + // (rule, matched_pattern) identity when a base is supplied. + let safety = core.scanSql(params.sql) + if (params.base_sql) { + try { + const baseKeys = new Set( + core.scanSql(params.base_sql).threats.map((t: any) => `${t.rule}|${t.matched_pattern}`), + ) + safety = { + ...safety, + threats: safety.threats.filter((t: any) => !baseKeys.has(`${t.rule}|${t.matched_pattern}`)), + } + } catch { + // Unscannable base — keep the full head scan (fail open to MORE findings). + } + } // PII exposure for the composite check — the tool has always rendered a // PII section; previously nothing populated it. Additive: a PII failure // must not fail the whole composite. diff --git a/packages/opencode/src/altimate/review/runner.ts b/packages/opencode/src/altimate/review/runner.ts index 061b7bc997..eec3732d81 100644 --- a/packages/opencode/src/altimate/review/runner.ts +++ b/packages/opencode/src/altimate/review/runner.ts @@ -188,7 +188,8 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun // native functions) — in BOTH full and lint-only modes. core's schema // validation requires ≥1 table, so when there's no manifest we attach a // throwaway table purely to carry the dialect; AST lint walks the query, - // not the schema, so it's inert (and validation errors aren't surfaced). + // not the schema, so it's inert (validation errors are only surfaced + // when a real schema exists — see hasTables below). const schema = (await resolveSchema()) as { tables?: Record } | undefined const hasTables = !!schema && Object.keys(schema.tables ?? {}).length > 0 const schemaContext = !dialect @@ -204,7 +205,14 @@ export function createDispatcherRunner(opts: DispatcherRunnerOptions): ReviewRun // failures surface as data.validation.errors. We also keep the legacy // top-level keys as a fallback for older core builds. const rawIssues = asArray(data.lint?.findings) - .concat(asArray(data.validation?.errors).map((e: any) => ({ ...e, rule: "validate", severity: "error" }))) + .concat( + // Only surface validation errors when a REAL schema was supplied — + // in lint-only mode the throwaway `_altimate_lint_` schema makes + // every real table "unknown" and would flood the review. + hasTables + ? asArray(data.validation?.errors).map((e: any) => ({ ...e, rule: "validate", severity: "error" })) + : [], + ) .concat( // ThreatFinding severity is low|medium|high|critical and its location // is a byte tuple — normalize both so downstream lanes (which only diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index d73e302057..ac49038b9b 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -160,7 +160,14 @@ async function runPolicy(sql: string, file: string, policyJson: string, schemaPa policy_json: policyJson, schema_path: schemaPath ?? "", }) - if (result.success && result.data.allowed !== false) return [] + // allowed:true can still carry advisory warnings — surface them as info. + const warningFindings = ((result.data.warnings ?? []) as Array>).map((w) => ({ + file, + rule: (w.rule ?? "policy") as string, + severity: "info" as const, + message: (w.message ?? "") as string, + })) + if (result.success && result.data.allowed !== false) return warningFindings const violations = (result.data.violations ?? result.data.findings ?? []) as Array> if (violations.length > 0) { return violations.map((f) => ({ diff --git a/packages/opencode/test/altimate/altimate-core-e2e.test.ts b/packages/opencode/test/altimate/altimate-core-e2e.test.ts index 68efd1ffd9..9891c779a6 100644 --- a/packages/opencode/test/altimate/altimate-core-e2e.test.ts +++ b/packages/opencode/test/altimate/altimate-core-e2e.test.ts @@ -1297,6 +1297,23 @@ describeIf("consumer contract sync (round 2)", () => { expect(result.output).not.toContain("No PII detected") }) + test("composite check diff-scopes safety threats against base_sql", async () => { + const { Dispatcher } = await import("../../src/altimate/native") + const threatSql = "SELECT * FROM customers WHERE first_name = 'x' OR 1=1 --" + // Full scan without base: the tautology threat is reported. + const full = await Dispatcher.call("altimate_core.check", { sql: threatSql, schema_context: SCHEMA }) + const fullThreats = ((full.data as any).safety?.threats ?? []).map((t: any) => t.rule) + expect(fullThreats).toContain("tautology_attack") + // Same threat already present in base: it is pre-existing, not introduced. + const scoped = await Dispatcher.call("altimate_core.check", { + sql: threatSql, + base_sql: threatSql, + schema_context: SCHEMA, + }) + const scopedThreats = ((scoped.data as any).safety?.threats ?? []).map((t: any) => t.rule) + expect(scopedThreats).not.toContain("tautology_attack") + }) + test("check tool PII section reports abstention for unparseable SQL, not a clean verdict", async () => { const { formatCheck } = await import("../../src/altimate/tools/altimate-core-check") const output = formatCheck({ diff --git a/packages/opencode/test/cli/check-e2e.test.ts b/packages/opencode/test/cli/check-e2e.test.ts index 3898b602e3..a76de49fab 100644 --- a/packages/opencode/test/cli/check-e2e.test.ts +++ b/packages/opencode/test/cli/check-e2e.test.ts @@ -702,6 +702,30 @@ describe("check command E2E", () => { expect(j.results.pii.findings[1].suggestion).toBeUndefined() }) + test("policy check surfaces advisory warnings on an allowed result", async () => { + const file = await writeSql(tmpDir.dir, "policy-warn.sql", "SELECT 1;") + const policyFile = path.join(tmpDir.dir, "policy.json") + await fs.writeFile(policyFile, JSON.stringify({ rules: [] })) + setDispatcherResponse("altimate_core.policy", () => ({ + success: true, + data: { + allowed: true, + violations: [], + warnings: [{ rule: "row_estimate", category: "cost_control", message: "Query may scan a large table" }], + policies_evaluated: 1, + }, + })) + installDispatcherMocks() + + const r = await runHandler(baseArgs({ files: [file], checks: "policy", policy: policyFile } as any)) + const j = parseJson(r.stdout) + expect(j.results.policy.findings).toHaveLength(1) + expect(j.results.policy.findings[0].severity).toBe("info") + expect(j.results.policy.findings[0].rule).toBe("row_estimate") + // Advisory warnings must not fail the check. + expect(j.summary.pass).toBe(true) + }) + test("pii check fails when the engine abstains via parse_error", async () => { // Unparseable SQL: engine returns success + parse_error + empty pii_columns. // No findings would let --fail-on PASS a file whose PII analysis never ran. From 90e2eaebd4285fee03f5d29f53593045827c40a4 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Thu, 13 Aug 2026 18:54:49 +0530 Subject: [PATCH 13/14] =?UTF-8?q?fix:=20round-9=20review=20polish=20?= =?UTF-8?q?=E2=80=94=20sound=20diff-scoping=20+=20composite=20lintDiff=20s?= =?UTF-8?q?chema=20bug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Safety diff-scoping is now a MULTISET subtraction (one base occurrence consumes one head occurrence of the same rule + matched_pattern) — a PR that adds a second identical injection still reports it; `safe` and `risk_score` are recomputed when every threat is pre-existing so the verdict doesn't stay stale-unsafe. - PII exposures are diff-scoped the same way: (table, column) pairs already exposed by the base are not reported as introduced. - Validation is deliberately NOT diff-scoped: the engine validates fail-fast (only the first error is reported), so base subtraction can hide genuinely new breakage behind a pre-existing error — proven by test; re-reporting a pre-existing error is the safe mode, documented inline. - Pre-existing bug found while testing: the composite check with `base_sql` + a flat agent schema_context crashed with "missing field tables" — lintDiff takes SchemaDefinition JSON; now normalized via `normalizeSchemaContext` (exported from schema-resolver). This made every diff-scoped composite call with a flat schema fail closed since the lintDiff wiring landed. - Semantic CLI abstention severity: error when a schema was provided (the analysis should have run), warning schema-less (abstains routinely). - Policy tool telemetry includes advisory warnings (`policy_warning`). - `pii-detector` gets the AGENTS.md namespace self-reexport. Co-Authored-By: Claude Fable 5 --- .../src/altimate/native/altimate-core.ts | 57 +++++++++++++++---- .../src/altimate/native/schema-resolver.ts | 2 +- .../altimate/native/schema/pii-detector.ts | 2 + .../altimate/tools/altimate-core-policy.ts | 8 ++- packages/opencode/src/cli/cmd/check.ts | 6 +- .../test/altimate/altimate-core-e2e.test.ts | 22 +++++++ 6 files changed, 80 insertions(+), 17 deletions(-) diff --git a/packages/opencode/src/altimate/native/altimate-core.ts b/packages/opencode/src/altimate/native/altimate-core.ts index 32129f015f..689f0cf134 100644 --- a/packages/opencode/src/altimate/native/altimate-core.ts +++ b/packages/opencode/src/altimate/native/altimate-core.ts @@ -11,7 +11,7 @@ import * as core from "@altimateai/altimate-core" import { EngineCoerce } from "./engine-coerce" import { register } from "./dispatcher" -import { schemaOrEmpty, resolveSchema } from "./schema-resolver" +import { schemaOrEmpty, resolveSchema, normalizeSchemaContext } from "./schema-resolver" import type { AltimateCoreResult } from "./types" // --------------------------------------------------------------------------- @@ -169,7 +169,11 @@ export function registerAll(): void { register("altimate_core.check", async (params) => { try { const schema = schemaOrEmpty(params.schema_path, params.schema_context) - const validation = await core.validate(params.sql, schema) + // NOTE: validation is deliberately NOT diff-scoped against base_sql. + // The engine validates fail-fast (only the FIRST error is reported), so + // subtracting base errors can hide genuinely new breakage behind a + // pre-existing one. Re-reporting a pre-existing error is the safe mode. + const validation: Record = toData(await core.validate(params.sql, schema)) // Diff-scoped lint: when a base SQL is supplied, core returns only the // findings the change INTRODUCED (pre-existing issues in the file are // dropped) — the structural comparison stays in the AST engine. @@ -178,21 +182,39 @@ export function registerAll(): void { ? core.lintDiff( params.sql, params.base_sql, - params.schema_context ? JSON.stringify(params.schema_context) : undefined, + // lintDiff takes SchemaDefinition JSON — normalize flat agent + // schemas too, or the whole composite throws "missing field tables". + params.schema_context ? normalizeSchemaContext(params.schema_context) : undefined, ) : core.lint(params.sql, schema) // Diff-scope safety like lint: threats present in the base SQL are - // pre-existing, not introduced by this change — subtract them by - // (rule, matched_pattern) identity when a base is supplied. - let safety = core.scanSql(params.sql) + // pre-existing, not introduced by this change. Subtract as a MULTISET on + // (rule, matched_pattern) — one base occurrence consumes one head + // occurrence, so a PR that ADDS a second identical injection still + // reports it. Recompute safe/risk_score from the surviving threats so a + // fully pre-existing threat set doesn't leave a stale unsafe verdict. + let safety: Record = toData(core.scanSql(params.sql)) if (params.base_sql) { try { - const baseKeys = new Set( - core.scanSql(params.base_sql).threats.map((t: any) => `${t.rule}|${t.matched_pattern}`), - ) + const baseCounts = new Map() + for (const t of core.scanSql(params.base_sql).threats) { + const k = `${t.rule}|${t.matched_pattern}` + baseCounts.set(k, (baseCounts.get(k) ?? 0) + 1) + } + const remaining = (safety.threats as any[]).filter((t: any) => { + const k = `${t.rule}|${t.matched_pattern}` + const left = baseCounts.get(k) ?? 0 + if (left > 0) { + baseCounts.set(k, left - 1) + return false + } + return true + }) safety = { ...safety, - threats: safety.threats.filter((t: any) => !baseKeys.has(`${t.rule}|${t.matched_pattern}`)), + threats: remaining, + safe: remaining.length === 0 ? true : safety.safe, + risk_score: remaining.length === 0 ? 0 : safety.risk_score, } } catch { // Unscannable base — keep the full head scan (fail open to MORE findings). @@ -204,13 +226,26 @@ export function registerAll(): void { let pii: Record try { pii = toData(core.checkQueryPii(params.sql, schema)) + if (params.base_sql && Array.isArray(pii.pii_columns) && (pii.pii_columns as any[]).length) { + try { + // Pre-existing exposures (same table.column already exposed by the + // base) are not introduced by this change. + const baseExposed = new Set( + core.checkQueryPii(params.base_sql, schema).pii_columns.map((c: any) => `${c.table}|${c.column}`), + ) + const remaining = (pii.pii_columns as any[]).filter((c: any) => !baseExposed.has(`${c.table}|${c.column}`)) + pii = { ...pii, pii_columns: remaining, accesses_pii: remaining.length > 0 ? pii.accesses_pii : false } + } catch { + // Unscannable base — keep the full head exposure list. + } + } } catch (e) { // Mark as an abstention — an empty object would render "No PII // detected", a false-clean verdict. pii = { parse_error: String(e) } } const data: Record = { - validation: toData(validation), + validation, lint: toData(lintResult), safety: toData(safety), pii, diff --git a/packages/opencode/src/altimate/native/schema-resolver.ts b/packages/opencode/src/altimate/native/schema-resolver.ts index 9dc042e129..1029fad29c 100644 --- a/packages/opencode/src/altimate/native/schema-resolver.ts +++ b/packages/opencode/src/altimate/native/schema-resolver.ts @@ -84,7 +84,7 @@ function flatToSchemaDefinition(flat: Record): Record * Normalize a schema_context into SchemaDefinition JSON format. * Accepts both flat and SchemaDefinition formats. */ -function normalizeSchemaContext(ctx: Record): string { +export function normalizeSchemaContext(ctx: Record): string { if (isSchemaDefinitionFormat(ctx)) { return JSON.stringify(ctx) } diff --git a/packages/opencode/src/altimate/native/schema/pii-detector.ts b/packages/opencode/src/altimate/native/schema/pii-detector.ts index cd311a714d..45b77e9a43 100644 --- a/packages/opencode/src/altimate/native/schema/pii-detector.ts +++ b/packages/opencode/src/altimate/native/schema/pii-detector.ts @@ -220,3 +220,5 @@ async function detectPiiLive(params: PiiDetectParams): Promise } } } + +export * as PiiDetector from "./pii-detector" diff --git a/packages/opencode/src/altimate/tools/altimate-core-policy.ts b/packages/opencode/src/altimate/tools/altimate-core-policy.ts index f4eb97c760..6831f8c266 100644 --- a/packages/opencode/src/altimate/tools/altimate-core-policy.ts +++ b/packages/opencode/src/altimate/tools/altimate-core-policy.ts @@ -25,9 +25,11 @@ export const AltimateCorePolicyTool = Tool.define("altimate_core_policy", { const error = result.error ?? data.error // altimate_change start — sql quality findings for telemetry const violations = Array.isArray(data.violations) ? data.violations : [] - const findings: Telemetry.Finding[] = violations.map((v: any) => ({ - category: v.rule ?? "policy_violation", - })) + const warnings = Array.isArray(data.warnings) ? data.warnings : [] + const findings: Telemetry.Finding[] = [ + ...violations.map((v: any) => ({ category: v.rule ?? "policy_violation" })), + ...warnings.map((w: any) => ({ category: w.rule ?? "policy_warning" })), + ] // altimate_change end // Engine PolicyResult: { allowed, violations, warnings, … } — `pass` // never existed, so every clean query used to render VIOLATIONS FOUND. diff --git a/packages/opencode/src/cli/cmd/check.ts b/packages/opencode/src/cli/cmd/check.ts index ac49038b9b..ad3bb9dbc1 100644 --- a/packages/opencode/src/cli/cmd/check.ts +++ b/packages/opencode/src/cli/cmd/check.ts @@ -276,7 +276,9 @@ async function runSemantic(sql: string, file: string, schemaPath?: string): Prom } if (result.data.valid === false) { // SemanticResult.validation_errors carries the reason the query is - // unplannable — surface it instead of a bare generic message. + // unplannable — surface it instead of a bare generic message. With a + // schema supplied this is an abstention that must fail closed (error); + // schema-less runs abstain routinely, so only warn there. const detail = (result.data.validation_errors as unknown[] | undefined) ?.map((e) => (typeof e === "string" ? e : ((e as Record)?.message ?? String(e)))) .filter(Boolean) @@ -285,7 +287,7 @@ async function runSemantic(sql: string, file: string, schemaPath?: string): Prom { file, rule: "semantic", - severity: "warning", + severity: schemaPath ? ("error" as const) : ("warning" as const), message: (result.error ?? detail) || "Semantic check found issues", }, ] diff --git a/packages/opencode/test/altimate/altimate-core-e2e.test.ts b/packages/opencode/test/altimate/altimate-core-e2e.test.ts index 9891c779a6..6948ea022b 100644 --- a/packages/opencode/test/altimate/altimate-core-e2e.test.ts +++ b/packages/opencode/test/altimate/altimate-core-e2e.test.ts @@ -1312,6 +1312,28 @@ describeIf("consumer contract sync (round 2)", () => { }) const scopedThreats = ((scoped.data as any).safety?.threats ?? []).map((t: any) => t.rule) expect(scopedThreats).not.toContain("tautology_attack") + // With every threat pre-existing, the verdict must not stay stale-unsafe. + expect((scoped.data as any).safety.safe).toBe(true) + expect((scoped.data as any).safety.risk_score).toBe(0) + }) + + test("composite check diff-scopes PII but NOT validation against base_sql", async () => { + const { Dispatcher } = await import("../../src/altimate/native") + const base = "SELECT zzz, email FROM customers" + const head = "SELECT zzz, yyy, email FROM customers" + const scoped = await Dispatcher.call("altimate_core.check", { sql: head, base_sql: base, schema_context: SCHEMA }) + const d = scoped.data as any + // Validation is deliberately NOT diff-scoped: the engine reports only + // the FIRST error, so base subtraction could hide new breakage (yyy). + expect(d.validation.valid).toBe(false) + expect((d.validation.errors ?? []).length).toBeGreaterThan(0) + // email was already exposed by the base — not introduced by this change. + const exposed = (d.pii?.pii_columns ?? []).map((c: any) => c.column) + expect(exposed).not.toContain("email") + // Full run (no base) still reports the exposure. + const full = await Dispatcher.call("altimate_core.check", { sql: head, schema_context: SCHEMA }) + const fullExposed = ((full.data as any).pii?.pii_columns ?? []).map((c: any) => c.column) + expect(fullExposed).toContain("email") }) test("check tool PII section reports abstention for unparseable SQL, not a clean verdict", async () => { From b35057fbdbeee00e83a7aeb90a5f1b23fe6cd96f Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Fri, 14 Aug 2026 14:34:10 +0530 Subject: [PATCH 14/14] =?UTF-8?q?fix:=20round-10=20review=20polish=20?= =?UTF-8?q?=E2=80=94=20alias-aware=20PII=20scoping=20+=20scoped=20scores?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - PII diff-scoping identity now includes the sorted `query_targets` — adding or renaming a SELECT-list alias for an already-exposed column is a NEW output exposure and surfaces; identical exposures stay filtered. Regression test covers rename vs identical. - `risk_score` is recomputed (documented severity-based approximation) when only SOME threats are pre-existing, instead of keeping the full head scan's score; `risk_level` resets to "None" when every PII exposure was pre-existing. - `schema-resolver` gets the AGENTS.md `SchemaResolver` self-reexport; the new `normalizeSchemaContext` consumer uses the namespace projection. Co-Authored-By: Claude Fable 5 --- .../src/altimate/native/altimate-core.ts | 38 ++++++++++++++----- .../src/altimate/native/schema-resolver.ts | 2 + .../test/altimate/altimate-core-e2e.test.ts | 22 +++++++++++ 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/packages/opencode/src/altimate/native/altimate-core.ts b/packages/opencode/src/altimate/native/altimate-core.ts index 689f0cf134..780f4ce210 100644 --- a/packages/opencode/src/altimate/native/altimate-core.ts +++ b/packages/opencode/src/altimate/native/altimate-core.ts @@ -11,7 +11,7 @@ import * as core from "@altimateai/altimate-core" import { EngineCoerce } from "./engine-coerce" import { register } from "./dispatcher" -import { schemaOrEmpty, resolveSchema, normalizeSchemaContext } from "./schema-resolver" +import { schemaOrEmpty, resolveSchema, SchemaResolver } from "./schema-resolver" import type { AltimateCoreResult } from "./types" // --------------------------------------------------------------------------- @@ -184,7 +184,7 @@ export function registerAll(): void { params.base_sql, // lintDiff takes SchemaDefinition JSON — normalize flat agent // schemas too, or the whole composite throws "missing field tables". - params.schema_context ? normalizeSchemaContext(params.schema_context) : undefined, + params.schema_context ? SchemaResolver.normalizeSchemaContext(params.schema_context) : undefined, ) : core.lint(params.sql, schema) // Diff-scope safety like lint: threats present in the base SQL are @@ -210,11 +210,20 @@ export function registerAll(): void { } return true }) + // The engine's risk_score covers the FULL head scan — once threats + // are filtered it no longer matches. Recompute a documented + // approximation from the surviving severities (matches the engine's + // observed single-threat scores closely enough for gating). + const severityScore: Record = { critical: 0.98, high: 0.95, medium: 0.6, low: 0.3 } + const rescored = + remaining.length === (safety.threats as any[]).length + ? (safety.risk_score as number) + : remaining.reduce((m: number, t: any) => Math.max(m, severityScore[t.severity] ?? 0.5), 0) safety = { ...safety, threats: remaining, safe: remaining.length === 0 ? true : safety.safe, - risk_score: remaining.length === 0 ? 0 : safety.risk_score, + risk_score: rescored, } } catch { // Unscannable base — keep the full head scan (fail open to MORE findings). @@ -228,13 +237,22 @@ export function registerAll(): void { pii = toData(core.checkQueryPii(params.sql, schema)) if (params.base_sql && Array.isArray(pii.pii_columns) && (pii.pii_columns as any[]).length) { try { - // Pre-existing exposures (same table.column already exposed by the - // base) are not introduced by this change. - const baseExposed = new Set( - core.checkQueryPii(params.base_sql, schema).pii_columns.map((c: any) => `${c.table}|${c.column}`), - ) - const remaining = (pii.pii_columns as any[]).filter((c: any) => !baseExposed.has(`${c.table}|${c.column}`)) - pii = { ...pii, pii_columns: remaining, accesses_pii: remaining.length > 0 ? pii.accesses_pii : false } + // Pre-existing exposures are not introduced by this change. The + // identity INCLUDES the sorted output aliases — adding or renaming + // a SELECT-list alias for an already-exposed column is a NEW + // output exposure and must still surface. + const exposureKey = (c: any) => + `${c.table}|${c.column}|${[...(c.query_targets ?? [])].sort().join(",")}` + const baseExposed = new Set(core.checkQueryPii(params.base_sql, schema).pii_columns.map(exposureKey)) + const remaining = (pii.pii_columns as any[]).filter((c: any) => !baseExposed.has(exposureKey(c))) + pii = { + ...pii, + pii_columns: remaining, + accesses_pii: remaining.length > 0 ? pii.accesses_pii : false, + // risk_level covered the full head report; with no surviving + // exposures it must not keep claiming risk. + risk_level: remaining.length === 0 ? "None" : pii.risk_level, + } } catch { // Unscannable base — keep the full head exposure list. } diff --git a/packages/opencode/src/altimate/native/schema-resolver.ts b/packages/opencode/src/altimate/native/schema-resolver.ts index 1029fad29c..0724ffe303 100644 --- a/packages/opencode/src/altimate/native/schema-resolver.ts +++ b/packages/opencode/src/altimate/native/schema-resolver.ts @@ -120,3 +120,5 @@ export function schemaOrEmpty( if (s !== null) return s return Schema.fromDdl("CREATE TABLE _empty_ (id INT);") } + +export * as SchemaResolver from "./schema-resolver" diff --git a/packages/opencode/test/altimate/altimate-core-e2e.test.ts b/packages/opencode/test/altimate/altimate-core-e2e.test.ts index 6948ea022b..7463c8140f 100644 --- a/packages/opencode/test/altimate/altimate-core-e2e.test.ts +++ b/packages/opencode/test/altimate/altimate-core-e2e.test.ts @@ -1334,6 +1334,28 @@ describeIf("consumer contract sync (round 2)", () => { const full = await Dispatcher.call("altimate_core.check", { sql: head, schema_context: SCHEMA }) const fullExposed = ((full.data as any).pii?.pii_columns ?? []).map((c: any) => c.column) expect(fullExposed).toContain("email") + // With every exposure pre-existing, risk_level must not stay stale. + expect((scoped.data as any).pii.risk_level).toBe("None") + }) + + test("PII diff-scoping keys on output aliases — a renamed alias still surfaces", async () => { + const { Dispatcher } = await import("../../src/altimate/native") + const base = "SELECT email AS old_contact FROM customers" + // Same source column, NEW output alias → a new exposure, not pre-existing. + const renamed = await Dispatcher.call("altimate_core.check", { + sql: "SELECT email AS new_contact FROM customers", + base_sql: base, + schema_context: SCHEMA, + }) + const renamedCols = ((renamed.data as any).pii?.pii_columns ?? []) as any[] + expect(renamedCols.some((c) => (c.query_targets ?? []).includes("new_contact"))).toBe(true) + // Identical alias → pre-existing, filtered. + const same = await Dispatcher.call("altimate_core.check", { + sql: base, + base_sql: base, + schema_context: SCHEMA, + }) + expect(((same.data as any).pii?.pii_columns ?? []).length).toBe(0) }) test("check tool PII section reports abstention for unparseable SQL, not a clean verdict", async () => {