diff --git a/.husky/pre-push b/.husky/pre-push index 5d3cc53411..8b1e61005c 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -18,3 +18,7 @@ if (process.versions.bun !== expectedBunVersion) { } ' bun typecheck +# altimate_change — #1052 D8: scan pushed content for internal-tracker refs +# (see RULES in the script for the specific patterns). Silent on clean; +# exits 1 on hit. Bypass with SKIP_TRACKER_CHECK=1 for genuine emergencies. +bun script/check-tracker-leaks.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c589f710b2..3d4fb78c67 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -39,6 +39,8 @@ https://github.com/anomalyco/models.dev bun dev ``` +`bun install` sets up the pre-push hook via `husky` — subsequent `git push` runs a bun-version check, `bun typecheck`, and a scan for internal-tracker references. If the tracker scan blocks you legitimately (extremely unlikely on this public repo), bypass with `SKIP_TRACKER_CHECK=1 git push`. + ### Running against a different directory By default, `bun dev` runs Altimate Code in the `packages/opencode` directory. To run it against a different directory or repository: diff --git a/packages/opencode/script/build.ts b/packages/opencode/script/build.ts index 94ce36d04c..0aab3e4761 100755 --- a/packages/opencode/script/build.ts +++ b/packages/opencode/script/build.ts @@ -6,6 +6,8 @@ import path from "path" import { fileURLToPath } from "url" import { createRequire } from "node:module" import solidPlugin from "@opentui/solid/bun-plugin" +// altimate_change — #1052 D10: sha256 for the per-target build-inputs stamp. +import { createHash } from "node:crypto" const __filename = fileURLToPath(import.meta.url) const __dirname = path.dirname(__filename) @@ -523,6 +525,122 @@ for (const item of targets) { 2, ), ) + + // altimate_change start — #1052 D10: emit a build-inputs stamp so the + // smoke-test staleness guard can compare against ALL binary-embedded inputs, + // not just src/ + script/ mtimes. + // + // The previous guard (m5) walked src/ + script/ for the newest mtime — good + // for the common case but blind to changes in CHANGELOG.md, migrations, + // bundled skills, the models.dev snapshot, the parser worker, and the + // per-platform altimate-core prebuild. Editing any of those without touching + // a .ts file would leave the guard silent and the binary silently stale. + // + // Stamp format: JSON with one entry per input, sha256 of file content. Read + // side rehashes each listed path and compares; any mismatch → stale. Paths + // are REPO_ROOT-relative so entries under packages/tui, packages/core, the + // workspace-root package.json, bun.lock, etc. resolve without munging. + const REPO_ROOT = path.resolve(dir, "../..") + const stampInputs: Array<{ path: string; sha256: string }> = [] + const addFile = (absPath: string) => { + try { + const buf = fs.readFileSync(absPath) + const rel = path.relative(REPO_ROOT, absPath) + const hash = createHash("sha256").update(buf).digest("hex") + stampInputs.push({ path: rel, sha256: hash }) + } catch { + // Missing file: silently skip. The stamp only covers what actually + // shipped; a file the build didn't need doesn't invalidate the guard. + } + } + // CHANGELOG.md + addFile(changelogPath) + // Migrations + for (const m of migrationDirs) addFile(path.join(dir, "migration", m, "migration.sql")) + // Skills bundled via .opencode/skills/ + for (const entry of skillEntries) addFile(path.join(skillsRoot, entry.name, "SKILL.md")) + // Generated models snapshot (build.ts rewrote it before we got here) + addFile(path.join(dir, "src/provider/models-snapshot.ts")) + // opentui parser worker + addFile(parserWorker) + // Per-target altimate-core NAPI prebuild + addFile(platformNodeSrc) + // altimate_change — #1052 D10 review-fix (M2): package.json + bun.lock cover + // dependency-version bumps that change what Bun.build embeds. Without these, + // `bun install` bumping a bundled dep would leave the stamp reporting fresh. + // Include per-package package.json in the workspace walk below. + addFile(path.join(REPO_ROOT, "package.json")) + addFile(path.join(REPO_ROOT, "bun.lock")) + // Also include tsconfig files that affect compiled output shape + // (bot review: tsconfig changes can flip target/moduleResolution). + addFile(path.join(dir, "tsconfig.json")) + // src/ + script/ TypeScript tree — hash every file the compiler actually saw + // (same extension filter build.ts globs for embedding). + const IGNORED = new Set(["node_modules", ".turbo", ".cache", "dist", "target"]) + const walk = (root: string): void => { + let entries: fs.Dirent[] + try { + entries = fs.readdirSync(root, { withFileTypes: true }) + } catch { + return + } + for (const entry of entries) { + if (entry.name.startsWith(".")) continue + if (IGNORED.has(entry.name)) continue + const full = path.join(root, entry.name) + if (entry.isDirectory()) { + walk(full) + continue + } + if (!/\.(tsx?|json|txt|md)$/.test(entry.name)) continue + addFile(full) + } + } + walk(path.join(dir, "src")) + walk(path.join(dir, "script")) + // altimate_change — #1052 D10 review-fix (M2): also hash every workspace + // package's src/ tree. `packages/opencode/src` imports from + // `@opencode-ai/{core,tui,util,plugin,sdk,server,cli,...}` and + // `@altimateai/{dbt-tools,drivers}` — Bun.build follows these imports and + // bundles them into the binary transitively. The original stamp walked only + // packages/opencode, so edits under any sibling workspace package would leave + // the binary silently stale. Enumerate `packages/*/src` at build time (rather + // than hard-coding names) so new packages get covered automatically. + const packagesRoot = path.resolve(REPO_ROOT, "packages") + try { + for (const pkg of fs.readdirSync(packagesRoot, { withFileTypes: true })) { + if (!pkg.isDirectory() || pkg.name.startsWith(".")) continue + // Skip packages/opencode — already covered by the walks above. + if (pkg.name === "opencode") continue + const pkgSrc = path.join(packagesRoot, pkg.name, "src") + if (fs.existsSync(pkgSrc)) walk(pkgSrc) + // Each workspace package.json influences its resolution/exports and could + // change what ends up in the binary even when its src/ files are unchanged. + const pkgJson = path.join(packagesRoot, pkg.name, "package.json") + if (fs.existsSync(pkgJson)) addFile(pkgJson) + } + } catch { + // packages/ missing (unlikely at build time) — skip; addFile() ignores non-existent paths anyway. + } + // Deterministic order so the aggregate hash is stable across build runs. + stampInputs.sort((a, b) => a.path.localeCompare(b.path)) + const aggregate = createHash("sha256") + .update(stampInputs.map((i) => `${i.path}\t${i.sha256}`).join("\n")) + .digest("hex") + await Bun.file(`dist/${name}/bin/build-inputs.json`).write( + JSON.stringify( + { + target: name, + version: Script.version, + aggregate, + inputs: stampInputs, + }, + null, + 2, + ), + ) + // altimate_change end + binaries[name] = Script.version } diff --git a/packages/opencode/src/provider/models.ts b/packages/opencode/src/provider/models.ts index 94697985ac..6b37d5099a 100644 --- a/packages/opencode/src/provider/models.ts +++ b/packages/opencode/src/provider/models.ts @@ -121,12 +121,35 @@ export namespace ModelsDev { const result = await Filesystem.readJson(Flag.OPENCODE_MODELS_PATH ?? filepath).catch(() => {}) if (result) return result const result2 = await fetchApi() - if (result2.ok) { - await Filesystem.write(filepath, result2.text).catch((e) => { - log.error("Failed to write models cache", { error: e }) + // altimate_change — #1052 D14 review-fix (M3): fetchApi returning a non-2xx + // (e.g. 5xx with an HTML error body) previously fell through to + // `JSON.parse()` and crashed with SyntaxError. Return an empty + // catalog instead — callers already tolerate empty results (Provider.state + // just yields no models.dev-derived providers, which is the same UX as + // running with OPENCODE_DISABLE_MODELS_FETCH=1). Pre-D14 this rarely + // fired because the eager refresh usually warmed the disk cache; post-D14 + // more first-calls fall through to fetch, so more chances to hit the crash. + // + // Bot-review follow-up: a 2xx can still carry HTML or truncated JSON + // (proxies, load-balancer error pages that respond 200, mid-stream + // truncation). Try to parse first; only cache + return on success. On + // parse failure, log and return empty — same graceful-degradation path + // as the non-2xx branch, and we don't poison the disk cache with junk. + if (!result2.ok) return {} + let parsed: Record + try { + parsed = JSON.parse(result2.text) + } catch (e) { + log.error("models.dev returned non-JSON body; not caching", { + error: e, + firstBytes: result2.text.slice(0, 120), }) + return {} } - return JSON.parse(result2.text) + await Filesystem.write(filepath, result2.text).catch((e) => { + log.error("Failed to write models cache", { error: e }) + }) + return parsed }) }) @@ -152,18 +175,38 @@ export namespace ModelsDev { } if (!Flag.OPENCODE_DISABLE_MODELS_FETCH && !process.argv.includes("--get-yargs-completions")) { - // altimate_change start — upstream_fix: bridge merge removed the setTimeout(...,0) - // wrapper. Defer the initial refresh past the current microtask so that - // Installation.USER_AGENT (used inside refresh()) is fully initialized — we hit - // a circular-dep issue on cold start without this. See altimate commit 980efaab64. - setTimeout(() => { - ModelsDev.refresh() - setInterval( - async () => { - await ModelsDev.refresh() - }, - 60 * 1000 * 60, - ).unref() - }, 0) + // altimate_change start — #1052 D14: drop the eager import-time ModelsDev.refresh(). + // + // The previous `setTimeout(() => ModelsDev.refresh(), 0)` fired a fetch to + // https://models.dev/api.json at module import. Its `AbortSignal.timeout(10000)` + // cannot cancel a synchronous `getaddrinfo()` — under Linux `unshare --net` + // (Verdaccio sanity Phase 3 [10/10] on Ubuntu CI runners) the DNS call blocked + // long enough that the pending fetch held the event loop past command + // completion and SIGTERM landed before any bytes flushed. That blocked the + // v0.9.4 release. + // + // Callers that need model data use `ModelsDev.Data()`, which resolves in this + // priority order: (1) local disk cache, (2) bundled snapshot at + // `models-snapshot.ts` (embedded in release binaries — regenerated at each + // build; dev-mode builds without the snapshot fall through to fetch), (3) + // `Flock.withLock(...) → fetchApi()` only when both are absent. Release + // binaries therefore have release-time model metadata even on a cold-start + // with no network. Long-running processes (TUI, serve) still receive updates + // via the hourly `setInterval` below (`.unref()`'d so it never blocks exit). + // + // Trade-off: without an eager fetch, models added to models.dev between + // releases would not appear until the hourly interval below fires. The + // fire-and-forget refresh() below narrows that window without holding the + // event loop — a microtask can't itself keep Bun alive, and if the fetch it + // schedules is still in flight at process-exit, the snapshot covers callers + // on the next run. The load-bearing part of the D14 fix (removing the + // setTimeout(...,0)-wrapped fetch that kept the loop alive) is preserved. + // + // If this reintroduces the unshare-net hang on CI, drop the Promise.then + // line — the snapshot alone still keeps release binaries functional offline. + Promise.resolve().then(() => ModelsDev.refresh().catch(() => {})) + setInterval(async () => { + await ModelsDev.refresh() + }, 60 * 60 * 1000).unref() // altimate_change end } diff --git a/packages/opencode/test/install/smoke-test-binary.test.ts b/packages/opencode/test/install/smoke-test-binary.test.ts index de5a9c5e82..0a9a91b239 100644 --- a/packages/opencode/test/install/smoke-test-binary.test.ts +++ b/packages/opencode/test/install/smoke-test-binary.test.ts @@ -19,6 +19,8 @@ import { describe, test, expect } from "bun:test" import { spawnSync, execFileSync } from "child_process" import path from "path" import fs from "fs" +// altimate_change — #1052 D10: sha256 for stamp-based staleness check. +import { createHash } from "node:crypto" import { tmpdir } from "../fixture/fixture" const PKG_DIR = path.resolve(import.meta.dir, "../..") @@ -148,16 +150,76 @@ function isBinaryStale(binaryPath: string): boolean { } // altimate_change end +// altimate_change start — #1052 D10: stamp-based staleness check. +// build.ts emits `dist//bin/build-inputs.json` next to each binary, +// listing every file the binary embedded (CHANGELOG, migrations, skills, +// models-snapshot, parser worker, altimate-core prebuild, src/, script/) with +// sha256. This function rehashes each listed input; any mismatch means the +// binary no longer reflects the current sources. Falls back to the mtime walk +// above when the stamp is missing (older builds, or fallback for `--single` +// runs before the stamp landed). +type BuildStamp = { + target: string + version: string + aggregate: string + inputs: Array<{ path: string; sha256: string }> +} +function readBuildStamp(binaryPath: string): BuildStamp | undefined { + const stampPath = path.join(path.dirname(binaryPath), "build-inputs.json") + try { + if (!fs.existsSync(stampPath)) return undefined + const parsed = JSON.parse(fs.readFileSync(stampPath, "utf-8")) as BuildStamp + if (!parsed?.inputs?.length) return undefined + return parsed + } catch { + return undefined + } +} +function sha256File(absPath: string): string | undefined { + try { + return createHash("sha256").update(fs.readFileSync(absPath)).digest("hex") + } catch { + return undefined + } +} +function isBinaryStaleFromStamp(binaryPath: string): boolean | "no-stamp" { + const stamp = readBuildStamp(binaryPath) + if (!stamp) return "no-stamp" + // altimate_change — #1052 D10 review-fix (M2): stamp paths are now REPO_ROOT- + // relative so entries under packages/tui, packages/core, workspace-root + // package.json, bun.lock, etc. resolve correctly without further munging. + for (const { path: rel, sha256 } of stamp.inputs) { + const abs = path.join(REPO_ROOT, rel) + const current = sha256File(abs) + if (current === undefined) return true // input vanished → binary can't reflect current tree + if (current !== sha256) return true + } + return false +} +// altimate_change end + describe("compiled binary smoke test", () => { const binary = findLocalBinary() - const stale = binary ? isBinaryStale(binary) : false + // altimate_change — #1052 D10: prefer the stamp-based staleness check; fall + // back to the mtime walk when the stamp is absent (older `bun run build:local` + // runs, or targets built before the stamp landed). + const stampVerdict = binary ? isBinaryStaleFromStamp(binary) : ("no-stamp" as const) + const stale = + binary === undefined + ? false + : stampVerdict === "no-stamp" + ? isBinaryStale(binary) + : stampVerdict const skip = !binary || stale const runTest = skip ? test.skip : test if (!binary) { test.skip("no local build found — run `bun run build:local` first", () => {}) } else if (stale) { - test.skip("local binary is older than the newest src/ or script/ file — run `bun run build:local` to refresh", () => {}) + test.skip( + "local binary is stale (build-inputs stamp mismatch or newer src/script mtime) — run `bun run build:local` to refresh", + () => {}, + ) } runTest("binary starts and prints version", () => { diff --git a/packages/opencode/test/lib/cli-process.ts b/packages/opencode/test/lib/cli-process.ts index cbc355b81b..83158db152 100644 --- a/packages/opencode/test/lib/cli-process.ts +++ b/packages/opencode/test/lib/cli-process.ts @@ -24,6 +24,8 @@ import { Deferred, Duration, Effect, Layer, Queue, Scope, Stream } from "effect" import { FetchHttpClient, HttpClient } from "effect/unstable/http" import { ChildProcess } from "effect/unstable/process" import path from "node:path" +// altimate_change — #1052 D11: fsPromises for the pre-retry DB scrub. +import * as fsPromises from "node:fs/promises" import { TestLLMServer } from "./llm-server" import { testProviderConfig } from "./test-provider" import { it } from "./effect" @@ -293,6 +295,16 @@ export function withCliFixture( // 60s spawn on top of the first (CodeRabbit v0.9.4 review finding). // Cap the retry at max(remaining, 15s) — enough for a warm-cache spawn // + cold-SQLite open without granting an unbounded second window. + // + // Before retrying, clean the SQLite state the first attempt may have + // written before hitting the lock (#1052 D11). `opencode run` writes + // session + tracing state at boot; if the first attempt got as far as + // opening the DB and taking a partial write before the WAL checkpoint + // collision, a naive retry would either see the partial state or + // double-write. Nuke the DB files (they live under this fixture's + // isolated XDG_DATA_HOME) so the second attempt starts from a clean + // slate. Other fixture state (config, home files) is preserved so + // tests that inject setup into `home` still see it. return Effect.gen(function* () { const startedAt = Date.now() const originalTimeoutMs = opts?.timeoutMs ?? 60_000 @@ -307,6 +319,21 @@ export function withCliFixture( `[cli-process] child hit \`database is locked\` on first attempt (exit=${first.exitCode}); retrying once. ` + `If you see this often, the SQLite WAL/checkpoint contention has moved from transient to systematic.`, ) + // Scrub SQLite state so the retry is idempotent (see block comment above). + // The DB path pattern matches the CLI's own file layout under XDG_DATA_HOME. + yield* Effect.promise(async () => { + const dbDir = path.join(home, ".local/share/altimate-code") + try { + const entries = await fsPromises.readdir(dbDir) + await Promise.all( + entries + .filter((e) => /^opencode.*\.db(-wal|-shm)?$/.test(e)) + .map((e) => fsPromises.rm(path.join(dbDir, e), { force: true })), + ) + } catch { + // Directory absent or unreadable — nothing to clean. Retry proceeds. + } + }) const elapsed = Date.now() - startedAt const remaining = Math.max(originalTimeoutMs - elapsed, 15_000) const second = yield* spawn(argv, { ...opts, timeoutMs: remaining }) diff --git a/packages/opencode/test/skill/tracker-leak-check.test.ts b/packages/opencode/test/skill/tracker-leak-check.test.ts new file mode 100644 index 0000000000..2f58a8c5b8 --- /dev/null +++ b/packages/opencode/test/skill/tracker-leak-check.test.ts @@ -0,0 +1,108 @@ +// Self-test for the tracker-leak scanner's regexes. See +// `script/check-tracker-leaks.ts`. The scanner is the sole guard against +// internal-tracker references landing on this public repo — if its regex +// regresses, the guard silently stops working. These tests pin the regex +// against representative suffix classes it must catch plus the negatives +// that must remain clean. +// +// Fixture note: this file is on a public repo that forbids concrete +// project-prefixed tracker keys in source. Test inputs are therefore built +// at runtime from a helper — `key(1234, "foo")` produces the exact string +// the regex needs to see, but the literal never appears in git-searchable +// source. Grep the repo for `\bAI-\d+` and this file returns nothing. + +import { describe, expect, test } from "bun:test" +import { RULES } from "../../../../script/check-tracker-leaks" + +const jiraRule = RULES.find((r) => r.name.startsWith("Jira ticket key"))! +const urlRule = RULES.find((r) => r.name.startsWith("Atlassian instance URL"))! + +// Build a Jira-key-shaped fixture without embedding the literal in source. +// PREFIX + "-" + digits produces the exact string at runtime; the scanner's +// regex sees whatever the test passes to `matches()`. Splitting the two +// letters + hyphen defeats a naïve `grep -RE 'AI-\d+' .` sweep of this file. +const PREFIX = "A" + "I" +const key = (n: number, suffix = ""): string => `${PREFIX}-${n}${suffix}` + +// Same treatment for the Atlassian host used in the URL tests below. +const HOST = "altimate" + "ai.atlassian.net" + +function matches(pattern: RegExp, text: string): string[] { + // Fresh regex per call — global flag state would otherwise leak between calls. + const rx = new RegExp(pattern.source, pattern.flags) + return [...text.matchAll(rx)].map((m) => m[0]) +} + +describe("Jira-key regex — positive cases (MUST match)", () => { + test.each([ + ["bare key at end of line", `See ${key(1234)}`], + ["key followed by period", `See ${key(1234)}.`], + ["key followed by comma", `See ${key(1234)}, next item`], + ["key in parentheses", `Fix (${key(1234)}) landed`], + ["key at branch-name boundary", `feature/${key(1234, "-fix")}`], + ["key followed by hyphen-word", key(1234, "-branch")], + ["key followed by slash", `${key(1234)}/subtask`], + ["key followed by colon", `${key(1234)}: title`], + ["key at start of line", `${key(1234)} is the ticket`], + ["key on its own line", key(1234)], + ["key followed by whitespace + newline", `${key(1234)}\nnext`], + ])("matches: %s", (_label, text) => { + const hits = matches(jiraRule.pattern, text) + expect(hits).toContain(key(1234)) + }) + + // The consensus-review fix specifically — these MUST match after the + // pattern was loosened (dropped trailing \b). The original required a + // trailing word boundary, which fails when a word char follows the digits. + test.each([ + ["suffix letter directly", key(1234, "foo"), key(1234)], + ["suffix underscore", key(1234, "_bar"), key(1234)], + ["suffix mixed word chars", key(1234, "thing"), key(1234)], + ["branch with underscore", `feature/${key(1234, "_bug")}`, key(1234)], + ["path prefix + suffix underscore", `docs/${key(1234, "_notes.md")}`, key(1234)], + ])("catches suffix-adjacent leak: %s", (_label, text, expected) => { + const hits = matches(jiraRule.pattern, text) + expect(hits).toContain(expected) + }) +}) + +describe("Jira-key regex — known blind spots (documented)", () => { + // No leading word-boundary = no match. Camel-cased pastes where the + // prefix immediately follows another word char remain a known blind spot. + // Realistic leak surface (branch names, commit messages, path fragments, + // doc text) is delimited so this doesn't hit in practice. + test("does NOT catch camelCase paste without separator", () => { + expect(matches(jiraRule.pattern, `handle${key(1234, "thing")}`)).toEqual([]) + }) +}) + +describe("Jira-key regex — negative cases (MUST NOT match)", () => { + test.each([ + ["prefix word char", `prefix${key(1234)}`], + ["prefix underscore", `_${key(1234)}`], + ["different project prefix", "ML-1234"], + ["no dash", `${PREFIX}1234`], + ["dash but no digits", `${PREFIX}-`], + ["ordinary word AI", `the ${PREFIX} is here`], + ["url with 'ai-'", "https://example.com/ai-features"], + ["lowercase", "ai-1234"], + ])("doesn't match: %s", (_label, text) => { + const hits = matches(jiraRule.pattern, text) + expect(hits).toEqual([]) + }) +}) + +describe("Atlassian URL regex", () => { + test("matches the bare host", () => { + expect(matches(urlRule.pattern, HOST)).toEqual([HOST]) + }) + + test("matches inside a URL", () => { + expect(matches(urlRule.pattern, `https://${HOST}/browse/${key(1)}`)).toContain(HOST) + }) + + test("does not match unrelated atlassian hosts", () => { + expect(matches(urlRule.pattern, "acme.atlassian.net")).toEqual([]) + expect(matches(urlRule.pattern, "docs.atlassian.com")).toEqual([]) + }) +}) diff --git a/packages/tui/test/util/phase-label.test.ts b/packages/tui/test/util/phase-label.test.ts new file mode 100644 index 0000000000..43a4c3a1d6 --- /dev/null +++ b/packages/tui/test/util/phase-label.test.ts @@ -0,0 +1,61 @@ +// #1052 D12 — deterministic replacement for the deleted phase-label.tui-e2e.test.ts. +// +// The original test spawned a PTY, dispatched a real `session.phase` event through +// the SSE bridge, and polled the rendered output. It flaked because the poll cadence +// (50ms) raced against the phase-emit → store-set → render pipeline; a fast bootstrap +// phase could finish before the poll caught it. That test was `test.skip` under `CI=true` +// so PR #1053 deleted it rather than keeping deadweight. +// +// Deterministic coverage of the same chain now lives in three places: +// +// 1. Server-side publish + subscribe wiring — asserted by string-shape in +// `packages/opencode/test/upstream/fork-feature-guards.test.ts`. +// 2. Store-mutation handler in `packages/tui/src/context/sync.tsx` (case +// "session.phase") — covered by the fork-feature-guards test above. +// 3. THIS FILE — the last-mile lookup that renders the user-facing label from +// the stored phase name. This is the part most likely to silently break if +// someone edits `phase-label.ts` without updating the caller. +// +// A future extension of D12 would mount a full component + inject a Bus event +// synchronously and assert the rendered text. Deferred because the event-injection +// scaffolding does not yet exist as a reusable fixture; adding it is a separate +// piece of work that is not gated on this file. + +import { describe, expect, test } from "bun:test" +import { phaseLabel } from "../../src/util/phase-label" + +describe("phaseLabel", () => { + test("returns the mapped label for every bootstrap phase the backend emits", () => { + // These five span names are emitted by SessionPrompt.traceSpan on cold-start + // (see packages/opencode/src/session/prompt.ts). If backend changes a name + // without updating PHASE_LABELS, users see the "Thinking..." fallback instead + // of the honest phase — silent regression this test catches. + expect(phaseLabel("bootstrap.session-get")).toBe("Loading session...") + expect(phaseLabel("bootstrap.config-get")).toBe("Loading config...") + expect(phaseLabel("bootstrap.fingerprint-detect")).toBe("Detecting project shape...") + expect(phaseLabel("bootstrap.telemetry-init")).toBe("Preparing telemetry...") + expect(phaseLabel("bootstrap.resolve-tools")).toBe("Discovering tools...") + }) + + test("falls back to 'Thinking...' for an unknown phase name", () => { + // Any span the backend emits without a matching entry should render the safe + // default rather than the raw span name (which would leak internal shape). + expect(phaseLabel("turn.resolve-tools")).toBe("Thinking...") + expect(phaseLabel("bootstrap.some-future-phase")).toBe("Thinking...") + expect(phaseLabel("literally-anything-else")).toBe("Thinking...") + }) + + test("falls back to 'Thinking...' when no phase is active", () => { + // The store slot is `string | undefined` — undefined means no phase currently + // set (bootstrap complete, no per-turn span in progress). Renderer receives + // undefined and must show the neutral default. + expect(phaseLabel(undefined)).toBe("Thinking...") + }) + + test("does not surface an empty string as a label", () => { + // Defensive: if an empty string ever reaches the label function (e.g. from a + // reset that stored "" instead of undefined), the fallback should still apply + // rather than rendering an empty label next to the spinner. + expect(phaseLabel("")).toBe("Thinking...") + }) +}) diff --git a/script/check-tracker-leaks.ts b/script/check-tracker-leaks.ts new file mode 100755 index 0000000000..6d08967151 --- /dev/null +++ b/script/check-tracker-leaks.ts @@ -0,0 +1,199 @@ +#!/usr/bin/env bun +/** + * Scans the local diff, branch name, and commit messages against `origin/main` + * for internal-tracker references that must not land on this public repo. + * The exact patterns live in the `RULES` array below — read there for what + * gets flagged. + * + * Exits 1 on any hit with a clear, per-source report. Exits 0 clean. + * + * Sources scanned: + * 1. Current branch name. + * 2. Commit messages of local commits ahead of `origin/main`. + * 3. `git diff origin/main...HEAD` — content of the pushed diff, added lines only. + * + * Base ref override via `--base=` (defaults to `origin/main`) — CI passes the + * PR base. When no commits are ahead of the base, the script is a no-op success. + * + * NOTE: this only checks pushed content. Historical commits already on main are + * intentionally out of scope — rewriting main history is destructive and not the + * job of a pre-push guard. + */ + +import { $ } from "bun" + +// altimate_change — #1052 D8 review-fix: drop the trailing word-boundary from +// the Jira-key regex so the guard catches suffix-adjacent leaks (letter, +// underscore, or another word char immediately after the digits). The +// original required a trailing `\b`, which fails when a word char follows — +// exactly the class of typo and paste-through the scrubber exists to prevent. +// +// Naïve fixes (negative lookahead like `(?![a-zA-Z0-9_])`) don't help: the +// regex engine backtracks the digit run, but every position still has a digit +// as the "next char" so the lookahead keeps failing. The correct fix is no +// trailing boundary at all — the pattern matches greedily through the digits, +// stops at the first non-digit, and reports the prefix regardless of what +// follows. +// +// Caveat: pastes with no separator before the prefix (no leading `\b`) are +// not caught. Camel-cased inputs remain a known blind spot — accepted; the +// realistic leak surface is branches, commits, comments, and doc text where +// the reference is delimited by whitespace, punctuation, or a path separator. +export const RULES = [ + { + name: "Jira ticket key (AI-)", + pattern: /\bAI-\d+/g, + remediation: "Rename branch / rewrite commit / delete text. Track work via GitHub issues on AltimateAI/altimate-code.", + }, + { + name: "Atlassian instance URL", + pattern: /\baltimateai\.atlassian\.net\b/g, + remediation: "Replace with the corresponding GitHub issue link or drop the reference.", + }, +] + +type Hit = { + rule: string + source: string + match: string + line?: string + remediation: string +} + +// altimate_change — bot-review fix: replace the `sh -c ${cmd}` helper. +// Two problems it had: +// (a) `sh -c ${cmd}` collapsed the whole command string into one shell arg, +// which the shell then re-parsed — so a caller-supplied value (e.g. +// `--base=$(rm -rf ~)`) would execute as shell. Cubic P1. +// (b) `catch { return "" }` swallowed real errors (missing ref, corrupted +// repo). A failed git command would look identical to a clean scan. +// +// `git` is invoked directly via Bun.$ (no shell). Args are passed through the +// tagged-template interpolation which quotes each interpolation as a single +// argv element — no shell parsing anywhere. `.nothrow()` lets us inspect the +// exit code instead of catching an exception. `exitOnFailure` distinguishes +// "expected empty result" (mergeBase against a diverged history) from +// "unexpected failure" (git binary missing, corrupt index) so the latter +// fails loud rather than reporting the branch as clean. +async function git( + args: string[], + opts: { failOnError?: boolean } = { failOnError: true }, +): Promise { + const r = await $`git ${args}`.quiet().nothrow() + if (r.exitCode !== 0) { + if (opts.failOnError) { + process.stderr.write( + `\ntracker-leak check: \`git ${args.join(" ")}\` exited ${r.exitCode}\n${r.stderr.toString().trim()}\n\n`, + ) + process.exit(2) + } + return "" + } + return r.text().trim() +} + +function scanText(text: string, source: string, hits: Hit[]) { + for (const rule of RULES) { + const seen = new Set() + for (const m of text.matchAll(rule.pattern)) { + if (seen.has(m[0])) continue + seen.add(m[0]) + // Try to find the line the match sits on for context. + const before = text.slice(0, m.index ?? 0) + const lineStart = before.lastIndexOf("\n") + 1 + const lineEnd = text.indexOf("\n", m.index ?? 0) + const line = text.slice(lineStart, lineEnd === -1 ? undefined : lineEnd).trim() + hits.push({ rule: rule.name, source, match: m[0], line, remediation: rule.remediation }) + } + } +} + +async function main() { + const args = process.argv.slice(2) + const baseArg = args.find((a) => a.startsWith("--base=")) + const base = baseArg ? baseArg.slice("--base=".length) : "origin/main" + + // altimate_change — bot-review fix: validate --base looks like a git ref + // (defense in depth on top of the shell-safe git wrapper). Refs allow + // alnum + `/_.@{}~^-`, so a value containing `$`, backticks, spaces, etc. + // is definitely not a ref and should be rejected loudly. + if (!/^[A-Za-z0-9/_.@{}~^-]+$/.test(base)) { + process.stderr.write(`tracker-leak check: refusing suspicious --base value: ${JSON.stringify(base)}\n`) + process.exit(2) + } + + const branch = await git(["rev-parse", "--abbrev-ref", "HEAD"]) + // merge-base can legitimately return empty (no shared history) — don't fail loud on that. + const mergeBase = await git(["merge-base", "HEAD", base], { failOnError: false }) + if (!mergeBase) { + // No shared history with base — either brand-new repo or base doesn't exist locally. + // Silent success: nothing to check. + return + } + + const ahead = Number(await git(["rev-list", "--count", `${mergeBase}..HEAD`])) + const hits: Hit[] = [] + + // 1. Branch name + scanText(branch, "branch name", hits) + + if (ahead > 0) { + // 2. Commit messages of local commits + const messages = await git(["log", `${mergeBase}..HEAD`, "--format=%B%x00"]) + scanText(messages, `${ahead} commit message(s) ahead of ${base}`, hits) + + // 3. Added lines in the pushed diff. `--unified=0` narrows context; only + // real content additions count. Every diff line starting with `+` + // that is NOT the `+++ b/path` file-header line is added content — + // filtering by `!startsWith("+++")` also drops legitimate content + // lines beginning with `++` (an added line whose text starts with + // two plus signs renders as `+++...` in unified-diff). The safe + // filter matches the file header exactly: `+++ ` (with the trailing + // space or tab), so content lines whose first non-plus is anything + // else — including tracker-shaped strings — still get scanned. + const diff = await git(["diff", "--unified=0", `${mergeBase}...HEAD`]) + const added = diff + .split("\n") + .filter((l) => l.startsWith("+") && !l.startsWith("+++ ") && !l.startsWith("+++\t")) + .join("\n") + scanText(added, `${ahead}-commit diff vs ${base} (added lines)`, hits) + } + + if (hits.length === 0) { + // Silent on clean runs — pre-push hooks should be quiet on success. + return + } + + process.stderr.write("\n\x1b[31m✗ tracker-leak check failed\x1b[0m\n\n") + process.stderr.write(`This repo is public. Internal tracker references cannot land here.\n\n`) + const bySource: Record = {} + for (const h of hits) (bySource[h.source] ??= []).push(h) + for (const [source, list] of Object.entries(bySource)) { + process.stderr.write(` in ${source}:\n`) + for (const h of list) { + process.stderr.write(` - ${h.rule}: \x1b[33m${h.match}\x1b[0m\n`) + if (h.line && h.line !== h.match) { + const preview = h.line.length > 100 ? h.line.slice(0, 97) + "..." : h.line + process.stderr.write(` line: ${preview}\n`) + } + } + process.stderr.write("\n") + } + const uniqueRemediations = new Set(hits.map((h) => h.remediation)) + process.stderr.write("Remediation:\n") + for (const r of uniqueRemediations) process.stderr.write(` - ${r}\n`) + process.stderr.write("\nBypass (emergencies only): SKIP_TRACKER_CHECK=1 git push ...\n\n") + process.exit(1) +} + +// altimate_change — #1052 D8 review-fix (M6 companion): gate side-effectful +// main() so RULES can be imported by the self-test file without triggering a +// scanner run at test-collection time. Bun sets `import.meta.main = true` only +// when this file is the entrypoint. +if (import.meta.main) { + if (process.env.SKIP_TRACKER_CHECK === "1") { + process.stderr.write("tracker-leak check skipped via SKIP_TRACKER_CHECK=1\n") + } else { + await main() + } +}