Skip to content

Add a npx hereby validate command to group all repo validations #64161

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
Wesley Wigham (weswigham) merged 2 commits into from
Sep 8, 2026
+85 −29
Merged

Add a npx hereby validate command to group all repo validations #64161

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
4aa5df6
Add a single `npx hereby validate` command to replace the chain of va…
weswigham Sep 3, 2026
24e4d32
More task breakouts, more instruction, more merged task
weswigham Sep 8, 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
20 changes: 9 additions & 11 deletions .github/copilot-instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,18 @@ IF THESE COMMANDS FAIL, CI WILL FAIL, AND YOUR PR WILL BE REJECTED OUT OF HAND.
FIXING ERRORS FROM THESE COMMANDS IS YOUR HIGHEST PRIORITY.
ENSURE YOU DO THE RIGHT THINGS TO MAKE THEM PASS.
```sh
npx hereby build # Build the project
npx hereby test # Run tests
npx hereby lint # Run linters
npx hereby format # Format the code
npx hereby validate # Build, test, lint, and format the project

@jakebailey Jake Bailey (jakebailey) Sep 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One of the things I want to do in this file is to make it clear that the build, test, lint, just matter when editing the tsc dir.

We can probably do that in a followup, as the "CRITICAL" language here should really be in the CCA file instead

@weswigham Wesley Wigham (weswigham) Sep 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

...isn't this the CCA file? It's copilot-instructions.md. The only other markdown file that ever even mentions hereby commands is the generic CONTRIBUTING.md (and the compiler test skill).

@jakebailey Jake Bailey (jakebailey) Sep 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the file used by all copilot stuff; they only recently added a file that is for CCA only. Unfortunately I now cannot find the docs for this. They do have https://github.blog/changelog/2025-11-12-copilot-code-review-and-coding-agent-now-support-agent-specific-instructions/ which lets you exclude certain things, but I know they have a separate CCA one somewhere...

```
</critical>

If you are writing or testing TS API features (eg, code in packages/typescript/src/api/async/api.ts), additionally, you need to run
```sh
npx hereby test:api
npx hereby validate --api # Also run the TypeScript API tests
```
which is not run as part of the primary suite.
instead. API tests are not run by `npx hereby validate` without `--api`.

If as part of your change you edit tools, benchmarks, or other ancillary parts of the repository, you should instead run `npx hereby validate --all` to ensure *everything* is working correctly. This will
most accurately approximate the results of a full CI run.

## Compiler Features, Fixes, and Tests

Expand Down Expand Up @@ -134,10 +134,8 @@ Were alternate fixes considered? Describe them briefly if so
## Copilot Checklist

<!-- don't lie! -->
I successfully ran these commands at the end of my session, and they completed without error:
* [ ] npx hereby build
* [ ] npx hereby test
* [ ] npx hereby lint
* [ ] npx hereby format
I successfully ran the applicable command at the end of my session, and it completed without error:
* [ ] npx hereby validate
* [ ] npx hereby validate --api (for TypeScript API changes)

```
94 changes: 76 additions & 18 deletions Herebyfile.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ const { values: rawOptions } = parseArgs({
options: {
tests: { type: "string", short: "t" },
fix: { type: "boolean" },
api: { type: "boolean" },
all: { type: "boolean" },
debug: { type: "boolean" },
dirty: { type: "boolean" },
release: { type: "boolean" },
Expand Down Expand Up @@ -967,13 +969,15 @@ export const generateAST = task({
run: () => run("node", ["./tools/scripts/tsc/generate.ts"]),
});

async function runGenerateAPI() {
await run("go", ["-C", "./tools", "run", "./gen-proto", "../tsc/internal/api/proto.go", "../packages/typescript/src/api/proto.generated.ts"]);
await run("npx", ["dprint", "fmt", "packages/typescript/src/api/proto.generated.ts"]);
}

export const generateAPI = task({
name: "generate:api",
description: "Generates API files from internal/api/proto.go and internal/api/session.go.",
run: async () => {
await run("go", ["-C", "./tools", "run", "./gen-proto", "../tsc/internal/api/proto.go", "../packages/typescript/src/api/proto.generated.ts"]);
await run("npx", ["dprint", "fmt", "packages/typescript/src/api/proto.generated.ts"]);
},
run: runGenerateAPI,
});

// ── Vendored npm dependencies ───────────────────────────────────
Expand Down Expand Up @@ -1186,13 +1190,16 @@ export const testTsc = task({
run: runTests,
});

export const testExtension = task({
name: "test:extension",
description: "Runs the VS Code extension tests.",
run: runTestExtension,
});

export const test = task({
name: "test",
description: "Runs all tests. This is the most typical test task to need.",
run: async () => {
await runTests();
await runTestExtension();
},
description: "Alias for test:tsc.",
dependencies: [testTsc],
});

async function runTestBenchmarks() {
Expand Down Expand Up @@ -1223,12 +1230,6 @@ export const testTools = task({
run: runTestTools,
});

export const testExtension = task({
name: "test:extension",
description: "Runs the VS Code extension tests.",
run: runTestExtension,
});

export const buildAPI = task({
name: "build:api",
description: "Builds @typescript/typescript JS API.",
Expand All @@ -1237,13 +1238,15 @@ export const buildAPI = task({
},
});

async function runBuildAPITests() {
await run("npm", ["run", "-w", "@typescript/typescript", "build:test"]);
}

export const buildAPITests = task({
name: "build:api:test",
description: "Builds the @typescript/typescript JS API tests.",
dependencies: [generateEnums, generateAPI],
run: async () => {
await run("npm", ["run", "-w", "@typescript/typescript", "build:test"]);
},
run: runBuildAPITests,
});

export const testAPI = task({
Expand Down Expand Up @@ -1364,6 +1367,61 @@ async function runFormat() {
await run("dprint", ["fmt"]);
}

export const validate = task({
name: "validate",

@jakebailey Jake Bailey (jakebailey) Sep 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have a lot of terms, test, check, validate... Sort of wonder if we should somehow name this with "all" in the name to make it very clear what must be done

@weswigham Wesley Wigham (weswigham) Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added a --all argument to validate that includes all the various tool/benchcmark/api tests and smoke test. Pretty sure that covers every test: subcommand listed in ci.yml.

description: "Builds, tests, lints, and formats the repo. Pass --api to include API tests, or --all to include all ancilliary repository tests.",
dependencies: [build],

@jakebailey Jake Bailey (jakebailey) Sep 3, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I didn't notice this, but yeah build doesn't need to be a dep, right? nothing needs that to have happened? I guess the API does?

@weswigham Wesley Wigham (weswigham) Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The API needs it to work and I have doubts on if lint, test, and format would reliably work in the presence of a non-functioning build.

@jakebailey Jake Bailey (jakebailey) Sep 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So right, I was just surprised by it being a dep versus another func call, but it doesn't practically matter.

run: async () => {
/** @type {{ name: string; error: unknown }[]} */
const failures = [];
/** @param {string} name @param {() => Promise<void>} action */
const runValidation = async (name, action) => {
try {
await action();
}
catch (error) {
failures.push({ name, error });
console.error(styleText("red", `${name} failed; continuing validation.`));
}
};

await runValidation("test:tsc", runTests);
await runValidation("test:extension", runTestExtension);
if (options.api || options.all) {
await runGenerateEnums(); // prereqs for test:api not included in `validate` deps
await runGenerateAPI();
await runBuildAPITests();
await runValidation("test:api", runTestAPI);
}
if (options.all) {
await runValidation("test:benchmarks", runTestBenchmarks);
await runValidation("test:tools", runTestTools);
await runValidation("test:smoke", runSmokeTest); // in CI this is run with `--race`
}
await runValidation("lint", runLint);
await runValidation("format", runFormat);

if (failures.length) {
throw new AggregateError(
failures.map(failure => failure.error),
`Validation failed: ${failures.map(failure => failure.name).join(", ")}`,
);
}
},
});

async function runSmokeTest() {
await run("./built/local/tsc", ["-p", "./tsc/testdata/fixtures/compiler", "--noEmit", "--singleThreaded"]);
await run("./built/local/tsc", ["-p", "./tsc/testdata/fixtures/compiler", "--noEmit"]);
}

export const smokeTest = task({
name: "test:smoke",
description: "Runs the smoke tests.",
dependencies: [build],
run: runSmokeTest,
});

export const checkFormat = task({
name: "check:format",
description: "Checks that the repo is formatted.",
Expand Down