From 6eb37a774930b5b9edb137edd441e30652dfd394 Mon Sep 17 00:00:00 2001 From: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> Date: Wed, 28 Oct 2026 00:20:07 +0800 Subject: [PATCH] fix(ci): restore qualification and reduce validation overhead Use one typed-checker worker by default without changing semantic scope or caller overrides. Reuse the bounded CLI worker for pure argument validation while retaining fresh-process coverage and all upstream cases. Keep registration tests independent of real package-runner downloads while asserting the actual prewarm command and startup context. Pass the advertised binary_session definition by name to package verification, retaining default SDK validation and complete catalog/contract checks while avoiding compilation of unrelated tool outputs. Preserve all four installed-tarball scenarios and existing deadlines. Strengthen Hermes preview operation/backup assertions and clarify physical-path routing documentation. Assisted-by: Suika (gensokyo-skills:suika-scatter-gather-planning) --- docs/client-configuration.md | 13 +- docs/testing.md | 6 +- scripts/ci/static-check-report.mjs | 3 +- scripts/lib/verify-package-core.mjs | 9 +- .../setup/hermesClientSetup.test.ts | 19 ++- .../boundary/cli/cliIntegerArguments.test.ts | 101 ++++++++------- .../cli/packageRunnerRegistration.test.ts | 69 +++++++++- .../filesystem/staticCheckReports.test.ts | 120 +++++++++++++++++- .../mcp/packageCatalogVerification.test.ts | 31 ++++- .../process/releaseArtifactCanary.test.ts | 16 ++- 10 files changed, 314 insertions(+), 73 deletions(-) diff --git a/docs/client-configuration.md b/docs/client-configuration.md index 0ef0a2c063..420b76273b 100644 --- a/docs/client-configuration.md +++ b/docs/client-configuration.md @@ -527,10 +527,15 @@ the profile Hermes actually uses and point `HERMES_HOME` directly at that `profiles/` directory before setup. Compatibility is tracked in [#3939](https://github.com/morluto/rea/issues/3939). -REA resolves existing ancestors when checking containment, including symlinks -and junctions, even if the final override directory is missing. An unresolved -home path is reported instead of guessing its location. Inspect setup's -previewed destinations before applying changes. +REA resolves existing ancestor directories before checking physical containment, +including symlinks and junctions, even when the override directory does not exist. +An override beneath a symlinked default home therefore uses the default home's +sticky selection. If `~/.hermes/link` points outside the default home, +`HERMES_HOME=~/.hermes/link/new` instead uses that override root, whether or not +`new` exists. Previewed destinations retain the configured path spelling. +An unreadable path or dangling alias leaves the home unresolved; repair that path +before configuring Hermes. Inspect setup's previewed destinations before applying +changes. An explicit `HERMES_HOME` pointing to `profiles/` selects that named profile directly, bypassing sticky selection. An invalid, missing or deleted diff --git a/docs/testing.md b/docs/testing.md index 5e76ff0dba..340cf17daf 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -210,7 +210,11 @@ changing a callback solely to satisfy its declared `void` return type. Ordinary Knip remains. `check`, `check:pr`, and `check:ci:deep` run reachability then typed lint sequentially after cached work. Typed lint builds the project type graph and stays separate from fast feedback. Its default Go settings are -`GOMAXPROCS=2` and `GOMEMLIMIT=1024MiB`; caller-selected settings take precedence. +`GOMAXPROCS=1` and `GOMEMLIMIT=1024MiB`; caller-selected settings take precedence. +One Go worker reuses checker caches across the file queue without changing the +file inventory or TypeScript project scope. The Go memory target is soft and +does not cap the complete process family's memory; measure the full invocation +when enforcing a host budget. Routine iterations and rebases need focused regressions and relevant checks. Before handing off a PR, run `npm run check` and generated-document checks when diff --git a/scripts/ci/static-check-report.mjs b/scripts/ci/static-check-report.mjs index 32a27ab020..5c2d89c952 100644 --- a/scripts/ci/static-check-report.mjs +++ b/scripts/ci/static-check-report.mjs @@ -91,7 +91,8 @@ try { typed ? { ...process.env, - GOMAXPROCS: process.env.GOMAXPROCS ?? "2", + // One worker reuses its checker caches across the full file queue. + GOMAXPROCS: process.env.GOMAXPROCS ?? "1", GOMEMLIMIT: process.env.GOMEMLIMIT ?? "1024MiB", } : process.env, diff --git a/scripts/lib/verify-package-core.mjs b/scripts/lib/verify-package-core.mjs index 297a18e046..e6f06e64da 100644 --- a/scripts/lib/verify-package-core.mjs +++ b/scripts/lib/verify-package-core.mjs @@ -36,9 +36,16 @@ export const verifyCompleteToolCatalog = async (client, options) => { ); } } + const sessionDefinition = listed.tools.find( + ({ name }) => name === "binary_session", + ); + if (sessionDefinition === undefined) + throw new Error("packaged MCP omitted the binary_session definition"); const status = await client.callTool( { name: "binary_session", arguments: {} }, - options, + // The SDK otherwise compiles every listed output schema on the first call. + // Keep its default validation for the actual named tool being exercised. + { ...options, toolDefinition: sessionDefinition }, ); const expanded = requireMcpLifecycleResult(status, "binary_session"); const availability = expanded.tool_availability; diff --git a/tests/acceptance/setup/hermesClientSetup.test.ts b/tests/acceptance/setup/hermesClientSetup.test.ts index 41888d87ed..b0a5a44efd 100644 --- a/tests/acceptance/setup/hermesClientSetup.test.ts +++ b/tests/acceptance/setup/hermesClientSetup.test.ts @@ -532,7 +532,7 @@ for (const selection of [ for (const layout of ["symlinked-default", "escaping-prefix"] as const) { cliTest( - `Hermes preview distinguishes missing and existing overrides: ${layout}`, + `Hermes preview resolves existing ancestors for missing and existing overrides: ${layout}`, async ({ cli }) => { const home = await createTestTempDirectory( "rea-hermes-missing-override-", @@ -577,13 +577,28 @@ for (const layout of ["symlinked-default", "escaping-prefix"] as const) { expect(plan.json).toMatchObject({ status: "planned", plannedActions: expect.arrayContaining([ - expect.objectContaining({ target: join(selected, "config.yaml") }), expect.objectContaining({ + id: "configure_client:hermes", + kind: "configure_client", + target: join(selected, "config.yaml"), + operation: usesNative ? "update" : "create", + ...(usesNative ? { backupPath: `${config}.rea.backup` } : {}), + }), + expect.objectContaining({ + id: "install_skill:hermes", + kind: "install_skill", target: join(selected, "skills", "reverse-engineer-anything"), + operation: "install", }), ]), appliedActions: [], }); + if (!usesNative) + expect(plan.json).not.toMatchObject({ + plannedActions: expect.arrayContaining([ + expect.objectContaining({ backupPath: expect.any(String) }), + ]), + }); if (!exists) await expect(access(root)).rejects.toMatchObject({ code: "ENOENT" }); for (const directory of [native, root, profile]) { diff --git a/tests/boundary/cli/cliIntegerArguments.test.ts b/tests/boundary/cli/cliIntegerArguments.test.ts index bb5ca9ec78..6412c191cb 100644 --- a/tests/boundary/cli/cliIntegerArguments.test.ts +++ b/tests/boundary/cli/cliIntegerArguments.test.ts @@ -1,6 +1,10 @@ import { describe, expect } from "vitest"; -import { cliTest, type CliResult } from "../../support/cli/cliFixture.js"; +import { + cliTest, + runFileCli, + type CliResult, +} from "../../support/cli/cliFixture.js"; const SESSION_ID = "27d3e3f1-f1e5-49ae-91ec-95af1f343a5a"; const NON_INTEGER_TEXT = [ @@ -28,53 +32,7 @@ const expectCliValidationError = (result: CliResult, field: string): void => { }; describe("CLI integer arguments", () => { - cliTest.for(NON_INTEGER_TEXT)( - "rejects %j as a Cutter expected generation", - async (value, { cli }) => { - const result = await cli.run({ - arguments: ["cutter-command", SESSION_ID, value, "pdi", "--json"], - }); - - expectCliValidationError(result, "expectedGeneration"); - }, - ); - - cliTest.for(NON_INTEGER_TEXT)( - "rejects %j as a web module script index", - async (value, { cli }) => { - const result = await cli.run({ - arguments: [ - "trace-web-module-imports", - "/nonexistent/manifest.json", - value, - "--json", - ], - }); - - expectCliValidationError(result, "scriptIndex"); - }, - ); - - cliTest.for(NON_INTEGER_OPTION_TEXT)( - "rejects %j in a repeated web capture record selector", - async (value, { cli }) => { - const result = await cli.run({ - arguments: [ - "inspect-web-network-capture", - "/nonexistent/capture.qlog", - "qlog", - "--record", - "0", - "--record", - value, - "--json", - ], - }); - - expectCliValidationError(result, "record"); - }, - ); - + // Run independent processes before retaining the shared validation worker. cliTest.for(NON_INTEGER_OPTION_TEXT)( "rejects %j as an Android method overload index", async (value, { cli }) => { @@ -170,4 +128,51 @@ describe("CLI integer arguments", () => { expectCliValidationError(result, "line"); }, ); + + cliTest.for(NON_INTEGER_TEXT)( + "rejects %j as a Cutter expected generation", + async (value, { cli }) => { + const result = await runFileCli(cli, { + arguments: ["cutter-command", SESSION_ID, value, "pdi", "--json"], + }); + + expectCliValidationError(result, "expectedGeneration"); + }, + ); + + cliTest.for(NON_INTEGER_TEXT)( + "rejects %j as a web module script index", + async (value, { cli }) => { + const result = await runFileCli(cli, { + arguments: [ + "trace-web-module-imports", + "/nonexistent/manifest.json", + value, + "--json", + ], + }); + + expectCliValidationError(result, "scriptIndex"); + }, + ); + + cliTest.for(NON_INTEGER_OPTION_TEXT)( + "rejects %j in a repeated web capture record selector", + async (value, { cli }) => { + const result = await runFileCli(cli, { + arguments: [ + "inspect-web-network-capture", + "/nonexistent/capture.qlog", + "qlog", + "--record", + "0", + "--record", + value, + "--json", + ], + }); + + expectCliValidationError(result, "record"); + }, + ); }); diff --git a/tests/boundary/cli/packageRunnerRegistration.test.ts b/tests/boundary/cli/packageRunnerRegistration.test.ts index fa1f2796b5..0d3d0c97e5 100644 --- a/tests/boundary/cli/packageRunnerRegistration.test.ts +++ b/tests/boundary/cli/packageRunnerRegistration.test.ts @@ -1,8 +1,9 @@ -import { readFile } from "node:fs/promises"; -import { join } from "node:path"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { delimiter, join } from "node:path"; import { expect } from "vitest"; import { z } from "zod"; import { packageRunnerRegistrationCommand } from "../../../src/application/ClientRegistrationIdentity.js"; +import { PRODUCT_IDENTITY } from "../../../src/identity.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest } from "../../support/cli/cliFixture.js"; @@ -12,7 +13,47 @@ const ompConfigurationSchema = z.object({ }), }); +const controlledRunnerFixture = async (home: string) => { + const bin = join(home, "bin"); + const script = join(home, "runner-fixture.cjs"); + const log = join(home, "runner-calls.jsonl"); + await mkdir(bin); + await writeFile(log, ""); + await writeFile( + script, + `const { appendFileSync } = require("node:fs"); +appendFileSync(process.env.REA_TEST_RUNNER_LOG, JSON.stringify({ + runner: process.argv[2], + arguments: process.argv.slice(3), + home: process.env.HOME, + npm_command: process.env.npm_command, + user_agent: process.env.npm_config_user_agent, +}) + "\\n"); +console.log("controlled runner fixture"); +`, + ); + for (const runner of ["npx", "bunx"]) { + await writeFile( + join(bin, process.platform === "win32" ? `${runner}.cmd` : runner), + process.platform === "win32" + ? `@echo off\r\n"%REA_TEST_NODE%" "%REA_TEST_RUNNER_SCRIPT%" ${runner} %*\r\n` + : `#!/bin/sh\nexec "$REA_TEST_NODE" "$REA_TEST_RUNNER_SCRIPT" ${runner} "$@"\n`, + { mode: 0o700 }, + ); + } + return { + log, + environment: { + PATH: [bin, process.env.PATH ?? ""].join(delimiter), + REA_TEST_NODE: process.execPath, + REA_TEST_RUNNER_SCRIPT: script, + REA_TEST_RUNNER_LOG: log, + }, + }; +}; + // Package runners set these variables for the bin they launch from a temporary cache. +// Only the prewarm child is controlled; each CLI still starts with its runner identity. for (const { runner, userAgent } of [ { runner: "npx", userAgent: "npm/10.9.0 node/v24.11.0 linux x64" }, { runner: "bunx", userAgent: "bun/1.4.2 npm/? node/v26.3.0 linux x64" }, @@ -21,11 +62,13 @@ for (const { runner, userAgent } of [ `${runner} setup registers the pinned ${runner} launcher instead of its cache`, async ({ cli }) => { const home = await createTestTempDirectory("rea-package-runner-"); + const controlledRunner = await controlledRunnerFixture(home); const run = (arguments_: readonly string[]) => cli.run({ arguments: arguments_, cwd: home, environment: { + ...controlledRunner.environment, HOME: home, USERPROFILE: home, npm_command: "exec", @@ -42,7 +85,10 @@ for (const { runner, userAgent } of [ "--json", ]); expect(setup.exitCode).toBe(0); - expect(setup.json).toMatchObject({ status: "ready" }); + expect(setup.json).toMatchObject({ + status: "ready", + prewarm: { status: "warmed" }, + }); const configuration = ompConfigurationSchema.parse( JSON.parse( await readFile(join(home, ".omp", "agent", "mcp.json"), "utf8"), @@ -65,6 +111,23 @@ for (const { runner, userAgent } of [ "--json", ]); expect(repeat.json).toMatchObject({ plannedActions: [] }); + const calls = (await readFile(controlledRunner.log, "utf8")) + .trim() + .split("\n") + .map((line) => JSON.parse(line)); + expect(calls).toEqual([ + { + runner, + arguments: [ + ...(runner === "npx" ? ["-y"] : []), + PRODUCT_IDENTITY.registrationPackageSpecifier, + "--version", + ], + home, + npm_command: "exec", + user_agent: userAgent, + }, + ]); }, ); } diff --git a/tests/boundary/filesystem/staticCheckReports.test.ts b/tests/boundary/filesystem/staticCheckReports.test.ts index a04f86142f..67c0e8297d 100644 --- a/tests/boundary/filesystem/staticCheckReports.test.ts +++ b/tests/boundary/filesystem/staticCheckReports.test.ts @@ -1,6 +1,6 @@ import { spawnSync } from "node:child_process"; import { access, chmod, mkdir, readFile, writeFile } from "node:fs/promises"; -import { dirname, join, sep } from "node:path"; +import { delimiter, dirname, join, sep } from "node:path"; import { fileURLToPath } from "node:url"; import { expect, it } from "vitest"; import { z } from "zod"; @@ -27,12 +27,20 @@ const fixture = async () => { ); return root; }; -const invoke = (root: string, mode: string, ...arguments_: string[]) => +const invokeWithEnvironment = ( + root: string, + mode: string, + environment: NodeJS.ProcessEnv, + ...arguments_: string[] +) => spawnSync(process.execPath, [script, mode, ...arguments_], { cwd: root, encoding: "utf8", timeout: 15_000, + env: environment, }); +const invoke = (root: string, mode: string, ...arguments_: string[]) => + invokeWithEnvironment(root, mode, process.env, ...arguments_); const lintDiagnostics = async (root: string) => z .object({ @@ -145,6 +153,114 @@ it("keeps lint errors failing and visible alongside the retained warnings", asyn ); }); +it("preserves cross-file typed errors and explicit worker choices", async () => { + const root = await fixture(); + await Promise.all([ + writeFile(join(root, "package.json"), '{"type":"module"}'), + writeFile( + join(root, "tsconfig.json"), + JSON.stringify({ + compilerOptions: { + module: "NodeNext", + target: "ES2023", + lib: ["ES2023"], + strict: true, + types: [], + }, + include: ["src/**/*.ts"], + }), + ), + writeFile( + join(root, ".oxlintrc.json"), + JSON.stringify({ + plugins: ["typescript"], + rules: { + "no-unused-vars": "warn", + "typescript/no-floating-promises": "error", + "typescript/switch-exhaustiveness-check": "error", + }, + }), + ), + writeFile( + join(root, "src/types.ts"), + 'export type State = "ready" | "failed";\n' + + "export function pending(): Promise { return Promise.resolve(); }\n", + ), + ]); + const caller = join(root, "src/caller.ts"); + const source = + 'import { pending, type State } from "./types.js";\n' + + "const forgotten = 1;\n"; + await writeFile( + caller, + source + + "pending();\n" + + 'export function inspect(state: State) { switch (state) { case "ready": return 1; } }\n', + ); + const diagnostics: string[][] = []; + const typedEnvironment = { + ...process.env, + PATH: [join(repository, "node_modules", ".bin"), process.env.PATH] + .filter((path) => path !== undefined) + .join(delimiter), + }; + for (const workers of [undefined, "2"]) { + const result = invokeWithEnvironment(root, "typed-lint", { + ...typedEnvironment, + GOMAXPROCS: workers, + OXC_LOG: "debug", + }); + expect(result.error).toBeUndefined(); + expect(result.status, result.stderr).toBe(1); + expect(result.stderr).toContain( + `Starting linter with ${workers ?? "1"} workers`, + ); + expect(result.stdout).toContain("2 errors, 1 warnings"); + const report = z + .object({ + number_of_files: z.number(), + diagnostics: z.array( + z.object({ code: z.string(), severity: z.string() }).passthrough(), + ), + }) + .parse( + JSON.parse( + await readFile( + join(root, ".cache/static-checks/typed-oxlint.json"), + "utf8", + ), + ), + ); + expect(report.number_of_files).toBe(2); + expect( + report.diagnostics + .filter(({ severity }) => severity === "error") + .map(({ code }) => code) + .sort(), + ).toEqual([ + "typescript(no-floating-promises)", + "typescript(switch-exhaustiveness-check)", + ]); + diagnostics.push( + report.diagnostics.map((item) => JSON.stringify(item)).sort(), + ); + } + expect(diagnostics[1]).toEqual(diagnostics[0]); + await writeFile( + caller, + source + + "await pending();\n" + + 'export function inspect(state: State) { switch (state) { case "ready": return 1; case "failed": return 0; } }\n', + ); + const corrected = invokeWithEnvironment(root, "typed-lint", { + ...typedEnvironment, + GOMAXPROCS: undefined, + }); + expect(corrected.error).toBeUndefined(); + expect(corrected.status, corrected.stderr).toBe(0); + expect(corrected.stdout).toContain("0 errors, 1 warnings"); +}); + it("excludes translation staging while retaining authored website errors", async () => { const root = await fixture(); await writeFile( diff --git a/tests/boundary/mcp/packageCatalogVerification.test.ts b/tests/boundary/mcp/packageCatalogVerification.test.ts index dcfc664394..73c47ba631 100644 --- a/tests/boundary/mcp/packageCatalogVerification.test.ts +++ b/tests/boundary/mcp/packageCatalogVerification.test.ts @@ -7,11 +7,27 @@ import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarnes it("verifies canonical names, converted schemas and session metadata together", async () => { const harness = await createApplicationMcpHarness(); try { - const tools = await harness.client.listTools(); - const status = await harness.client.callTool({ - name: "binary_session", - arguments: {}, - }); + const listed = await harness.client.listTools(); + const sessionDefinition = listed.tools.find( + ({ name }) => name === "binary_session", + ); + if (sessionDefinition === undefined) + throw new Error("Expected the binary_session definition"); + // Catalog order must not select which schema validates the lifecycle call. + const tools = { + ...listed, + tools: [ + ...listed.tools.filter(({ name }) => name !== "binary_session"), + sessionDefinition, + ], + }; + const status = await harness.client.callTool( + { + name: "binary_session", + arguments: {}, + }, + { toolDefinition: sessionDefinition }, + ); const forwarded: unknown[][] = []; const client = { listTools: async (...args: unknown[]) => { @@ -27,7 +43,10 @@ it("verifies canonical names, converted schemas and session metadata together", const canonical = await verifyCompleteToolCatalog(client, options); expect(forwarded).toEqual([ [undefined, options], - [{ name: "binary_session", arguments: {} }, options], + [ + { name: "binary_session", arguments: {} }, + { ...options, toolDefinition: sessionDefinition }, + ], ]); expect(canonical).toEqual(tools.tools.map(({ name }) => name).sort()); diff --git a/tests/boundary/process/releaseArtifactCanary.test.ts b/tests/boundary/process/releaseArtifactCanary.test.ts index 5ee9c82140..c91c520814 100644 --- a/tests/boundary/process/releaseArtifactCanary.test.ts +++ b/tests/boundary/process/releaseArtifactCanary.test.ts @@ -28,14 +28,20 @@ it.skipIf(process.platform === "win32")( const harness = await createApplicationMcpHarness(); let snapshot; try { + const tools = await harness.client.listTools(); + const sessionDefinition = tools.tools.find( + ({ name }) => name === "binary_session", + ); + if (sessionDefinition === undefined) + throw new Error("Canary snapshot omitted binary_session"); snapshot = { serverInfo: harness.client.getServerVersion(), protocolVersion: harness.client.getNegotiatedProtocolVersion(), - tools: await harness.client.listTools(), - reply: await harness.client.callTool({ - name: "binary_session", - arguments: {}, - }), + tools, + reply: await harness.client.callTool( + { name: "binary_session", arguments: {} }, + { toolDefinition: sessionDefinition }, + ), }; } finally { await harness.close();