Skip to content

fix: two tests flaky under parallel CI load (S27 + trace snapshot) #880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
anandgupta42 merged 2 commits into from
Jun 4, 2026
+38 −12
Merged

fix: two tests flaky under parallel CI load (S27 + trace snapshot) #880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba6cce2
fix: two tests flaky under parallel CI load (S27 + trace snapshot)
anandgupta42 Jun 3, 2026
79df91a
fix: raise CI test timeout 30s→90s to kill resource-contention flakiness
anandgupta42 Jun 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,13 @@ jobs:
# Cloud E2E tests (Snowflake, BigQuery, Databricks) auto-skip when
# ALTIMATE_CODE_CONN_* env vars are not set. Docker E2E tests auto-skip
# when Docker is not available. No exclusion needed — skipIf handles it.
# --timeout 30000: matches package.json "test" script; prevents 5s default
# from cutting off tests that run bun install or bootstrap git instances.
# --timeout 90000: the full suite (9500+ tests across 379 files) runs in
# one parallel bun process. Under CPU contention a handful of slower tests
# (real fs/spawn/git-bootstrap work) get starved and exceed a tight timeout
# NON-deterministically — different tests each run — failing CI with
# "timed out after Nms" (observed 32s/51s at the old 30s limit). 90s gives
# ~3x headroom over the worst observed, killing the resource-contention
# flakiness without masking genuinely-hung tests.
#
# Bun 1.3.x has a known segfault during process cleanup after all tests
# pass (exit code 143/SIGTERM or 134/SIGABRT). We capture test output and
Expand All @@ -108,7 +113,7 @@ jobs:
run: |
# Redirect bun output to file, then cat it for CI visibility.
# This avoids tee/pipe issues where SIGTERM kills tee before flush.
bun test --timeout 30000 > /tmp/test-output.txt 2>&1 || true
bun test --timeout 90000 > /tmp/test-output.txt 2>&1 || true
cat /tmp/test-output.txt

# Extract pass/fail counts from Bun test summary (e.g., " 5362 pass")
Expand Down
32 changes: 24 additions & 8 deletions packages/opencode/test/altimate/tracing-adversarial-snapshot.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,26 @@ const ZERO_STEP = {
tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } },
}

// Poll the on-disk snapshot until it reaches `expected` status, instead of a
// single fixed sleep. Snapshot writes are debounced/async, so a hardcoded delay
// is too short under heavy parallel CI load (the snapshot hasn't flushed yet) →
// flaky reads of a stale status. Polling is robust regardless of machine load.
async function pollStatus(tracer: { getTracePath(): string | undefined }, expected: string, timeoutMs = 4000) {
const start = Date.now()
let last = "<none>"
while (Date.now() - start < timeoutMs) {
try {
const snap = JSON.parse(await fs.readFile(tracer.getTracePath()!, "utf-8")) as TraceFile
last = snap.summary.status
if (last === expected) return snap
} catch {
/* file mid-write or not yet created — keep polling */
}
await new Promise((r) => setTimeout(r, 25))
}
throw new Error(`timed out after ${timeoutMs}ms waiting for status '${expected}' (last seen '${last}')`)
}
Comment on lines +44 to +58
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add defensive null check for getTracePath() return value.

Line 49 uses a non-null assertion (!) on tracer.getTracePath(), but the type signature correctly indicates it can return undefined (e.g., when using HttpExporter only, as shown in the test at lines 460-464). If pollStatus is called with a tracer lacking a FileExporter, it will crash with an unclear error message.

🛡️ Proposed fix: add explicit null check 
 async function pollStatus(tracer: { getTracePath(): string | undefined }, expected: string, timeoutMs = 4000) {
+  const tracePath = tracer.getTracePath()
+  if (!tracePath) {
+    throw new Error("getTracePath() returned undefined - ensure tracer has a FileExporter configured")
+  }
   const start = Date.now()
   let last = "<none>"
   while (Date.now() - start < timeoutMs) {
     try {
-      const snap = JSON.parse(await fs.readFile(tracer.getTracePath()!, "utf-8")) as TraceFile
+      const snap = JSON.parse(await fs.readFile(tracePath, "utf-8")) as TraceFile
       last = snap.summary.status
       if (last === expected) return snap
     } catch {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function pollStatus(tracer: { getTracePath(): string | undefined }, expected: string, timeoutMs = 4000) {
const start = Date.now()
let last = "<none>"
while (Date.now() - start < timeoutMs) {
try {
const snap = JSON.parse(await fs.readFile(tracer.getTracePath()!, "utf-8")) as TraceFile
last = snap.summary.status
if (last === expected) return snap
} catch {
/* file mid-write or not yet created — keep polling */
}
await new Promise((r) => setTimeout(r, 25))
}
throw new Error(`timed out after ${timeoutMs}ms waiting for status '${expected}' (last seen '${last}')`)
}
async function pollStatus(tracer: { getTracePath(): string | undefined }, expected: string, timeoutMs = 4000) {
const tracePath = tracer.getTracePath()
if (!tracePath) {
throw new Error("getTracePath() returned undefined - ensure tracer has a FileExporter configured")
}
const start = Date.now()
let last = "<none>"
while (Date.now() - start < timeoutMs) {
try {
const snap = JSON.parse(await fs.readFile(tracePath, "utf-8")) as TraceFile
last = snap.summary.status
if (last === expected) return snap
} catch {
/* file mid-write or not yet created — keep polling */
}
await new Promise((r) => setTimeout(r, 25))
}
throw new Error(`timed out after ${timeoutMs}ms waiting for status '${expected}' (last seen '${last}')`)
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/test/altimate/tracing-adversarial-snapshot.test.ts` around
lines 44 - 58, The pollStatus function uses a non-null assertion on
tracer.getTracePath(), which can return undefined; replace that usage by reading
tracer.getTracePath() into a local variable (e.g., tracePath) before the loop
and add a defensive null/undefined check that throws a clear Error if no trace
path is available, then use tracePath (without !) inside fs.readFile; update the
function signature/body references to refer to tracePath to avoid the crash when
a FileExporter is absent.


// ---------------------------------------------------------------------------
// 1. buildTraceFile — snapshot isolation from mutations
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -110,8 +130,6 @@ describe("buildTraceFile — snapshot isolation", () => {
test("buildTraceFile shows 'running' status during active generation", async () => {
const tracer = Recap.withExporters([new FileExporter(tmpDir)])
tracer.startTrace("s-running", { prompt: "test" })
// Wait for initial snapshot to complete
await new Promise((r) => setTimeout(r, 50))

tracer.logStepStart({ id: "1" })
tracer.logToolCall({
Expand All @@ -120,15 +138,13 @@ describe("buildTraceFile — snapshot isolation", () => {
state: { status: "completed", input: {}, output: "ok", time: { start: 1, end: 2 } },
})

// Wait for snapshot — should show "running" since generation is in progress
await new Promise((r) => setTimeout(r, 50))
const snap = JSON.parse(await fs.readFile(tracer.getTracePath()!, "utf-8")) as TraceFile
// Snapshot should show "running" since generation is in progress.
const snap = await pollStatus(tracer, "running")
expect(snap.summary.status).toBe("running")

// After finishing generation, should show "completed"
// After finishing generation, should show "completed".
tracer.logStepFinish(ZERO_STEP)
await new Promise((r) => setTimeout(r, 50))
const snap2 = JSON.parse(await fs.readFile(tracer.getTracePath()!, "utf-8")) as TraceFile
const snap2 = await pollStatus(tracer, "completed")
expect(snap2.summary.status).toBe("completed")

await tracer.endTrace()
Expand Down
7 changes: 6 additions & 1 deletion packages/opencode/test/session/real-tool-simulation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
import { describe, expect, test, beforeEach, mock } from "bun:test"
import { Dispatcher } from "../../src/altimate/native"
import { Log } from "../../src/util/log"
// Static import so resetShownSuggestions() targets the SAME module instance that
// the tools (sql-analyze, schema-inspect) use. A dynamic `await import` here can
// resolve to a different instance under parallel CI, leaving the real dedup Set
// un-reset → flaky cross-test pollution of the progressive-suggestion state.
import { PostConnectSuggestions } from "../../src/altimate/tools/post-connect-suggestions"

Log.init({ print: false })

Expand All @@ -37,7 +42,6 @@ function makeCtx(agent = "builder") {
// ---------------------------------------------------------------------------
beforeEach(async () => {
Dispatcher.reset()
const { PostConnectSuggestions } = await import("../../src/altimate/tools/post-connect-suggestions")
PostConnectSuggestions.resetShownSuggestions()
})

Expand Down Expand Up @@ -291,6 +295,7 @@ describe("REAL EXEC: sql_analyze tool", () => {

test("S27: sql_analyze with parse error — no suggestion", async () => {
Dispatcher.reset()
PostConnectSuggestions.resetShownSuggestions()
Dispatcher.register("sql.analyze", async () => ({
success: true, issues: [], issue_count: 0, confidence: "none",
confidence_factors: [], error: "Parse error at line 1",
Expand Down
Loading