Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
68 changes: 51 additions & 17 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
- 'release/**'
merge_group:
workflow_dispatch:
# Manual trigger - no inputs needed, uses repository variables

Check warning on line 16 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

16:5 [comments-indentation] comment not indented like content

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
Expand Down Expand Up @@ -247,14 +247,14 @@
- name: 'Install shellcheck' # Actionlint uses shellcheck
run: |-
mkdir -p "${RUNNER_TEMP}/shellcheck"
curl -sSLo "${RUNNER_TEMP}/.shellcheck.txz" "https://github.com/koalaman/shellcheck/releases/download/v${SHELLCHECK_VERSION}/shellcheck-v${SHELLCHECK_VERSION}.linux.x86_64.tar.xz"

Check warning on line 250 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

250:121 [line-length] line too long (189 > 120 characters)
tar -xf "${RUNNER_TEMP}/.shellcheck.txz" -C "${RUNNER_TEMP}/shellcheck" --strip-components=1
echo "${RUNNER_TEMP}/shellcheck" >> "${GITHUB_PATH}"

- name: 'Install actionlint'
run: |-
mkdir -p "${RUNNER_TEMP}/actionlint"
curl -sSLo "${RUNNER_TEMP}/.actionlint.tgz" "https://github.com/rhysd/actionlint/releases/download/v${ACTIONLINT_VERSION}/actionlint_${ACTIONLINT_VERSION}_linux_amd64.tar.gz"

Check warning on line 257 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

257:121 [line-length] line too long (184 > 120 characters)
tar -xzf "${RUNNER_TEMP}/.actionlint.tgz" -C "${RUNNER_TEMP}/actionlint"
echo "${RUNNER_TEMP}/actionlint" >> "${GITHUB_PATH}"

Expand All @@ -264,7 +264,7 @@
run: |-
actionlint \
-color \
-format "{{range \$err := .}}::error file={{\$err.Filepath}},line={{\$err.Line}},col={{\$err.Column}}::{{\$err.Filepath}}@{{\$err.Line}} {{\$err.Message}}%0A\`\`\`%0A{{replace \$err.Snippet \"\\\\n\" \"%0A\"}}%0A\`\`\`\\n{{end}}" \

Check warning on line 267 in .github/workflows/ci.yml

View workflow job for this annotation

GitHub Actions / Lint (YAML)

267:121 [line-length] line too long (243 > 120 characters)
-ignore 'SC2002:' \
-ignore 'SC2016:info' \
-ignore 'SC2129:' \
Expand Down Expand Up @@ -412,15 +412,27 @@
run: |-
npm run lint:affected-shards

# Build must run BEFORE the type-aware lint: several workspace tsconfigs
# map cross-workspace imports to `dist/*.d.ts` (e.g. storage, settings),
# so tsserver (projectService) needs the compiled declarations on disk to
# resolve types. Under npm `npm ci`'s postinstall builds implicitly; under
# `bun install` (whose postinstall only symlinks, see scripts/postinstall.cjs)
# it does not, so the build is explicit here.
- name: 'Build project'
# Build must run BEFORE the type-aware lint: three workspace tsconfigs
# map cross-workspace imports to `dist/*.d.ts` — cli -> tools, core -> mcp,
# and a2a-server -> settings/storage/tools — so tsserver (projectService)
# needs the compiled declarations on disk to resolve types. Under npm
# `npm ci`'s postinstall builds implicitly; under `bun install` (whose
# postinstall only symlinks, see scripts/postinstall.cjs) it does not, so
# the build is explicit here.
#
# `build:types` emits declarations only (issue #2983). Every step in this
# job reads types and none executes application code, so the transpiled
# `.js` was pure cost here. Do not reuse `build:types` in a job that runs
# the CLI or a Bun test suite: Bun honors tsconfig `paths`, so a
# declaration-only `dist` makes cross-package imports resolve to a
# `.d.ts` with no JavaScript behind it. The release path
# (release.yml -> `npm run build:packages`) still does a full emit,
# because every published library workspace declares `main:
# dist/index.js` and ships `dist`, so npm consumers resolving without the
# `bun` export condition load that JavaScript.
- name: 'Build declarations for type-aware lint'
run: |-
npm run build
npm run build:types

- name: 'Run agents API-surface guard'
run: |-
Expand Down Expand Up @@ -1008,18 +1020,40 @@
run: |-
bun install
git checkout -- bun.lock
- name: 'Build project'
# Issue #2983: tests never read compiled output. Every workspace declares
# a `bun` export condition resolving to TypeScript source, and
# packages/cli/vitest.config.ts aliases cross-workspace imports straight
# at source, so a full build here produced JavaScript nothing loaded.
#
# Two shards are the exception. Both run the agents API-surface guard
# (`scripts/check-agents-api-surface.ts`) — the `agents` shard through its
# package `pretest` hook, the `scripts` shard through
# scripts/tests/check-agents-api-surface.test.ts. That guard's temp
# tsconfig resolves `@vybestack/llxprt-code-{telemetry,mcp}` and several
# `storage/*` subpaths through node_modules to `dist/*.d.ts`, because
# packages/agents/tsconfig.json has no source mapping for them. Issue
# #2618 (tsconfig bypasses) owns repointing those mappings at source;
# until it lands, these two legs need a built workspace.
#
# These legs must run the FULL build, not `build:types`. Bun applies
# tsconfig `paths` at runtime, so `packages/core/tsconfig.json`'s
# `@vybestack/llxprt-code-mcp -> ../mcp/dist/mcp/index.d.ts` mapping wins
# over the package's `bun` export condition whenever that file exists. A
# declaration-only `dist` therefore resolves to a `.d.ts` whose relative
# re-export has no emitted JavaScript behind it, and every test importing
# core through mcp dies at import time. Either a complete `dist` or no
# `dist` at all works; a partial one does not. `build:types` is confined
# to `lint_javascript`, which runs no application code.
#
# The standalone `Run agents API-surface guard` step is gone: the
# `lint_javascript` job already runs it after its own declaration build,
# and the `agents` shard runs it through the agents package `pretest`
# hook.
- name: 'Build project (agents API-surface guard prerequisite)'
if: matrix.shard == 'agents' || matrix.shard == 'scripts'
run: |-
npm run build

- name: 'Run agents API-surface guard'
# Only the agents shard needs this guard (it inspects the agents
# package's public API surface). Running it on every shard wastes CI
# time on shards that don't exercise the agents package.
if: matrix.shard == 'agents'
run: |-
npm run lint:agents-api-surface

- name: 'Verify test shard completeness (issue #2707)'
# Fails if any workspace declared in package.json is missing from the
# shard map, so adding a package can never silently go untested.
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"docs:settings": "bun ./scripts/generate-settings-doc.ts",
"docs:keybindings": "bun ./scripts/generate-keybindings-doc.ts",
"build": "bun scripts/build.ts",
"build:types": "cross-env LLXPRT_EMIT_DECLARATIONS_ONLY=1 npm run build",
"preinstall": "node scripts/preinstall.cjs",
"postinstall": "node scripts/postinstall.cjs",
"build-and-start": "npm run build && npm run start",
Expand Down
26 changes: 8 additions & 18 deletions packages/auth/src/__tests__/package-boundary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,21 +427,11 @@ describe('Auth workspace DAG constraints', () => {
// ─────────────────────────────────────────────────────────────────
// Build artifact checks
// ─────────────────────────────────────────────────────────────────

describe('Auth package build artifacts', () => {
/**
* @plan:PLAN-20260608-ISSUE1586.P04
* @requirement:REQ-AUTH-001.3
*/
it('dist/index.js exists', () => {
expect(fs.existsSync(path.join(AUTH_DIR, 'dist', 'index.js'))).toBe(true);
});

/**
* @plan:PLAN-20260608-ISSUE1586.P04
* @requirement:REQ-AUTH-001.3
*/
it('dist/index.d.ts exists', () => {
expect(fs.existsSync(path.join(AUTH_DIR, 'dist', 'index.d.ts'))).toBe(true);
});
});
//
// Removed by issue #2983. These asserted that `dist/index.js` and
// `dist/index.d.ts` were present on disk, which only held because the test
// job compiled the workspace first. Tests resolve TypeScript source through
// the `bun` export condition and never read `dist`, so the assertions
// measured the CI harness rather than this package. REQ-AUTH-001.3's
// published contract stays pinned by the `main`, `types`, and `exports`
// assertions above.
35 changes: 31 additions & 4 deletions packages/cli/src/integration-tests/cli-args-test-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,20 @@

import { spawn } from 'child_process';
import * as path from 'path';
import { fileURLToPath } from 'node:url';

/**
* The CLI's real entry point. Nothing compiles this workspace before tests run
* (issue #2983), so spawning `node dist/index.js` had no target; `index.ts` is
* what the installed launcher and `npm run start` execute anyway.
*/
const CLI_ENTRY = path.resolve(
fileURLToPath(import.meta.url),
'..',
'..',
'..',
'index.ts',
);

export type CliRunResult = {
stdout: string;
Expand Down Expand Up @@ -47,10 +61,10 @@ export async function runCli(
env.LLXPRT_CONFIG_HOME ?? process.env.LLXPRT_CONFIG_HOME ?? '';

return new Promise((resolve) => {
// Use the compiled CLI entry point
const cliPath = path.join(process.cwd(), 'dist', 'index.js');

const child = spawn('node', [cliPath, ...args], {
// `process.execPath` is the Bun binary running this suite — the CLI
// workspace executes Bun-native (issue #2843) — which is exactly the
// runtime the shipped launcher execs `index.ts` with.
const child = spawn(process.execPath, [CLI_ENTRY, ...args], {
env: {
...process.env,
// The CI test step injects real provider credentials. These cases
Expand Down Expand Up @@ -118,6 +132,19 @@ export async function runCli(
});
}, 60_000);

// A spawn failure (e.g. a moved or deleted CLI entry) emits 'error' and
// then 'close' with a null exit code. Settling here keeps the spawn
// message; the later 'close' cannot change an already-settled promise.
// Without this the case would report a bare exit code with no cause.
child.on('error', (error: Error) => {
clearTimeout(timeout);
resolve({
stdout,
stderr: `${stderr}Failed to spawn ${CLI_ENTRY}: ${error.message}`,
exitCode: -1,
});
});

Comment thread
acoliver marked this conversation as resolved.
child.on('close', (code) => {
clearTimeout(timeout);
resolve({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { spawn } from 'child_process';
import * as path from 'path';
import { fileURLToPath } from 'node:url';
import { Storage } from '@vybestack/llxprt-code-storage';
import * as fs from 'fs/promises';
import type {
Expand All @@ -26,15 +27,29 @@ interface CliResult {
exitCode: number;
}

/**
* The CLI's real entry point. Nothing compiles this workspace before tests run
* (issue #2983), so spawning `node dist/index.js` had no target; `index.ts` is
* what the installed launcher and `npm run start` execute anyway.
*/
const CLI_ENTRY = path.resolve(
fileURLToPath(import.meta.url),
'..',
'..',
'..',
'index.ts',
);

async function runCli(
args: string[],
env: Record<string, string> = {},
input?: string,
): Promise<CliResult> {
return new Promise((resolve) => {
const cliPath = path.join(process.cwd(), 'dist', 'index.js');

const child = spawn('node', [cliPath, ...args], {
// `process.execPath` is the Bun binary running this suite — the CLI
// workspace executes Bun-native (issue #2843) — which is exactly the
// runtime the shipped launcher execs `index.ts` with.
const child = spawn(process.execPath, [CLI_ENTRY, ...args], {
env: {
...process.env,
...env,
Expand Down Expand Up @@ -78,6 +93,19 @@ async function runCli(
});
}, 60_000);

// A spawn failure (e.g. a moved or deleted CLI entry) emits 'error' and
// then 'close' with a null exit code. Settling here keeps the spawn
// message; the later 'close' cannot change an already-settled promise.
// Without this the case would report a bare exit code with no cause.
child.on('error', (error: Error) => {
clearTimeout(timeout);
resolve({
stdout,
stderr: `${stderr}Failed to spawn ${CLI_ENTRY}: ${error.message}`,
exitCode: -1,
});
});

child.on('close', (code) => {
clearTimeout(timeout);
resolve({
Expand Down
Loading
Loading