shifathosn
/
rea
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor: add incremental capability migration safeguards (#819)

This commit is contained in:
N0zoM1z0 2026-10-07 10:56:06 +08:00 committed by GitHub
parent 6caa38a0ca
commit 75f50d86f6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 421 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.github/workflows/ci.yml vendored
View File

@ -77,6 +77,8 @@ jobs:
- run: npm ci
- if: github.event_name == 'push' || needs.changes.outputs.code == 'true'
run: npm run check:ci
- if: github.event_name == 'push' || needs.changes.outputs.code == 'true'
run: npm run verify:test-discovery
- if: github.event_name == 'pull_request' && needs.changes.outputs.code != 'true'
run: npm run format:check

53
.oxlintrc.json
View File

@ -6,11 +6,19 @@
"max-depth": ["warn", 4],
"max-lines": [
"warn",
{ "max": 500, "skipBlankLines": true, "skipComments": true }
{
"max": 500,
"skipBlankLines": true,
"skipComments": true
}
],
"max-lines-per-function": [
"warn",
{ "max": 100, "skipBlankLines": true, "skipComments": true }
{
"max": 100,
"skipBlankLines": true,
"skipComments": true
}
],
"max-params": ["warn", 4],
"no-var": "warn",
@ -46,13 +54,52 @@
]
}
},
{
"files": ["src/{contracts,domain}/**/*.ts"],
"rules": {
"no-restricted-imports": [
"error",
{
"patterns": [
{
"group": [
"**/application/**",
"**/server/**",
"**/cli/**",
"**/main/**",
"**/cli.*",
"**/main.*",
"**/android/**",
"**/artifacts/**",
"**/browser/**",
"**/dotnet/**",
"**/firmware/**",
"**/ghidra/**",
"**/hopper/**",
"**/ida/**",
"**/native/**",
"**/process/**",
"**/reference/**",
"**/windows/**"
],
"message": "Domain and contracts depend inward; use typed ports for application, caller and provider boundaries."
}
]
}
]
}
},
{
"files": ["tests/**/*.ts", "src/**/*.test.ts", "src/**/*.fixture.ts"],
"rules": {
"max-lines": "off",
"max-lines-per-function": [
"error",
{ "max": 150, "skipBlankLines": true, "skipComments": true }
{
"max": 150,
"skipBlankLines": true,
"skipComments": true
}
]
}
},

4
CONTRIBUTING.md
View File

@ -2,6 +2,10 @@
REA welcomes focused bug fixes, documentation improvements, tests, and reverse-engineering workflow enhancements. Open an issue before a large contract or architecture change so its scope can be agreed before implementation.
For capability organization and provider composition, follow the incremental
[migration guide](docs/capability-migration.md). Run `npm run verify:test-discovery`
after adding or moving tests.
When adding or changing an MCP tool, follow the [tool design guide](docs/tool-design.md) and preserve the canonical contracts and generated catalog.
## Development setup

79
docs/capability-migration.md Normal file
View File

@ -0,0 +1,79 @@
# Incremental capability migration
This working guide tracks the migration in #740. Update it as concrete domains
migrate. Existing public contracts and domain-specific ports remain authoritative;
there is no new universal provider interface or fixed module manifest.
## Ownership to preserve
| Responsibility | Current owner | Migration direction |
| --------------------------------------------- | ------------------------------------------- | -------------------------------------------------------------- |
| Analyst semantics and Evidence representation | domain | Keep pure and provider-neutral |
| Named public input/output contracts | contracts | Keep exact CLI/MCP meaning and complete discovery |
| Producer parsing and external tool protocols | provider adapters | Keep behind the relevant typed port |
| Shared analyst workflows | application | Share between CLI and MCP |
| Concrete provider construction | binary runtime, CLI registrars, MCP startup | Consolidate one capability at a time in production composition |
| Investigation Evidence and Unknowns | BinarySessionRecords | Extract one composed record owner after caller migration |
| Target/profile snapshots and invalidation | BinarySessionRecords/BinarySession | Keep binary-owned |
| Subprocess ownership and host primitives | process/windows | Reuse; preserve cancellation and cleanup |
| Public translation | CLI/MCP adapters | Delegate to shared workflows and named contracts |
Retain exact Evidence IDs, provenance, detached reads, atomic Evidence/Unknown
mutations and optimistic revisions. Preserve current close and failed-cleanup
record clearing while extracting ownership; #721 owns any retention-policy change.
Each CLI invocation and MCP connection keeps its own state. Provider registration
must not launch engines or install prerequisites.
## Migrate before generalizing
1. Record the actual callers, state, environment, protocol and verification owner
of the capability being migrated.
2. Move concrete construction behind its existing typed port. Preserve injected
ports, selected environment, queues, failure state and cleanup behavior.
3. Test both caller paths and the applicable package/real-provider boundary.
4. Use independently migrated capabilities to identify repeated metadata and
composition needs. Derive a small source declaration only when those needs
are established; keep unlike target/lifecycle semantics separate.
5. Move a coherent capability's files, fixtures and verification lanes in a
mechanical PR. Update all path-sensitive consumers in that same PR.
Pure helpers do not need an interface each. A capture-format adapter or typed
factory may be sufficient. Avoid generic JSON execution, external MCP catalog
passthrough, eager provider acquisition, new permission fields or a multi-package
rewrite. Optional loading should preserve successful peers and report the failed
capability without hiding tools from the catalog.
## Checks before and after a move
`npm run verify:test-discovery` compares actual Vitest file discovery with every
repository-owned `*.test.ts` under src and tests, including newly added files. The
check requires Git, includes untracked non-ignored tests, and excludes removed
paths during a move. It
rejects missing files and overlapping project ownership. It only lists files;
it does not replace behavior tests or real-provider verification.
Check imports, project isolation and coverage, development-test selection,
lint exceptions, catalog/generator imports, npm and CI verification entrypoints,
fixture ownership, and installed-package bridge/native path resolution.
Domain and contracts must not import application workflows, caller adapters or
provider implementations. Production composition is a deliberate outer boundary.
Other existing adapter imports will migrate incrementally; do not widen a lint
exception to conceal new protocol code inside application workflows.
Select verification by the claim changed: schema/composition, package, real
engine or host-native workflow. Record host/target prerequisites and source-owned
fixtures. Run heavy lanes serially with bounded workers/heaps and reuse existing
engines. Recording ports do not prove real provider or platform support.
## Extension examples to validate during migration
An implementation of an existing port should require a provider adapter, typed
production factory selection, exact producer parsing/provenance and matching
verification. Existing workflow meaning and public contracts should stay stable.
A new analyst capability deliberately adds semantics, named contracts, a typed
port/workflow when needed, both caller adapters, availability, documentation and
verification. Its integration is broader than swapping a provider. The migration
should eliminate incidental duplicate wiring, not erase those design decisions.
Update these examples with real migrated entrypoints before closing #740.

3
package.json
View File

@ -175,7 +175,8 @@
"build:ghidra:nativeaot": "node scripts/build-ghidra-nativeaot.mjs",
"build:fixtures:nativeaot": "node scripts/generate-nativeaot-fixtures.mjs",
"verify:ghidra:nativeaot": "npm run build:cached && node scripts/verify-real-ghidra-nativeaot.mjs",
"verify:browser:scripts": "npm run build:cached && node scripts/verify-browser-scripts.mjs"
"verify:browser:scripts": "npm run build:cached && node scripts/verify-browser-scripts.mjs",
"verify:test-discovery": "node scripts/verify-test-discovery.mjs"
},
"lint-staged": {
"*.{cjs,cts,js,jsx,mjs,mts,ts,tsx}": [

14
scripts/lib/test-discovery.d.mts Normal file
View File

@ -0,0 +1,14 @@
/** Compare actual Vitest discovery with repository-owned test paths. */
export function compareTestDiscovery(
expectedPaths: readonly string[],
discovery: unknown,
root: string,
): {
readonly missing: readonly string[];
readonly unexpected: readonly string[];
readonly duplicates: readonly {
readonly file: string;
readonly projects: readonly string[];
}[];
readonly discovered: number;
};

41
scripts/lib/test-discovery.mjs Normal file
View File

@ -0,0 +1,41 @@
import { relative, resolve } from "node:path";
/** Compare actual Vitest discovery with repository-owned test paths. */
export const compareTestDiscovery = (expectedPaths, discovery, root) => {
if (!Array.isArray(discovery))
throw new TypeError("Vitest file discovery must be an array");
const projectsByFile = new Map();
for (const entry of discovery) {
if (
entry === null ||
typeof entry !== "object" ||
typeof entry.file !== "string" ||
entry.file.length === 0 ||
typeof entry.projectName !== "string" ||
entry.projectName.length === 0
)
throw new TypeError(
"Vitest discovery requires file and projectName strings",
);
const path = relative(root, resolve(root, entry.file)).replaceAll(
"\\",
"/",
);
const projects = projectsByFile.get(path) ?? [];
projects.push(entry.projectName);
projectsByFile.set(path, projects);
}
const expected = new Set(expectedPaths);
const sorted = (values) => [...values].sort();
return {
missing: sorted(expected).filter((path) => !projectsByFile.has(path)),
unexpected: sorted(projectsByFile.keys()).filter(
(path) => !expected.has(path),
),
duplicates: sorted(projectsByFile.keys()).flatMap((file) => {
const projects = projectsByFile.get(file);
return projects.length > 1 ? [{ file, projects: sorted(projects) }] : [];
}),
discovered: projectsByFile.size,
};
};

65
scripts/verify-test-discovery.mjs Normal file
View File

@ -0,0 +1,65 @@
import { execFile } from "node:child_process";
import { mkdtemp, readFile, rm, stat } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { promisify } from "node:util";
import { compareTestDiscovery } from "./lib/test-discovery.mjs";
const execute = promisify(execFile);
const root = resolve(import.meta.dirname, "..");
const temporary = await mkdtemp(join(tmpdir(), "rea-test-discovery-"));
try {
const output = join(temporary, "discovery.json");
await execute(
process.execPath,
[
join(root, "node_modules/vitest/vitest.mjs"),
"list",
"--filesOnly",
`--json=${output}`,
],
{ cwd: root, maxBuffer: 8 * 1024 * 1024 },
);
const files = await execute(
"git",
[
"ls-files",
"-z",
"--cached",
"--others",
"--exclude-standard",
"--",
"src",
"tests",
],
{ cwd: root, maxBuffer: 8 * 1024 * 1024 },
);
const expected = [];
for (const path of files.stdout
.split("\0")
.filter((path) => path.endsWith(".test.ts"))) {
try {
if ((await stat(join(root, path))).isFile()) expected.push(path);
} catch (cause) {
if (cause.code !== "ENOENT") throw cause;
}
}
const report = compareTestDiscovery(
expected,
JSON.parse(await readFile(output, "utf8")),
root,
);
if (
report.missing.length ||
report.unexpected.length ||
report.duplicates.length
) {
console.error(JSON.stringify(report, null, 2));
process.exitCode = 1;
} else
console.log(`Discovered all ${report.discovered} test files exactly once.`);
} finally {
await rm(temporary, { recursive: true, force: true });
}

13
src/application/ReconstructionCoverageService.ts
View File

@ -1,19 +1,10 @@
import { z } from "zod";
import { reconstructionCoverageEvaluationInputSchema } from "../domain/reconstructionCoverageInput.js";
import { AnalysisInputError } from "../domain/analysisErrorCore.js";
import { type AnalysisError } from "../domain/analysisErrorBase.js";
import {
evaluateReconstructionClosure,
reconstructionCoverageDataSchema,
} from "../domain/reconstructionCoverage.js";
import { evaluateReconstructionClosure } from "../domain/reconstructionCoverage.js";
import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js";
import { err, ok, type Result } from "../domain/result.js";
export const reconstructionCoverageEvaluationInputSchema = z.strictObject({
coverage: reconstructionCoverageDataSchema,
boundary_id: z.string().min(1),
});
/** Evaluate inline reconstruction coverage against a named boundary. */
export const evaluateReconstructionCoverage = (
rawInput: unknown,

6
src/cli/applicationCommands.ts
View File

@ -1,3 +1,4 @@
import { reconstructionCoverageEvaluationInputSchema } from "../domain/reconstructionCoverageInput.js";
import { Cli, z } from "incur";
import {
@ -7,10 +8,7 @@ import {
traceApplicationFeatureEvidenceValidated,
} from "../application/JavaScriptApplicationWorkflowService.js";
import { traceJavaScriptSemanticsEvidenceValidated } from "../application/JavaScriptSemanticTraceService.js";
import {
evaluateReconstructionCoverage,
reconstructionCoverageEvaluationInputSchema,
} from "../application/ReconstructionCoverageService.js";
import { evaluateReconstructionCoverage } from "../application/ReconstructionCoverageService.js";
import { buildReconstructionObligationLedgerEvidenceValidated } from "../application/ReconstructionObligationLedgerService.js";
import { CLI_COMMANDS } from "../cliCommandNames.js";
import { parseCliJsonInput } from "../cliJsonInput.js";

2
src/contracts/applicationToolContracts.ts
View File

@ -10,7 +10,7 @@ import {
traceApplicationFeatureRequestSchema,
traceJavaScriptSemanticsRequestSchema,
} from "./applicationWorkflowInputContracts.js";
import { reconstructionCoverageEvaluationInputSchema } from "../application/ReconstructionCoverageService.js";
import { reconstructionCoverageEvaluationInputSchema } from "../domain/reconstructionCoverageInput.js";
import { reconstructionClosureResultSchema } from "../domain/reconstructionCoverage.js";
import {
reconstructionObligationLedgerInputSchema,

46
src/contracts/contractSnapshot.test.ts
View File

@ -1,7 +1,5 @@
import { describe, expect, it } from "vitest";
import { z } from "zod";
import { createCli } from "../cli.js";
import { createCliInventory } from "../../scripts/lib/product-catalog.mjs";
import { ENHANCED_TOOL_CONTRACTS } from "./enhancedToolContracts.js";
import { OFFICIAL_TOOL_CONTRACTS } from "./officialToolContracts.js";
@ -19,14 +17,7 @@ import { JAVASCRIPT_RUNTIME_OBSERVATION_TOOL_CONTRACTS } from "./javascriptRunti
import { APPLICATION_TOOL_CONTRACTS } from "./applicationToolContracts.js";
import { WEB_SCRIPT_TOOL_CONTRACTS } from "./webScriptToolContracts.js";
import { TOOL_EFFECTS } from "./toolEffects.js";
import { TOOL_CONTRACTS, toolContract } from "./toolContracts.js";
import {
CLI_COMMAND_ALIASES,
CLI_COMMAND_NAMES,
CLI_COMMAND_TOOL_ALIASES,
MCP_TOOLS_WITHOUT_DEDICATED_CLI,
} from "../cliCommandNames.js";
import { toolContract } from "./toolContracts.js";
const contractJsonSchema = (schema: z.ZodType) =>
z.toJSONSchema(schema, { target: "draft-07", unrepresentable: "any" });
@ -135,39 +126,4 @@ describe("tool contract surface", () => {
destructiveHint: false,
});
});
it("accounts for every MCP tool on the CLI or marks it intentionally MCP-only", () => {
const registeredCli = createCliInventory(createCli());
const registeredCommandNames = new Set([
...registeredCli.primary,
...registeredCli.aliases.map(({ name }) => name),
]);
const cliToolNames = new Set(
registeredCli.primary.map((command) => command.replaceAll("-", "_")),
);
for (const [command, names] of Object.entries(CLI_COMMAND_TOOL_ALIASES)) {
expect(registeredCommandNames.has(command)).toBe(true);
for (const name of names) cliToolNames.add(name);
}
for (const command of CLI_COMMAND_NAMES)
expect(registeredCommandNames.has(command)).toBe(true);
for (const command of CLI_COMMAND_ALIASES)
expect(registeredCommandNames.has(command)).toBe(true);
const intentionalMcpOnly = new Set<string>(MCP_TOOLS_WITHOUT_DEDICATED_CLI);
expect(
[...intentionalMcpOnly].filter((name) => cliToolNames.has(name)).sort(),
).toEqual([]);
const unaccounted = TOOL_CONTRACTS.map(({ name }) => name).filter(
(name) => !cliToolNames.has(name) && !intentionalMcpOnly.has(name),
);
expect(unaccounted).toEqual([]);
expect(new Set(MCP_TOOLS_WITHOUT_DEDICATED_CLI).size).toBe(
MCP_TOOLS_WITHOUT_DEDICATED_CLI.length,
);
expect(
MCP_TOOLS_WITHOUT_DEDICATED_CLI.every((name) =>
TOOL_CONTRACTS.some((contract) => contract.name === name),
),
).toBe(true);
});
});

8
src/domain/reconstructionCoverageInput.ts Normal file
View File

@ -0,0 +1,8 @@
import { z } from "zod";
import { reconstructionCoverageDataSchema } from "./reconstructionCoverage.js";
/** Named-boundary input for inline reconstruction coverage evaluation. */
export const reconstructionCoverageEvaluationInputSchema = z.strictObject({
coverage: reconstructionCoverageDataSchema,
boundary_id: z.string().min(1),
});

48
tests/boundary/cli/cliToolCoverage.test.ts Normal file
View File

@ -0,0 +1,48 @@
import { describe, expect, it } from "vitest";
import { createCli } from "../../../src/cli.js";
import { createCliInventory } from "../../../scripts/lib/product-catalog.mjs";
import { TOOL_CONTRACTS } from "../../../src/contracts/toolContracts.js";
import {
CLI_COMMAND_ALIASES,
CLI_COMMAND_NAMES,
CLI_COMMAND_TOOL_ALIASES,
MCP_TOOLS_WITHOUT_DEDICATED_CLI,
} from "../../../src/cliCommandNames.js";
describe("CLI canonical tool coverage", () => {
it("accounts for every MCP tool on the CLI or marks it intentionally MCP-only", () => {
const registeredCli = createCliInventory(createCli());
const registeredCommandNames = new Set([
...registeredCli.primary,
...registeredCli.aliases.map(({ name }) => name),
]);
const cliToolNames = new Set(
registeredCli.primary.map((command) => command.replaceAll("-", "_")),
);
for (const [command, names] of Object.entries(CLI_COMMAND_TOOL_ALIASES)) {
expect(registeredCommandNames.has(command)).toBe(true);
for (const name of names) cliToolNames.add(name);
}
for (const command of CLI_COMMAND_NAMES)
expect(registeredCommandNames.has(command)).toBe(true);
for (const command of CLI_COMMAND_ALIASES)
expect(registeredCommandNames.has(command)).toBe(true);
const intentionalMcpOnly = new Set<string>(MCP_TOOLS_WITHOUT_DEDICATED_CLI);
expect(
[...intentionalMcpOnly].filter((name) => cliToolNames.has(name)).sort(),
).toEqual([]);
const unaccounted = TOOL_CONTRACTS.map(({ name }) => name).filter(
(name) => !cliToolNames.has(name) && !intentionalMcpOnly.has(name),
);
expect(unaccounted).toEqual([]);
expect(new Set(MCP_TOOLS_WITHOUT_DEDICATED_CLI).size).toBe(
MCP_TOOLS_WITHOUT_DEDICATED_CLI.length,
);
expect(
MCP_TOOLS_WITHOUT_DEDICATED_CLI.every((name) =>
TOOL_CONTRACTS.some((contract) => contract.name === name),
),
).toBe(true);
});
});

44
tests/evaluation/moduleBoundaries.test.ts Normal file
View File

@ -0,0 +1,44 @@
import { spawnSync } from "node:child_process";
import { copyFile, mkdir, writeFile } from "node:fs/promises";
import { dirname, join, resolve } from "node:path";
import { describe, expect, it } from "vitest";
import { createTestTempDirectory } from "../fixtures/temporaryDirectory.js";
describe("incremental module import boundaries", () => {
it.each([
["src/domain/probe.ts", "../application/Workflow.js", false],
["src/domain/probe.ts", "../browser/Provider.js", false],
["src/contracts/probe.ts", "../android/Provider.js", false],
["src/contracts/probe.ts", "../cli.js", false],
["src/domain/probe.ts", "./result.js", true],
["src/contracts/probe.ts", "../domain/result.js", true],
["src/application/probe.ts", "../domain/result.js", true],
["src/composition/probe.ts", "../ghidra/Provider.js", true],
])("checks %s importing %s", async (file, dependency, allowed) => {
const temporary = await createTestTempDirectory("rea-module-boundary-");
const config = join(temporary, ".oxlintrc.json");
await copyFile(resolve(".oxlintrc.json"), config);
const fixture = join(temporary, file);
await mkdir(dirname(fixture), { recursive: true });
await writeFile(
fixture,
`import { value } from ${JSON.stringify(dependency)}; export { value };\n`,
);
const result = spawnSync(
process.execPath,
[
resolve("node_modules/oxlint/bin/oxlint"),
"-c",
config,
"--format",
"json",
file,
],
{ cwd: temporary, encoding: "utf8" },
);
if (result.error !== undefined) throw result.error;
expect(result.status).toBe(allowed ? 0 : 1);
if (!allowed) expect(result.stdout).toContain("no-restricted-imports");
});
});

56
tests/evaluation/testDiscovery.test.ts Normal file
View File

@ -0,0 +1,56 @@
import { resolve } from "node:path";
import { describe, expect, it } from "vitest";
import { compareTestDiscovery } from "../../scripts/lib/test-discovery.mjs";
const root = resolve("test-discovery-fixture");
const first = "src/domain/sample.test.ts";
const second = "src/new-provider/sample.test.ts";
describe("test discovery migration guard", () => {
it("accepts absolute discovered paths without changing their project ownership", () => {
expect(
compareTestDiscovery(
[first],
[{ file: resolve(root, first), projectName: "domain" }],
root,
),
).toEqual({ missing: [], unexpected: [], duplicates: [], discovered: 1 });
});
it("reports a new provider test omitted by the current project globs", () => {
expect(
compareTestDiscovery(
[first, second],
[{ file: first, projectName: "domain" }],
root,
).missing,
).toEqual([second]);
});
it("reports overlapping project discovery and unexpected files separately", () => {
const report = compareTestDiscovery(
[first],
[
{ file: first, projectName: "domain" },
{ file: first, projectName: "adapters" },
{ file: second, projectName: "adapters" },
],
root,
);
expect(report.duplicates).toEqual([
{ file: first, projects: ["adapters", "domain"] },
]);
expect(report.unexpected).toEqual([second]);
});
it.each([
null,
{},
[{ file: "", projectName: "domain" }],
[{ file: first }],
[{ file: first, projectName: null }],
])("rejects malformed producer output: %j", (input) => {
expect(() => compareTestDiscovery([first], input, root)).toThrow(TypeError);
});
});

3
turbo.json
View File

@ -28,7 +28,8 @@
"tests/**/*.ts",
"tsconfig.json",
"tsconfig.build.json",
"vitest.config.ts"
"vitest.config.ts",
"scripts/**/*.d.mts"
],
"outputs": [".cache/tsconfig.typecheck.tsbuildinfo"]
},