Skip to content

Commit 142643c

Browse files
authored
Merge branch 'main' into copilot/add-apply-command
2 parents edaa2da + b80ac8e commit 142643c

File tree

11 files changed

+144
-130
lines changed

11 files changed

+144
-130
lines changed

.changeset/renovate-0941c05.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'counterfact': patch
3+
---
4+
5+
Updated dependency `ajv` to `8.18.0`.

.github/copilot-instructions.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ Each item must use this format:
3333
- [ ] Response includes `x-test` header when defined
3434
- [ ] Existing routes without examples behave unchanged
3535

36+
### Exception: Design PRs
37+
38+
When the goal of the **PR** is to **create more issues rather than write code** (e.g., proposing issue files under `.github/issue-proposals/`), treat the PR as a **design PR**:
39+
40+
1. Add the `design` label to the PR.
41+
2. Do **not** include a "## Manual acceptance tests" section — omit it entirely.
42+
3. The CI check for manual acceptance tests will automatically pass for PRs with the `design` label.
43+
3644
## Test-driven workflow
3745

3846
When implementing a change, work in a test-first or test-guided way whenever practical.
@@ -68,6 +76,30 @@ Add or update tests for:
6876
- generated code, when generation behavior is being changed
6977
- any change that affects external contracts, APIs, schemas, or CLI behavior
7078

79+
### File system operations in tests
80+
81+
When tests need to read or write files, always use `usingTemporaryFiles()` from the `using-temporary-files` package (already a devDependency). Never import `node:fs`, `fs`, `node:fs/promises`, or `fs/promises` directly in test files.
82+
83+
The `$` helper passed to the callback provides:
84+
- `$.add(relativePath, contents)` — create or overwrite a file
85+
- `$.addDirectory(relativePath)` — create a directory
86+
- `$.read(relativePath)` — read a file's contents (returns `Promise<string>`)
87+
- `$.remove(relativePath)` — delete a file
88+
- `$.path(relativePath)` — resolve an absolute path within the temporary directory (use this when passing paths to the code under test)
89+
90+
```ts
91+
import { usingTemporaryFiles } from "using-temporary-files";
92+
93+
it("example", async () => {
94+
await usingTemporaryFiles(async ($) => {
95+
await $.add("input.json", JSON.stringify({ key: "value" }));
96+
const result = await myFunction($.path("input.json"));
97+
const output = await $.read("output.txt");
98+
expect(output).toBe("expected content");
99+
});
100+
});
101+
```
102+
71103
### When tests are hard to add
72104

73105
If a test is difficult to write:

.github/workflows/manual-acceptance-tests.yaml

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,26 @@ jobs:
1515
env:
1616
PR_BRANCH: ${{ github.event.pull_request.head.ref }}
1717
PR_BODY: ${{ github.event.pull_request.body }}
18+
PR_LABELS: ${{ toJson(github.event.pull_request.labels.*.name) }}
1819
run: |
19-
if [[ "$PR_BRANCH" == copilot* ]] || grep -iq "## manual acceptance tests" <<<"$PR_BODY"; then
20+
if python3 -c "
21+
import json, os, sys
22+
try:
23+
labels = json.loads(os.environ.get('PR_LABELS', '[]'))
24+
sys.exit(0 if 'design' in labels else 1)
25+
except Exception as e:
26+
print(f'Error parsing PR labels: {e}', file=sys.stderr)
27+
sys.exit(1)
28+
"; then
29+
echo "enforce=false" >> "$GITHUB_OUTPUT"
30+
elif [[ "$PR_BRANCH" == copilot* ]] || grep -iq "## manual acceptance tests" <<<"$PR_BODY"; then
2031
echo "enforce=true" >> "$GITHUB_OUTPUT"
2132
else
2233
echo "enforce=false" >> "$GITHUB_OUTPUT"
2334
fi
2435
- name: Skip (no enforcement needed)
2536
if: steps.decide.outputs.enforce == 'false'
26-
run: echo "Skipping manual acceptance test validation (not a Copilot PR and no section present)"
37+
run: echo "Skipping manual acceptance test validation (design PR, not a Copilot PR, or no section present)"
2738

2839
- name: Validate PR body
2940
if: steps.decide.outputs.enforce == 'true'

eslint.config.cjs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,33 @@ module.exports = [
114114
"max-lines": "off",
115115
"new-cap": ["error", { capIsNewExceptionPattern: "GET|PUT|POST|DELETE" }],
116116
"no-magic-numbers": "off",
117+
"no-restricted-imports": [
118+
"error",
119+
{
120+
paths: [
121+
{
122+
name: "fs",
123+
message:
124+
"Do not import 'fs' in tests. Use usingTemporaryFiles() from 'using-temporary-files' instead.",
125+
},
126+
{
127+
name: "node:fs",
128+
message:
129+
"Do not import 'node:fs' in tests. Use usingTemporaryFiles() from 'using-temporary-files' instead.",
130+
},
131+
{
132+
name: "fs/promises",
133+
message:
134+
"Do not import 'fs/promises' in tests. Use usingTemporaryFiles() from 'using-temporary-files' instead.",
135+
},
136+
{
137+
name: "node:fs/promises",
138+
message:
139+
"Do not import 'node:fs/promises' in tests. Use usingTemporaryFiles() from 'using-temporary-files' instead.",
140+
},
141+
],
142+
},
143+
],
117144
"no-shadow": "off",
118145
},
119146
},

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@
126126
"@apidevtools/json-schema-ref-parser": "13.0.5",
127127
"@hapi/accept": "6.0.3",
128128
"@types/json-schema": "7.0.15",
129-
"ajv": "6.14.0",
129+
"ajv": "8.18.0",
130130
"chokidar": "5.0.0",
131131
"commander": "14.0.3",
132132
"debug": "4.4.3",

src/server/request-validator.ts

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { OpenApiOperation } from "../counterfact-types/index.js";
44

55
const ajv = new Ajv({
66
allErrors: true,
7-
unknownFormats: "ignore",
7+
strict: false,
88
coerceTypes: false,
99
});
1010

@@ -57,10 +57,7 @@ export function validateRequest(
5757

5858
if (!valid && ajv.errors) {
5959
for (const error of ajv.errors) {
60-
const path =
61-
(error as { instancePath?: string }).instancePath ??
62-
error.dataPath ??
63-
"";
60+
const path = error.instancePath ?? "";
6461

6562
errors.push(`body${path} ${error.message ?? "is invalid"}`);
6663
}
@@ -78,10 +75,7 @@ export function validateRequest(
7875

7976
if (!valid && ajv.errors) {
8077
for (const error of ajv.errors) {
81-
const path =
82-
(error as { instancePath?: string }).instancePath ??
83-
error.dataPath ??
84-
"";
78+
const path = error.instancePath ?? "";
8579

8680
errors.push(`body${path} ${error.message ?? "is invalid"}`);
8781
}

test/migrate/update-route-types.test.ts

Lines changed: 17 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "@jest/globals";
2-
import { promises as fs } from "node:fs";
2+
33
import { usingTemporaryFiles } from "using-temporary-files";
44

55
// Import the main function - other functions are not exported so we test through integration
@@ -48,10 +48,7 @@ export const GET: HTTP_GET = ($) => {
4848

4949
expect(migrated).toBe(true);
5050

51-
const updatedContent = await fs.readFile(
52-
$.path("routes/pet/{id}.ts"),
53-
"utf8",
54-
);
51+
const updatedContent = await $.read("routes/pet/{id}.ts");
5552

5653
expect(updatedContent).toContain("import type { getPetById }");
5754
expect(updatedContent).toContain("export const GET: getPetById");
@@ -100,10 +97,7 @@ export const POST: HTTP_POST = ($) => {
10097

10198
expect(migrated).toBe(true);
10299

103-
const updatedContent = await fs.readFile(
104-
$.path("routes/pet.ts"),
105-
"utf8",
106-
);
100+
const updatedContent = await $.read("routes/pet.ts");
107101

108102
expect(updatedContent).toContain("import type { addPet }");
109103
expect(updatedContent).toContain("export const POST: addPet");
@@ -157,10 +151,7 @@ export const DELETE: HTTP_DELETE = ($) => {
157151

158152
expect(migrated).toBe(true);
159153

160-
const updatedContent = await fs.readFile(
161-
$.path("routes/pet/{id}.ts"),
162-
"utf8",
163-
);
154+
const updatedContent = await $.read("routes/pet/{id}.ts");
164155

165156
expect(updatedContent).toContain(
166157
"import type { getPetById, updatePet, deletePet }",
@@ -204,10 +195,7 @@ export const GET: HTTP_GET = ($) => {
204195

205196
expect(migrated).toBe(true);
206197

207-
const updatedContent = await fs.readFile(
208-
$.path("routes/index.ts"),
209-
"utf8",
210-
);
198+
const updatedContent = await $.read("routes/index.ts");
211199

212200
expect(updatedContent).toContain("import type { getRoot }");
213201
expect(updatedContent).toContain("export const GET: getRoot");
@@ -244,10 +232,7 @@ export const GET: HTTP_GET = ($) => {
244232
// Should return false because no changes needed (HTTP_GET stays HTTP_GET)
245233
expect(migrated).toBe(false);
246234

247-
const updatedContent = await fs.readFile(
248-
$.path("routes/anonymous.ts"),
249-
"utf8",
250-
);
235+
const updatedContent = await $.read("routes/anonymous.ts");
251236

252237
// File should remain unchanged
253238
expect(updatedContent).toContain("import type { HTTP_GET }");
@@ -287,10 +272,7 @@ export const GET: HTTP_GET = ($) => {
287272

288273
expect(migrated).toBe(true);
289274

290-
const updatedContent = await fs.readFile(
291-
$.path("routes/api/v1/users/{id}.ts"),
292-
"utf8",
293-
);
275+
const updatedContent = await $.read("routes/api/v1/users/{id}.ts");
294276

295277
expect(updatedContent).toContain("import type { getUserById }");
296278
expect(updatedContent).toContain("export const GET: getUserById");
@@ -346,21 +328,15 @@ export const GET: HTTP_GET = ($) => ({ status: 200 });`,
346328

347329
expect(migrated).toBe(true);
348330

349-
const petsContent = await fs.readFile($.path("routes/pets.ts"), "utf8");
331+
const petsContent = await $.read("routes/pets.ts");
350332
expect(petsContent).toContain("import type { listPets }");
351333
expect(petsContent).toContain("export const GET: listPets");
352334

353-
const petByIdContent = await fs.readFile(
354-
$.path("routes/pets/{id}.ts"),
355-
"utf8",
356-
);
335+
const petByIdContent = await $.read("routes/pets/{id}.ts");
357336
expect(petByIdContent).toContain("import type { getPetById }");
358337
expect(petByIdContent).toContain("export const GET: getPetById");
359338

360-
const usersContent = await fs.readFile(
361-
$.path("routes/users.ts"),
362-
"utf8",
363-
);
339+
const usersContent = await $.read("routes/users.ts");
364340
expect(usersContent).toContain("import type { listUsers }");
365341
expect(usersContent).toContain("export const GET: listUsers");
366342
});
@@ -399,10 +375,7 @@ export const GET: getPetById = ($) => {
399375
// Should return false because file is already migrated
400376
expect(migrated).toBe(false);
401377

402-
const updatedContent = await fs.readFile(
403-
$.path("routes/pet/{id}.ts"),
404-
"utf8",
405-
);
378+
const updatedContent = await $.read("routes/pet/{id}.ts");
406379

407380
// File should remain exactly the same
408381
expect(updatedContent).toBe(alreadyMigratedFile);
@@ -439,10 +412,7 @@ export const POST: HTTP_POST = ($) => {
439412
);
440413
expect(firstMigration).toBe(true);
441414

442-
const afterFirstMigration = await fs.readFile(
443-
$.path("routes/pet.ts"),
444-
"utf8",
445-
);
415+
const afterFirstMigration = await $.read("routes/pet.ts");
446416

447417
// Second migration
448418
const secondMigration = await updateRouteTypes(
@@ -451,10 +421,7 @@ export const POST: HTTP_POST = ($) => {
451421
);
452422
expect(secondMigration).toBe(false);
453423

454-
const afterSecondMigration = await fs.readFile(
455-
$.path("routes/pet.ts"),
456-
"utf8",
457-
);
424+
const afterSecondMigration = await $.read("routes/pet.ts");
458425

459426
// Content should be identical after both runs
460427
expect(afterFirstMigration).toBe(afterSecondMigration);
@@ -492,10 +459,7 @@ export const GET: HTTP_GET = ($) => ({ status: 200 });`,
492459
await $.add("routes/_.context.ts", contextFile);
493460
await updateRouteTypes($.path(""), $.path("openapi.json"));
494461

495-
const contextContent = await fs.readFile(
496-
$.path("routes/_.context.ts"),
497-
"utf8",
498-
);
462+
const contextContent = await $.read("routes/_.context.ts");
499463

500464
// Context file should remain unchanged
501465
expect(contextContent).toBe(contextFile);
@@ -558,7 +522,7 @@ export const config = {
558522

559523
expect(migrated).toBe(false);
560524

561-
const content = await fs.readFile($.path("routes/config.ts"), "utf8");
525+
const content = await $.read("routes/config.ts");
562526
expect(content).toBe(nonMigrationFile);
563527
});
564528
});
@@ -601,10 +565,7 @@ export const POST : HTTP_POST= ($) => {
601565

602566
expect(migrated).toBe(true);
603567

604-
const updatedContent = await fs.readFile(
605-
$.path("routes/pet.ts"),
606-
"utf8",
607-
);
568+
const updatedContent = await $.read("routes/pet.ts");
608569

609570
expect(updatedContent).toContain("getPet");
610571
expect(updatedContent).toContain("addPet");
@@ -671,10 +632,7 @@ export const OPTIONS: HTTP_OPTIONS = ($) => ({ status: 200 });
671632

672633
expect(migrated).toBe(true);
673634

674-
const updatedContent = await fs.readFile(
675-
$.path("routes/resource.ts"),
676-
"utf8",
677-
);
635+
const updatedContent = await $.read("routes/resource.ts");
678636

679637
expect(updatedContent).toContain("getResource");
680638
expect(updatedContent).toContain("createResource");

0 commit comments

Comments
 (0)