diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fb6c7aeb..73c367336 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.9.6] - 2026-08-23 + +Correctness release: `check --checks {validate,semantic,grade,pii,safety,policy}` and the `altimate-core-*` tools (`migration`, `compare`, `track-lineage`, `query-pii`, `classify-pii`) now tell you the truth. Several previously returned false-clean or wrong output — teams gating CI on `check --fail-on error|warning` may see new failures on unchanged SQL. **These are real findings the tool previously missed, not regressions in your code.** Also fixes a data-hygiene bug where truncated tool-output files were being deleted the moment they were written. + +### Changed + +- **`check --checks *` now surfaces the findings it was missing — expect newly-failing CI runs.** `altimate-core` upgrade `0.5.1 → 0.7.0` plus a consumer-contract sync catches a class of latent shape-mismatch bugs where the CLI was reading fields the engine no longer emits and rendering false-clean output as a result. Concretely: (a) `check --checks validate` gated on the wrong field and passed every file, now maps `ValidationError.location` and fails closed on engine failure; (b) `check --checks semantic` treated `valid:true` as clean, but `valid` means "plannable" — cartesian products came back "OK" for months, now reads `findings` and never gates on `valid`; (c) `check --checks grade` read fields `evaluate()` doesn't return, so no grade or finding ever surfaced, now reads `overall_grade`/`scores.overall`/`lint.findings` (with nested findings from `validation.errors` + `safety.threats`); (d) `check --checks pii` mapped the column name to the numeric column-position field and printed `[object Object]` for `{ Custom: string }` classifications, now reports the exposing alias and stringifies properly; (e) `check --checks policy` titled on `pass` (engine returns `allowed`), so clean SQL always rendered "VIOLATIONS FOUND", now inverted; (f) `check --checks safety` maps engine `high → error` and `medium → warning` (previously both degraded to `info`), so `--fail-on error|warning` no longer silently passes high-risk SQL injection threats; a new `unbalanced_quote` safety rule ships from the engine. Findings' `rule` field (when set — `lint` findings may omit it if the engine didn't attach one) names the rule in `--format json` output — that's how the safety and policy catalog is discovered in practice. (#1090) +- **`altimate-core-migration` tool no longer renders "SAFE" for destructive migrations.** Previously read a non-existent `risks` field; every migration including `DROP COLUMN` came back safe. Now reads `findings`/`safe`/`overall_risk` (engine `MigrationResult`); never renders SAFE on engine error; counts only non-"safe" findings as risks. (#1090) +- **`altimate-core-compare` tool no longer says "IDENTICAL" for different queries.** Read a non-existent `differences` field; different SQL always compared equal. Now reads the engine's `identical`/`diff_count`/`diffs` shape with an error-gated title. (#1090) +- **`altimate-core-track-lineage` no longer returns "0 edges" for everything.** The tool read `edges` off the top-level result; the engine has always emitted them at `queries[].edges`, so lineage tracking silently produced nothing. Now collects correctly, renders `impact_map`, formats `{table, column}` refs, and renders ERROR instead of "0 edges" when the engine call itself fails. (#1090) +- **`schema.detect_pii` returned zero findings for every scan.** Same shape-mismatch class: the detector read `piiData.findings`, but the engine's `PiiReport` is `{ columns, pii_count, … }`. Both cache and live paths now filter `classification !== "None"` through the shared `piiColumnsFromReport` helper. Malformed `PiiReport` shapes now throw rather than silently yield zero findings. Failed per-column classifications flip `success` to `false` — the schema-detect-pii tool renders "PII Scan: ERROR" with any partial findings attached, instead of a clean "no findings" verdict. (#1090) +- **`altimate-core-{compare,column-lineage,extract-metadata,import-ddl}` no longer crash on the default invocation.** Empty-string dialects now coerce to `undefined` before hitting `Schema.fromDdl`, centralized as `dialectHint()` in `native/engine-coerce.ts` and applied across all 7 dialect-forwarding handlers. (#1090) + +### Fixed + +- **Truncated tool-output files were being deleted the moment they were written (since 2026-08-14).** `Identifier.create` packs `timestamp * 4096 + counter` into 6 bytes and wraps every ~795 days; the 26th wrap landed 2026-08-14T11:19:55Z. Both truncation cleanups (`tool/truncate.ts` Effect service and `tool/truncation.ts` legacy module — used by bootstrap, bash, prompt) computed a pre-wrap cutoff astronomically larger than every post-wrap file's decoded timestamp, so every truncated tool output written after Aug 14 was garbage-collected on the next cleanup pass. Both cleanups now age files by `mtime` (which doesn't wrap); `stat` failures keep the file (deletion fails safe, so a dangling symlink or transient FS error can't wipe real cached truncations). The Effect-service cleanup now `stat`s through the injected `FSUtil.Service` for consistency with every other operation. Tagged `upstream_fix` — the wrap-prone encoding is upstream OpenCode code. (#1113, closes #1112) +- **Native bridge no longer poisons itself for the process lifetime on a transient NAPI load failure.** The lazy registration hook was nulled *before* the `await fn()` that loads all bridge handler modules; if the altimate-core NAPI binding failed to load mid-sequence (older glibc, unusual arch, permissions), every subsequent `Dispatcher.call` — not just the failing one — threw `No native handler for X` for the rest of the process, and the CLI had to be restarted. Registration is now cached as an in-flight promise: concurrent callers share one attempt, and on failure the cached promise is cleared so subsequent calls can retry. (v0.9.6 review) + ## [0.9.5] - 2026-08-10 Windows `grep` back for the ~16% of Windows users it silently broke since v0.9.2, plus a mid-session YOLO toggle and a welcome panel that stops eating half of narrow terminals. diff --git a/docs/docs/usage/check.md b/docs/docs/usage/check.md index 851bb2c01..f79aedc5d 100644 --- a/docs/docs/usage/check.md +++ b/docs/docs/usage/check.md @@ -50,6 +50,8 @@ altimate-code check --fail-on warning | `semantic` | Semantic validation (cartesian products, wrong JOINs) | Optional | No | | `grade` | SQL quality grading with recommendations | Optional | No | +Findings usually carry a machine-readable `rule` field (e.g. `unbalanced_quote`, `sql_injection`, `L003`) visible in `--format json` output — see the [Finding Object](#finding-object) schema below. `safety` and `policy` findings always include one; `lint` findings may omit both `rule` and `code` if the engine didn't attach them, so switch on presence rather than assuming. + By default, `lint` and `safety` are enabled. Override with `--checks`: ```bash diff --git a/packages/opencode/src/altimate/native/dispatcher.ts b/packages/opencode/src/altimate/native/dispatcher.ts index abeb40898..5ea907647 100644 --- a/packages/opencode/src/altimate/native/dispatcher.ts +++ b/packages/opencode/src/altimate/native/dispatcher.ts @@ -20,15 +20,31 @@ export function register(method: BridgeMethod, handler: NativeHandler): void { /** Lazy registration hook — set by native/index.ts */ let _ensureRegistered: (() => Promise) | null = null +/** In-flight registration promise (deduped across concurrent callers). */ +let _registrationPromise: Promise | null = null + +/** Generation counter — bumped whenever the hook or in-flight promise is + * replaced. An in-flight attempt captures its generation at start; if the + * counter advanced by the time its settle handler fires, another caller + * (reset / setRegistrationHook / a distinct new attempt after failure) + * has already installed replacement state, and the stale attempt must NOT + * mutate it. Prevents a stale success from clobbering a replacement hook, + * and a stale failure from clobbering a newer in-flight promise. */ +let _registrationGeneration = 0 + /** Clear all registered handlers and lazy registration hook (for test isolation). */ export function reset(): void { nativeHandlers.clear() _ensureRegistered = null + _registrationPromise = null + _registrationGeneration++ } /** Called by native/index.ts to set the lazy registration function. */ export function setRegistrationHook(fn: () => Promise): void { _ensureRegistered = fn + _registrationPromise = null + _registrationGeneration++ } /** Dispatch a method call to the registered native handler. */ @@ -36,11 +52,52 @@ export async function call( method: M, params: (typeof BridgeMethods)[M] extends { params: infer P } ? P : never, ): Promise<(typeof BridgeMethods)[M] extends { result: infer R } ? R : never> { - // Lazy registration: load all handler modules on first call + // Lazy registration: load all handler modules on first call. Cache the + // in-flight promise so concurrent callers share one attempt; on failure + // clear the cached promise so a subsequent call can retry. Previously + // ``_ensureRegistered`` was nulled BEFORE the await, so a transient NAPI + // load failure poisoned the bridge for the process lifetime — every + // subsequent ``call`` threw ``No native handler for X`` with no way to + // recover without restarting the CLI. Generation guard prevents a stale + // attempt from mutating state a concurrent ``reset()``/``setRegistrationHook()`` + // has since replaced. (coderabbit round 1 — release/v0.9.6 review.) + // Concurrency contract: + // • ``reset()`` / ``setRegistrationHook()`` MAY be called while an + // older ``Dispatcher.call`` is in flight — the generation guard below + // blocks the stale attempt's ``.then`` handler from mutating shared + // state (``_ensureRegistered`` / ``_registrationPromise``) that the + // replacement installed. Adversarial tests below exercise both races. + // • What we DO NOT guarantee: if the stale hook body itself resumes + // after replacement and calls ``Dispatcher.register(...)`` late, + // that late write overwrites whatever the newer hook wrote — and + // no in-band signal lets us self-heal it without recreating the + // shared-state race the round-1 guard is meant to prevent (see the + // coderabbit + cubic round-2 exchange on release/v0.9.6). Callers + // that need late-write safety must serialise hook mutations + // against outstanding calls themselves. + // • Production never triggers late-write clobber: ``setRegistrationHook`` + // is called exactly once at startup by ``native/index.ts``, and + // ``reset()`` is test-only. if (_ensureRegistered) { - const fn = _ensureRegistered - _ensureRegistered = null - await fn() + if (!_registrationPromise) { + const fn = _ensureRegistered + const generation = ++_registrationGeneration + _registrationPromise = fn().then( + () => { + // Only clear _ensureRegistered if our generation is still current + // — otherwise a concurrent reset()/setRegistrationHook() already + // installed a replacement, and clearing would clobber it. + if (generation === _registrationGeneration) _ensureRegistered = null + }, + (err) => { + // Same guard on the failure path: don't null a newer in-flight + // promise from another attempt. + if (generation === _registrationGeneration) _registrationPromise = null + throw err + }, + ) + } + await _registrationPromise } const native = nativeHandlers.get(method as string) diff --git a/packages/opencode/test/skill/release-v0.9.6-adversarial.test.ts b/packages/opencode/test/skill/release-v0.9.6-adversarial.test.ts new file mode 100644 index 000000000..92e178b0f --- /dev/null +++ b/packages/opencode/test/skill/release-v0.9.6-adversarial.test.ts @@ -0,0 +1,249 @@ +/** + * Adversarial coverage for the v0.9.6 release payload. + * + * Focus: fixes that landed IN this release (not the whole PR history) — + * 1. Dispatcher retry-after-registration-failure (v0.9.6 review gremlin fix) + * 2. Dispatcher generation guard on shared-state mutations from stale + * .then handlers (coderabbit round 1) + * + * Explicitly NOT covered (test-isolation contract, see dispatcher.ts): + * - Late ``register()`` from a stale hook body after a replacement hook + * has already run. That scenario requires calling ``reset()`` / + * ``setRegistrationHook()`` while a call is still in flight — a + * production impossibility (hook is set once at startup, reset is + * test-only) and a violation of the test-isolation contract. See + * ``dispatcher.ts`` for the design decision and the coderabbit/cubic + * round-2 exchange that arrived at it. + * + * Not covered here (existing test suites are authoritative): + * - altimate-core 0.7.0 shape corrections — see + * packages/opencode/test/altimate/altimate-core-e2e.test.ts + * - Truncation ID-wrap regression — see + * packages/opencode/test/tool/truncation.test.ts + * + * Scheduling discipline: these tests never use ``setTimeout``. Every + * synchronisation point is a Promise gate that the test controls, so the + * ordering is deterministic regardless of the underlying scheduler. Bun's + * ``async`` function bodies run synchronously until the first ``await``, so + * ``Dispatcher.call(...)`` has already registered its cached promise and + * hit ``await _registrationPromise`` by the time control returns to us — + * we can immediately act on shared state without racing the call's setup. + */ +import { afterEach, beforeAll, afterAll, beforeEach, describe, expect, test } from "bun:test" + +import * as Dispatcher from "../../src/altimate/native/dispatcher" + +let _priorTelemetryDisabled: string | undefined +beforeAll(() => { + _priorTelemetryDisabled = process.env.ALTIMATE_TELEMETRY_DISABLED + process.env.ALTIMATE_TELEMETRY_DISABLED = "true" +}) +afterAll(() => { + // Restore any pre-existing value rather than unconditionally deleting — + // an outer suite may have set it and expects to see its own value after + // this file runs. (cubic P2 round 3.) + if (_priorTelemetryDisabled === undefined) { + delete process.env.ALTIMATE_TELEMETRY_DISABLED + } else { + process.env.ALTIMATE_TELEMETRY_DISABLED = _priorTelemetryDisabled + } +}) + +describe("v0.9.6 release: Dispatcher registration retry", () => { + beforeEach(() => { + Dispatcher.reset() + Dispatcher.setRegistrationHook(null as any) + }) + afterEach(() => { + Dispatcher.reset() + Dispatcher.setRegistrationHook(null as any) + }) + + test("failed registration does NOT poison future calls — hook re-runs on next call", async () => { + // Reproduces the gremlin finding: previously _ensureRegistered was + // nulled BEFORE the await, so a transient failure permanently disabled + // lazy registration for the rest of the process — every subsequent + // call threw "No native handler for X" with no path to recovery. + let attempts = 0 + Dispatcher.setRegistrationHook(async () => { + attempts += 1 + if (attempts === 1) { + throw new Error("simulated NAPI load failure") + } + Dispatcher.register("ping", async () => ({ status: "recovered" })) + }) + + await expect(Dispatcher.call("ping", {} as any)).rejects.toThrow( + "simulated NAPI load failure", + ) + expect(attempts).toBe(1) + + // Second call: registration hook MUST run again (this is the fix). + // Before v0.9.6 this call threw "No native handler for ping" because + // _ensureRegistered was permanently nulled after the first attempt. + const result = await Dispatcher.call("ping", {} as any) + expect(result).toEqual({ status: "recovered" }) + expect(attempts).toBe(2) + }) + + test("concurrent calls share ONE registration attempt", async () => { + // Fires 20 concurrent calls before the hook completes; asserts the + // hook body ran exactly once. All 20 Dispatcher.call invocations run + // sync-then-yield in a row, so by the time the array is populated + // every caller is already awaiting the shared _registrationPromise — + // no external synchronisation needed. + let attempts = 0 + let resolveRegistration: () => void = () => {} + const registrationGate = new Promise((r) => (resolveRegistration = r)) + Dispatcher.setRegistrationHook(async () => { + attempts += 1 + await registrationGate + Dispatcher.register("ping", async () => ({ status: "ok" })) + }) + + const calls = Array.from({ length: 20 }, () => Dispatcher.call("ping", {} as any)) + resolveRegistration() + const results = await Promise.all(calls) + + expect(results).toHaveLength(20) + for (const r of results) expect(r).toEqual({ status: "ok" }) + // Critical: the hook fired exactly ONCE despite 20 concurrent callers. + expect(attempts).toBe(1) + }) + + test("concurrent calls share ONE registration attempt on failure — all reject with the SAME error", async () => { + // Companion to the success case: assert dedup also holds when the + // hook fails. All N concurrent callers must reject with the same + // error object (proving they awaited the same in-flight promise), + // and the hook body must have run exactly once. (cubic P2 round 3.) + let attempts = 0 + let rejectGate: (err: Error) => void = () => {} + const gate = new Promise((_, rej) => (rejectGate = rej)) + Dispatcher.setRegistrationHook(async () => { + attempts += 1 + await gate + }) + + const calls = Array.from({ length: 20 }, () => Dispatcher.call("ping", {} as any)) + const failure = new Error("shared-failure") + rejectGate(failure) + + const results = await Promise.allSettled(calls) + expect(results.length).toBe(20) + for (const r of results) { + expect(r.status).toBe("rejected") + if (r.status === "rejected") { + // Same error instance = same underlying promise = dedup held. + expect(r.reason).toBe(failure) + } + } + expect(attempts).toBe(1) + }) + + test("successful registration is memoized — a later call does NOT re-run the hook", async () => { + // Guard against overcorrection: once the current-generation hook has + // completed, the resolved _registrationPromise memoizes success so + // subsequent calls fast-path through an already-settled await instead + // of re-importing all handler modules for no reason. + let attempts = 0 + Dispatcher.setRegistrationHook(async () => { + attempts += 1 + Dispatcher.register("ping", async () => ({ status: "ok" })) + }) + + await Dispatcher.call("ping", {} as any) + expect(attempts).toBe(1) + await Dispatcher.call("ping", {} as any) + expect(attempts).toBe(1) + }) + + test("reset() while a hook is pending — stale success does NOT clobber the replacement (generation guard)", async () => { + // coderabbit round 1 on release/v0.9.6: without a generation guard, + // an old attempt's success handler would null _ensureRegistered even + // after reset() + setRegistrationHook() had installed a replacement, + // wiping it out and causing later calls to skip registration. + let resolvePending: () => void = () => {} + const pending = new Promise((r) => (resolvePending = r)) + Dispatcher.setRegistrationHook(async () => { + await pending + Dispatcher.register("ping", async () => ({ status: "old-hook-ran" })) + }) + + // Dispatcher.call runs sync until its first await, so by the time this + // returns the call is already blocked on _registrationPromise and we + // can safely mutate shared state without a race. + const firstCall = Dispatcher.call("ping", {} as any) + + Dispatcher.reset() + let replacementRan = false + Dispatcher.setRegistrationHook(async () => { + replacementRan = true + Dispatcher.register("ping", async () => ({ status: "replacement" })) + }) + + resolvePending() + await firstCall.catch(() => {}) + + const result = await Dispatcher.call("ping", {} as any) + expect(replacementRan).toBe(true) + expect(result).toEqual({ status: "replacement" }) + }) + + test("setRegistrationHook() while a hook is pending — stale failure does NOT clobber the new in-flight promise", async () => { + // Failure handler also mutates shared state (_registrationPromise = null), + // so it needs the same generation guard. Otherwise a stale failure + // clears a newer in-flight promise, breaking dedup for anyone awaiting it. + let rejectOld: (err: Error) => void = () => {} + const oldPending = new Promise((_, rej) => (rejectOld = rej)) + Dispatcher.setRegistrationHook(async () => { + await oldPending + }) + const firstCall = Dispatcher.call("ping", {} as any) + + let newHookAttempts = 0 + let resolveNew: () => void = () => {} + const newPending = new Promise((r) => (resolveNew = r)) + Dispatcher.setRegistrationHook(async () => { + newHookAttempts += 1 + await newPending + Dispatcher.register("ping", async () => ({ status: "new-hook-ran" })) + }) + const secondCall = Dispatcher.call("ping", {} as any) + + rejectOld(new Error("old attempt failed")) + await firstCall.catch(() => {}) + + // Third caller arrives — must share the second call's still-cached + // promise (dedup works), not start a third registration attempt. + const thirdCall = Dispatcher.call("ping", {} as any) + resolveNew() + + const [r2, r3] = await Promise.all([secondCall, thirdCall]) + expect(r2).toEqual({ status: "new-hook-ran" }) + expect(r3).toEqual({ status: "new-hook-ran" }) + // Critical: new hook fired ONCE despite second + third both dedup-sharing + // its promise — proving the stale failure handler didn't null the cache. + expect(newHookAttempts).toBe(1) + }) + + test("reset() clears both the hook and the cached in-flight promise", async () => { + // reset() must wipe both _ensureRegistered AND _registrationPromise — + // otherwise a failed registration in one test leaves the cached + // rejected promise pinned for the next test. + let attempts = 0 + Dispatcher.setRegistrationHook(async () => { + attempts += 1 + throw new Error("boom") + }) + await expect(Dispatcher.call("ping", {} as any)).rejects.toThrow("boom") + expect(attempts).toBe(1) + + Dispatcher.reset() + Dispatcher.setRegistrationHook(async () => { + Dispatcher.register("ping", async () => ({ status: "fresh" })) + }) + + const result = await Dispatcher.call("ping", {} as any) + expect(result).toEqual({ status: "fresh" }) + }) +})