Skip to content

test: cover stream handler, billing, limits, and permission evaluation; fix CI checks - #288

Open
usehoplite[bot] wants to merge 5 commits into
mainfrom
hoplite/megale-polis-09426822
Open

test: cover stream handler, billing, limits, and permission evaluation; fix CI checks#288
usehoplite[bot] wants to merge 5 commits into
mainfrom
hoplite/megale-polis-09426822

Conversation

@usehoplite

@usehoplite usehoplite Bot commented Sep 2, 2026

Copy link
Copy Markdown

What

Fixes the failing CI checks on this PR (TypeScript Check, Lint, Test) — which were failing on every run, including on main, for the same pre-existing reasons — and adds focused tests for the riskiest untested production paths.

Root cause of the red checks

bun.lock was stale: it still referenced the deleted apps/video workspace, so bun install --frozen-lockfile (step 1 of every job) failed in ~2 seconds. Once that is fixed, three layers of pre-existing debt surface: missing generated Prisma clients in CI, 30+ TypeScript errors, and ~90 lint errors.

Changes

CI (ci.yml, package.json, bun.lock)

  • Regenerate bun.lock with the pinned bun 1.2.21 (removes only the stale apps/video subtree + outdated configVersion).
  • New root db:generate script; run it (with DATABASE_URL) before typecheck and test — two packages import generated Prisma clients that CI never generated (CLI server → src/generated, superdesign-dbprisma/client).

Tests (69 new, all passing)

File Coverage
openai-compatible-stream.test.ts (28) SSE handler shared by 6+ provider routes: chunk boundaries, reasoning deltas, streamed tool_calls assembly, dedup, error events, usage, embedded MiniMax calls, non-streaming fallback, writeFinish
permission-rules.test.ts (11) dangerous-command detection, read-only allowlist, session override, agent precedence
token-budget.test.ts (12) daily budget (DB + proxy), opus caps, 48h pruning, device id
credit-meter.test.ts (10) credit cost math, soft-gate deduction paths
pricing.test.ts (8) cost math, alias lookup, provider labels

Production bug fix surfaced by the testsstreamOpenAICompatibleChat flushed and cleared pending tool calls after every SSE read, silently dropping tool calls whose arguments arrive across multiple reads and returning a bogus "empty response" error. Removed the in-loop flush (EOF flush still handles [DONE]); covered by a regression test.

Pre-existing failures fixed to make the checks green

  • TypeScript: server strict-narrowing (todowrite, question), missing MergeConnectorManager methods used by /connectors (implemented), test mock typing; cortex-sdk v2/v3 AI SDK model types unified via GatewayModel + defensive usage normalization.
  • Lint: supercode-cli-client (Link, entity escaping, purity-safe skeleton width, useSyncExternalStore mounted flag) and apps/web (unescaped entities in marketing pages, explicit any in chart/server-action code, setState-in-effect and purity fixes, motion slot component caching).
  • Test drift: permission-prompt.test.ts asserted the pre-allowlist prompting behavior for read-only commands; speech.test.ts re-imported the module mid-run with a replaced process.env (host-dependent ffmpeg check, TDZ flakes). Both rewritten to the intended contract, host-independent, static imports.

Review feedback (both threads resolved) — the daily-budget rejects.toThrow assertion is now awaited (79d2824); the write-only sawToolCalls variable was removed from the stream handler. The pull-requests page's module-scope setInterval (which leaks a timer into SSR/build workers) was replaced with an effect-scoped hook (e3e0098).

Verification

Run with bun 1.2.21 (CI-pinned) and bun install --frozen-lockfile:

  • turbo typecheck: 18/18 packages pass
  • turbo lint: 17/17 packages pass
  • bun test (root): 288 pass / 0 fail / 0 error
  • next build --webpack (apps/web): compiles cleanly (✓); the only local build error is a pre-existing, env-dependent Pinecone config requirement in /api/reviews/trigger and /api/webhooks/github (untouched by this PR)
  • Vercel: supercli, supercli-client, and supercli-docs deployments all successful on the head commit (e3e0098)
  • GitHub Actions: the three jobs do not start — every run on this repo (including main) fails immediately with "The job was not started because your account is locked due to a billing issue." This is a GitHub account billing lock on the repo owner's account (re-confirmed on run 358, e3e0098); only the account owner can clear it. The branch's code-side verification above is green with the exact CI toolchain, so the checks should pass once the lock is lifted.

Open in Hoplite

Adds focused tests for the riskiest untested production paths in the CLI
server, and fixes a bug they surfaced.

- openai-compatible-stream: 28 tests for the SSE handler shared by every
  provider route (chunked content, reasoning deltas, streamed tool_calls
  assembly, dedup, error events, usage, non-streaming fallback, finish).
  Fixes a premature per-read flush that dropped tool calls whose arguments
  arrived across multiple SSE reads, returning a bogus empty-response
  error instead of executing the tool.
- pricing: cost math, alias/fallback lookup, provider labels.
- credit-meter: credit cost multipliers and soft-gate deduction paths
  (no balance, insufficient funds, decrement, DB fail-open).
- token-budget: device id, daily token budget, opus caps, proxy usage
  aggregation and 48h pruning.
- permission-manager: dangerous-command detection, read-only allowlist,
  session-level override, and agent ruleset precedence.
- .hoplite/settings.json: record bun install as the setup command so fresh
  sandboxes get dependencies before tests run.

Full suite with these files: 162 pass / 9 fail / 1 error. The 9 failures
and 1 error are pre-existing (permission-prompt test drift vs the read-only
allowlist, macOS/env-dependent speech tests, and subscription-check failing
to load because the generated Prisma client is absent without DATABASE_URL);
all 69 new tests pass.

Co-authored-by: Yash Dewasthale <yashdev.yvd@gmail.com>
@vercel

vercel Bot commented Sep 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
supercli Ready Ready Preview Sep 3, 2026 7:16am UTC
supercli-client Ready Ready Preview Sep 3, 2026 7:16am UTC
supercli-docs Ready Ready Preview Sep 3, 2026 7:16am UTC

Request Review

Comment thread apps/supercode-cli/server/src/lib/__tests__/token-budget.test.ts Outdated
Comment thread apps/supercode-cli/server/src/lib/openai-compatible-stream.ts
Fixes the three CI jobs (TypeScript Check, Lint, Test) that have been failing on every run, including on main:
- bun.lock was stale (still referenced the deleted apps/video workspace), so 'bun install --frozen-lockfile' failed in ~2s on every job. Regenerated with the pinned bun 1.2.21 (pure lockfile repair).
- Wire 'bun run db:generate' (new root script) with DATABASE_URL into the typecheck and test jobs: two packages import generated Prisma clients (CLI server -> src/generated, superdesign-db -> prisma/client) that were never generated in CI.
- TypeScript errors (pre-existing): server (todowrite/question strict narrowing, missing MergeConnectorManager methods used by /connectors, test mock typing), cortex-sdk (v2/v3 AI SDK model types unified via GatewayModel plus defensive usage normalization).
- Lint errors (pre-existing): supercode-cli-client (Link, entity escaping, purity-safe skeleton width, useSyncExternalStore for the mounted flag) and apps/web (unescaped entities in marketing pages, explicit any in chart/server-action code, setState-in-effect and purity fixes, motion slot component caching).
- Test drift: permission-prompt tests asserted the pre-allowlist prompting behavior for read-only commands; speech tests re-imported the module mid-run with a replaced process.env (host-dependent ffmpeg check, TDZ flakes). Both rewritten to the intended contract, host-independent, static imports.

Local verification with bun 1.2.21 (CI-pinned) and 'bun install --frozen-lockfile':
- turbo typecheck: 18/18 packages
- turbo lint: 17/17 packages
- bun test: 288 pass, 0 fail, 0 error

Co-authored-by: Yash Dewasthale <yashdev.yvd@gmail.com>
@usehoplite usehoplite Bot changed the title test: cover stream handler, billing, limits, and permission evaluation test: cover stream handler, billing, limits, and permission evaluation; fix CI checks Sep 3, 2026
Review feedback on #288:
- token-budget.test.ts: the daily-budget block test never awaited the
  rejects assertion, so a non-rejection would pass silently. Add await.
- openai-compatible-stream.ts: after removing the in-loop tool-call
  flush, sawToolCalls is write-only. Remove the declaration and all
  assignments (emitToolCall and the two delta branches); flushing still
  happens on finish_reason tool_calls and at EOF.

Co-authored-by: Yash Dewasthale <yashdev.yvd@gmail.com>
The module-scope setInterval for the ticking clock ran when the page module is evaluated during SSR/build, leaking a timer into build workers. Move the interval inside the useNow hook (useEffect + useState) so render stays pure and no module-level side effects exist.

Co-authored-by: Yash Dewasthale <yashdev.yvd@gmail.com>
@usehoplite

usehoplite Bot commented Sep 3, 2026

Copy link
Copy Markdown
Author

Status update on the checks:

Review feedback — both threads (@tembo) are addressed in 79d2824 and resolved: the rejects.toThrow assertion is now awaited, and the write-only sawToolCalls variable was removed.

GitHub Actions (TypeScript Check / Lint / Test) — all three jobs are failing with: "The job was not started because your account is locked due to a billing issue." This is not a code failure: the runners never start, and it affects every run on this repo including main (e.g. run 351). The code-side breakages that used to exist (stale bun.lock failing --frozen-lockfile, missing generated Prisma clients, 30+ TS errors, ~90 lint errors, stale tests) are all fixed and verified locally with the CI-pinned bun 1.2.21: turbo typecheck 18/18, turbo lint 17/17, bun test 288/288. Resolving the billing lock on the GitHub account will unblock the runners — no further code changes are needed for these jobs.

Vercel – supercli (apps/web) — the deployment on 79d2824 failed; I removed the one pattern from my changes that could break a webpack build worker (a module-scope setInterval in the pull-requests page, now an effect-scoped hook in e3e0098). next build --webpack compiles the branch cleanly (✓ 48s); the only local build failure is a pre-existing, env-dependent Pinecone requirement in /api/reviews/trigger and /api/webhooks/github (untouched by this PR, last modified in 1378aaf). e3e0098 retriggers the Vercel deployment.

@yashdev9274

Copy link
Copy Markdown
Owner

🤖 Supercode AI Review

Summary

This PR fixes a broken CI pipeline (stale bun.lock referencing the deleted apps/video workspace), adds 69 new tests covering the stream handler, billing, token budgets, and permission evaluation, and fixes a real production bug in streamOpenAICompatibleChat where pending tool calls were flushed and cleared after every SSE read (dropping multi-read tool call assembly). Supporting lint/TypeScript fixes across the monorepo are included to get all three CI jobs green.

Walkthrough

  • CI / lockfile: Regenerate bun.lock stripping the stale apps/video subtree; add db:generate step to the typecheck job (already existed in test job).
  • Production bug fix: openai-compatible-stream.ts — remove the in-loop flushPending() call that cleared pendingToolCalls after every read, silently dropping fragmented tool-call arguments.
  • New test suites: openai-compatible-stream.test.ts (28 cases), permission-rules.test.ts (11), token-budget.test.ts (12), credit-meter.test.ts (10), pricing.test.ts (8).
  • MergeConnectorManager: Implement missing connect, disconnect, getConnectorList, setConfig, setupInstructions methods required by the /connectors route.
  • cortex-sdk gateways: Unify AI SDK model types via GatewayModel + defensive normalization across 8 gateway files.
  • React purity fixes: useSyncExternalStore-based useMounted hook (web + CLI client), module-scope SKELETON_WIDTHS array replacing Math.random() in useMemo, motionComponentCache for Framer Motion slot components, effect-scoped useNow hook replacing module-scope setInterval in pull-requests page, sidebar pathname-change handled during render instead of in an effect.
  • TypeScript / lint: Replace any annotations across analytics.ts, index.ts, github.ts, contribution-graph.tsx, analytics-chart-card.tsx, data/page.tsx; HTML entity escaping across 10+ marketing pages; <a><Link> in CLI client.
  • speech.test.ts: Rewrite to static imports, env-key save/restore pattern, and a stub ffmpeg (/bin/echo) to avoid host-dependent checks.
  • permission-prompt.test.ts: Update commands to ones that actually trigger the prompt path (non-allowlisted write commands vs. previously allowlisted read-only commands).

Changes table

File Summary
.github/workflows/ci.yml Add db:generate step before typecheck; label existing step in test job
.hoplite/settings.json New Hoplite agent config (preview port, setup script)
apps/supercode-cli/client/app/studio/page.tsx <a><Link>, HTML entity escapes for apostrophes
apps/supercode-cli/client/components/auth/login-form.tsx <a><Link>
apps/supercode-cli/client/components/auth/particle-background.tsx useSyncExternalStore for mounted flag; extract generateParticles
apps/supercode-cli/client/components/ui/sidebar.tsx Module-scope SKELETON_WIDTHS; useId-based deterministic width
apps/supercode-cli/server/src/connectors/mergedev.ts Implement connect, disconnect, getConnectorList, setConfig, setupInstructions
apps/supercode-cli/server/src/lib/__tests__/credit-meter.test.ts New: 10 tests for credit cost math and deduction paths
apps/supercode-cli/server/src/lib/__tests__/model-access.test.ts Fix mock.module call typing
apps/supercode-cli/server/src/lib/__tests__/openai-compatible-stream.test.ts New: 28 tests for SSE handler
apps/supercode-cli/server/src/lib/__tests__/pricing.test.ts New: 8 tests for cost math and provider labels
apps/supercode-cli/server/src/lib/__tests__/token-budget.test.ts New: 12 tests for daily budget, opus cap, proxy usage
apps/supercode-cli/server/src/lib/openai-compatible-stream.ts Remove premature in-loop flushPending() and sawToolCalls variable
apps/supercode-cli/server/src/tools/__tests__/permission-prompt.test.ts Update commands to ones that actually trigger prompting
apps/supercode-cli/server/src/tools/__tests__/permission-rules.test.ts New: 11 permission evaluation tests
apps/supercode-cli/server/src/tools/definitions/question.ts Narrow item.options! to local options for TS strict safety
apps/supercode-cli/server/src/tools/definitions/todowrite.ts Non-null assertions on indexed todos access
apps/supercode-cli/server/src/voice/__tests__/speech.test.ts Rewrite to static imports, env isolation, stub ffmpeg
apps/web/app/(pages)/case-study/dodo-payments/page.tsx HTML entity escaping (apostrophes, quotes)
apps/web/app/(pages)/changelog/page.tsx HTML entity escaping throughout
apps/web/app/(pages)/code-review/page.tsx Entity escape one apostrophe
apps/web/app/(pages)/data/page.tsx Replace any[] with typed Array<Record<...>> in two props
apps/web/app/(pages)/launch/page.tsx Entity escape one apostrophe
apps/web/app/dashboard/pull-requests/page.tsx Extract useNow hook; pass now to timeAgo
apps/web/components/animate-ui/primitives/animate/slot.tsx Module-scope motionComponentCache replacing per-render motion.create
apps/web/components/dashboard/analytics/analytics-chart-card.tsx any[] → typed array
apps/web/components/dashboard/components/contribution-graph.tsx as anyas React.HTMLAttributes<SVGRectElement>
apps/web/components/dashboard/sidebar.tsx Pathname change handled during render; remove effect
apps/web/components/homepage/beta-countdown-banner.tsx Consolidate to single effect; useMounted for hydration guard
apps/web/components/homepage/changelog-card.tsx // SHIP{"// SHIP"} to avoid lint error
apps/web/components/homepage/faq-section.tsx Same pattern for // faq
apps/web/components/homepage/providers-section.tsx Same pattern for // providers
apps/web/components/ui/sidebar.tsx Module-scope SKELETON_WIDTHS; useId-based width (web copy)
apps/web/hooks/use-mounted.ts Rewrite with useSyncExternalStore
apps/web/modules/dashboard/actions/analytics.ts Replace any with named interfaces; fix merged_at null guard
apps/web/modules/dashboard/actions/index.ts Add ContributionDay/Week/CalendarLike interfaces; remove any
apps/web/modules/github/lib/github.ts Type octokit.graphql response inline instead of as any
apps/web/modules/setings/components/profile-form.tsx Replace useEffect sync with render-time update pattern
bun.lock Remove stale apps/video workspace; drop configVersion
package.json Add root db:generate script
packages/cortex-sdk/src/gateway/base.ts GatewayModel type; defensive usage normalization
packages/cortex-sdk/src/gateway/*.ts (7 files) Switch to GatewayModel type

Findings

  • [high] token-budget.test.ts mocks node:os module but the module under test imports os at load timeapps/supercode-cli/server/src/lib/__tests__/token-budget.test.ts (lines 18–23)

    (mock as any).module("node:os", ...) is registered before await import("../token-budget"), which is correct for Bun's hoisted module mock. However, the homedir mock spreads the live os object at mock-registration time (...os), capturing the real homedir. If the spread happens before Bun installs the mock shim the real homedir will be captured in the spread, and the override will only take effect on the named homedir export — not on os.default.homedir() calls made through the default import in the production module. This is fragile and may silently use ~ on some Bun versions, leaving config-file assertions hitting the real home directory.

    // Safer: don't spread the live os object; only override what you need
    ;(mock as any).module("node:os", () => ({
      homedir: () => TEST_HOME,
      default: { homedir: () => TEST_HOME },
    }))
  • [high] MergeConnectorManager.connect returns a hard-coded "connected" status without actually verifying the MCP endpoint is reachableapps/supercode-cli/server/src/connectors/mergedev.ts (lines 68–85)

    The method constructs a ConnectorSession with status: "connected" the moment getMcpConfig() returns non-null. No HTTP probe or auth check is performed. If MERGE_AH_API_KEY is set to a bogus value the caller will receive a "connected" session that silently fails on first MCP use. This was a stub to satisfy TypeScript, but callers that display or act on the session status will show a false positive.

    At minimum, add a comment making the optimistic assumption explicit, or add a health-check ping and set status to "pending" until verified.

  • [medium] credit-meter.test.ts asserts exact fractional cent values that depend on undocumented rounding in production codeapps/supercode-cli/server/src/lib/__tests__/credit-meter.test.ts (lines 31–32)

    expect(getCreditCost("deepseek-v4-flash")).toBe(0.25) // 1.0 / 4x
    expect(getCreditCost("MiniMax-M3")).toBe(0.56)        // 1.5 / 2.7

    toBe on floating-point results is brittle. 0.56 is Math.round(1.5 / 2.7 * 100) / 100, but IEEE 754 could produce 0.5599999… or 0.5600000…1 depending on the rounding path, causing intermittent failures across Bun/Node versions or if the pricing catalog changes.

    expect(getCreditCost("MiniMax-M3")).toBeCloseTo(0.56, 2)
  • [medium] openai-compatible-stream.test.ts [DONE] behavior assertion is semantically invertedapps/supercode-cli/server/src/lib/__tests__/openai-compatible-stream.test.ts (lines 97–110)

    The test comment says "stops processing a read batch at [DONE] but continues reading the stream", and asserts result.fullContent equals "Hellonext batch". This means the batch after [DONE] (containing "next batch") is processed — which contradicts the description "stops at [DONE]". If the intent is that [DONE] only terminates the current read's line processing and the next read() call starts fresh, the test name and comment need to be corrected to match the actual contract so future maintainers don't misread the behavior.

  • [medium] sidebar.tsx render-time state update uses object identity comparison, which breaks for reference-type profileapps/web/modules/setings/components/profile-form.tsx (lines 55–61)

    const [profileVersion, setPrevPathname] = useState(profile)
    if (profile && profile !== profileVersion) {

    This pattern (derived state via render-time setState) is the React-recommended alternative to useEffect for sync. However, profile here comes from a useQuery result — React Query may return a new object reference on every successful fetch even if the data hasn't changed (especially with staleTime: 0). This would cause name/email state to reset to server values on every re-render that re-fetches, potentially clobbering in-flight user edits. A usePrevious/useRef pattern keyed on a stable ID field (e.g., profile.id or a hash) would be safer.

    const profileId = profile?.id ?? null
    const [syncedId, setSyncedId] = useState(profileId)
    if (profileId && profileId !== syncedId) {
      setSyncedId(profileId)
      setName(profile!.name || "")
      setEmail(profile!.email || "")
    }
  • [medium] motionComponentCache is module-scope and never cleared, creating a memory leak for dynamic element types in long-lived SSR workersapps/web/components/animate-ui/primitives/animate/slot.tsx (lines 62–69)

    The Map grows unboundedly for each distinct React element type that passes through Slot. In a Next.js environment with many page navigations or RSC re-renders, this is unlikely to be a practical problem (element types are stable), but the eslint-disable comment on line 94 (// eslint-disable-next-line react-hooks/static-components) suggests this was flagged by the linter and silenced rather than addressed structurally. The comment is correct as a workaround, but should include a WeakRef or a maximum-size cap if the component set is open-ended.

  • [low] useNow in pull-requests/page.tsx initializes with Date.now() during SSR, producing a hydration mismatchapps/web/app/dashboard/pull-requests/page.tsx (lines 44–51)

    const [now, setNow] = useState(() => Date.now())

    Date.now() returns different values on the server and client, so the rendered timeAgo strings will differ on hydration. Since this is a "use client" component the SSR output is used for the initial paint; the mismatch causes a React hydration warning. Initializing with 0 and setting on mount (or using useSyncExternalStore with a server snapshot of 0) avoids this.

    const [now, setNow] = useState(0)
    useEffect(() => {
      setNow(Date.now())
      const id = setInterval(() => setNow(Date.now()), 30_000)
      return () => clearInterval(id)
    }, [])
  • [low] ConnectorEntry and ConnectorSession types are imported from ./types.ts but that file is not shown in the diffapps/supercode-cli/server/src/connectors/mergedev.ts (line 2)

    The import from "./types.ts" may not exist or may not export these shapes, which would cause a TypeScript error at build time in the server package. Confirm ConnectorEntry and ConnectorSession are exported from that path.

  • [low] FFMPEG_STUB = "/bin/echo" in speech.test.ts is POSIX-only; CI on Windows would failapps/supercode-cli/server/src/voice/__tests__/speech.test.ts (line 10)

    The test is inside the supercode-cli server which targets Node/Bun on Linux/macOS, so this is likely acceptable. But it should be called out: the stub will break on Windows runners. Add a if (process.platform === "win32") skip() guard or use process.execPath (always present).

  • [nit] bun.lock still contains configVersion removal as a diff hunkbun.lock (line 3)

    configVersion: 1 was removed. This is correct (Bun 1.2.x dropped this field), but if the project ever downgrades Bun below the version that introduced this field, bun install --frozen-lockfile will fail again. Worth a comment in CONTRIBUTING.md noting the minimum Bun version is 1.2.21.

  • [nit] (mock as any).module(...) cast in test files is a workaround for a Bun typing issueapps/supercode-cli/server/src/lib/__tests__/credit-meter.test.ts (line 11), model-access.test.ts (line 4)

    The cast suggests mock.module is not typed on the mock export from bun:test. This is a known Bun issue. Consider adding a // @ts-expect-error bun:test mock.module not typed comment instead of as any to make the suppression intention explicit and get a build error if Bun fixes the typing.


Risk assessment

Medium — The stream handler bug fix is a real behavioral change in a hot path (SSE tool-call assembly shared by 6+ provider routes); the fix is directionally correct and regression-tested, but any edge case in flushPending at EOF now rests entirely on the end-of-stream path. The MergeConnectorManager stub methods are newly introduced with optimistic status reporting. Everything else is test coverage, lint/type fixes, and lockfile hygiene with no schema or auth changes.


Test plan

  • Run bun install --frozen-lockfile with Bun 1.2.21; confirm it completes without workspace errors.
  • Run bun run db:generate at the repo root; confirm both apps/supercode-cli/server/src/generated and packages/superdesign-db/prisma/client are populated.
  • Run turbo typecheck; confirm 18/18 packages pass with zero errors.
  • Run turbo lint; confirm 17/17 packages pass.
  • Run bun test; confirm 288 pass, 0 fail, particularly the new suites: openai-compatible-stream.test.ts, permission-rules.test.ts, token-budget.test.ts, credit-meter.test.ts, pricing.test.ts.
  • Manually trigger a multi-step tool call (e.g., run_command with a long argument) via the CLI to confirm arguments are no longer silently dropped across SSE reads.
  • Verify the pull-requests page loads without React hydration warnings in the browser console.
  • Confirm the // SHIP, // faq, // providers comment strings render correctly (not as JSX comment syntax) in the marketing pages.
  • Check that MergeConnectorManager.connect is not relied upon for actual MCP session health in any existing route that would act on status: "connected".

Suggested PR description

What

Fixes failing CI on this repo (and main) caused by a stale bun.lock that still referenced the deleted apps/video workspace, causing bun install --frozen-lockfile to fail in ~2 seconds on every run. Also fixes a production bug in the SSE stream handler and adds comprehensive test coverage for the riskiest untested paths.

Why

  • Every CI run was dead on arrival due to the lockfile issue — TypeScript, lint, and test jobs never reached their actual work.
  • streamOpenAICompatibleChat was flushing and clearing pendingToolCalls after every SSE read, silently dropping tool-call arguments that arrived across multiple reads and returning a bogus "empty response" error.
  • The stream handler, billing/credit, token budget, and permission evaluation code had zero test coverage.

Changes

CI / lockfile

  • Regenerate bun.lock with Bun 1.2.21 (removes stale apps/video subtree and configVersion).
  • Add root db:generate script; run before typecheck and test jobs.

Bug fix

  • openai-compatible-stream.ts: Remove in-loop flushPending() that cleared pending tool calls after every read. EOF flush still handles [DONE] and missing finish_reason: "tool_calls". Covered by regression test.

Tests (69 new)

  • openai-compatible-stream.test.ts — chunk boundaries, reasoning deltas, streamed tool-call assembly, dedup, error events, usage, embedded MiniMax calls, non-streaming fallback, writeFinish
  • permission-rules.test.ts — dangerous-command detection, read-only allowlist, session override, agent precedence
  • token-budget.test.ts — daily budget (DB + proxy), opus caps, 48h pruning, device ID
  • credit-meter.test.ts — credit cost math, soft-gate deduction paths
  • pricing.test.ts — cost math, alias lookup, provider labels

Pre-existing fixes for CI green

  • TypeScript: question.ts/todowrite.ts strict narrowing; MergeConnectorManager missing interface methods; cortex-sdk model type unification.
  • Lint: HTML entity escaping in marketing pages, <a><Link>, useSyncExternalStore mounted flag, module-scope skeleton widths, motion slot component caching, effect-scoped useNow hook.
  • Tests: permission-prompt.test.ts updated to use commands that actually trigger prompting; speech.test.ts rewritten to static imports with env isolation.

How tested

  • turbo typecheck: 18/18 packages pass
  • turbo lint: 17/17 packages pass
  • bun test: 288 pass / 0 fail
  • next build --webpack (apps/web): clean
  • Vercel deployments for supercli, supercli-client, supercli-docs: successful on head commit

Automated review by Supercode · leave a 👍/👎 reaction to rate this review

Applies the actionable items from the Supercode review comment:

- stream test: [DONE] only cuts off the current read batch's remaining
  lines (next read continues, trailing bytes drained at EOF); rename the
  test and comment to match that actual contract instead of implying it
  stops the whole stream.
- credit-meter.test: use toBeCloseTo for the fractional MiniMax cost so
  IEEE-754 rounding across engines can't fail it.
- profile-form: sync name/email on a stable profile.id instead of object
  identity, so a refetched profile object can't clobber in-flight edits.
- pull-requests page: useNow via useSyncExternalStore with a server
  snapshot of 0 (no SSR/client Date.now() hydration mismatch); timeAgo
  renders a placeholder until mounted.
- mergedev.connect: document that the session status is optimistic and
  real MCP health is verified by McpManager.reconnectServer (which
  throws on failure).

Declined after verification: the node:os mock spread and ./types.ts
imports are already correct (types.ts exports both shapes; the mock
overrides both named and default homedir), mock.module is genuinely
untyped in the resolved bun:test types so the cast stays, and the
ffmpeg stub + motion cache concerns don't apply to the Linux-only CI
and bounded element-type key set.

Co-authored-by: Yash Dewasthale <yashdev.yvd@gmail.com>
@usehoplite

usehoplite Bot commented Sep 3, 2026

Copy link
Copy Markdown
Author

Thanks for the thorough review — I worked through each finding. Addressed in ecf13cc, with verification notes for the ones I declined:

Applied

  • [DONE] test semantics — Confirmed against the handler: [DONE] breaks the current read batch's line loop only; the outer while keeps reading and trailing bytes are drained at EOF, so later-chunk events ARE processed. Renamed the test and rewrote its comment to describe that contract accurately.
  • credit-meter.test.tstoBe(0.56)toBeCloseTo(0.56, 2).
  • profile-form.tsx render-time sync — Now keyed on stable profile.id instead of object identity, so a refetched object can't reset in-flight edits.
  • useNow hydration mismatch — Rewritten with useSyncExternalStore and a server snapshot of 0 (server and client render the same initial value; timeAgo shows a placeholder until mounted).
  • MergeConnectorManager.connect optimistic status — Added a comment documenting the optimistic status. Real MCP health is verified by McpManager.reconnectServer in the only caller (slashCommands/connectors.ts), which throws when the server fails to start; I checked nothing acts on status: "connected" for health.

Verified and declined (with evidence)

  • token-budget.test.ts node:os mock — The factory overrides both the named homedir export and default.homedir, so os.default.homedir() resolves to TEST_HOME. The spread is required: mock.module is process-global across the test run, and dropping it breaks other node:os consumers (e.g. is-wsl). The config-file assertions land in the temp dir and pass consistently on the CI-pinned bun 1.2.21.
  • mergedev.ts types import./types.ts exists and exports both ConnectorEntry and ConnectorSession; server typecheck passes with the import as-is.
  • (mock as any).module castmock.module is genuinely untyped on the mock export in the bun:test types this package resolves: removing the cast produces TS2339: Property 'module' does not exist, so the cast is required. I verified this directly rather than swapping to @ts-expect-error, which would be fragile here.
  • FFMPEG_STUB = "/bin/echo" — CI runs ubuntu-latest only and the server targets Linux/macOS, so the win32 guard would be dead code here; noted for future Windows runners.
  • motionComponentCache — Keys are stable module-level element-type references from a closed set, so the Map is bounded in practice; agreed it's a workaround, but no realistic leak in this codebase.
  • bun.lock configVersionci.yml pins bun-version: 1.2.21, which makes the requirement explicit and prevents a downgrade-based regression.

Verification after the changes: server tsc --noEmit clean, web tsc --noEmit clean (after clearing a stale gitignored .next, unrelated to code), web lint 0 errors, root bun test 288/288. Committed as ecf13cc, pushed to the head branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant