Skip to content

Commit 7160d1a

Browse files
refactor(core): enforce the resource boundary and make two branches reachable
Layering is now a declared allow-list rather than an unwritten convention: projects may reach environments, deployments and git because creating a project genuinely creates all three, and nothing else may reach anything. The check covers test files too, which is where the last two edges hid. Both crossings are gone. The command and its test import through the resource that owns the composition, and core's test asserts the api surface's contract instead of a resource's concrete class. Two paths could not be covered as written. The retry loop had no exit condition, so its closing brace was unreachable; it is now bounded, with a final attempt after the last delay. The listen-failure mapping was a ternary reachable only through a real socket error, so it is now a function both of whose branches are tested directly, and serve returns the server it already had. #claude_code# 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 1a4c4d6 commit 7160d1a

7 files changed

Lines changed: 192 additions & 22 deletions

File tree

‎src/commands/launch/projects/create.test.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
import { DEPLOYMENT_WAIT_TIMEOUT_MS } from '../../../deployments/deployment.watcher';
2-
import { ProjectCreator } from '../../../projects/project.create';
3-
import { serverCommandFrameworkGate } from '../../../environments/environment.inputs';
1+
import {
2+
DEPLOYMENT_WAIT_TIMEOUT_MS,
3+
ProjectCreator,
4+
serverCommandFrameworkGate,
5+
} from '../../../projects/project.create';
46
import ProjectsCreate from './create';
57

68
function commandUnderTest(resolved: Record<string, unknown>, dataDir = '/tmp/site') {

‎src/commands/launch/projects/create.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import { LaunchCommand } from '../../../core/launch-command';
22
import { flagsFor, inputs } from '../../../core/inputs';
3-
import { defaultWatchTiming } from '../../../deployments/deployment.watcher';
4-
import { serverCommandFrameworkGate } from '../../../environments/environment.inputs';
5-
import { ProjectCreator } from '../../../projects/project.create';
3+
import { ProjectCreator, defaultWatchTiming, serverCommandFrameworkGate } from '../../../projects/project.create';
64

75
const createInputs = inputs({
86
org: { required: true },

‎src/core/layering.test.ts‎

Lines changed: 134 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,81 @@
11
import { readFileSync, readdirSync } from 'node:fs';
2-
import { join } from 'node:path';
2+
import { join, relative, sep } from 'node:path';
33

44
const SRC = join(__dirname, '..');
55
const LAYERS = ['core', 'transport'];
66
const RESOURCES = ['projects', 'functions', 'environments', 'deployments', 'git'];
77

8+
const RESOURCE_EDGES: Record<string, string[]> = {
9+
projects: ['environments', 'deployments', 'git'],
10+
functions: [],
11+
environments: [],
12+
deployments: [],
13+
git: [],
14+
};
15+
16+
function filesUnder(directory: string): string[] {
17+
return readdirSync(join(SRC, directory), { withFileTypes: true }).flatMap((entry) => {
18+
const child = join(directory, entry.name);
19+
20+
if (entry.isDirectory()) {
21+
return filesUnder(child);
22+
}
23+
24+
return entry.name.endsWith('.ts') ? [child] : [];
25+
});
26+
}
27+
828
function sourceFilesIn(layer: string): string[] {
9-
return readdirSync(join(SRC, layer))
10-
.filter((entry) => entry.endsWith('.ts') && !entry.endsWith('.test.ts'))
11-
.map((entry) => join(layer, entry));
29+
return filesUnder(layer).filter((path) => !path.endsWith('.test.ts'));
1230
}
1331

1432
function importedPaths(relativePath: string): string[] {
1533
const contents = readFileSync(join(SRC, relativePath), 'utf8');
1634
return [...contents.matchAll(/from '([^']+)'/g)].map((match) => match[1]);
1735
}
1836

37+
function resourceOf(specifier: string, fromDirectory: string): string | undefined {
38+
if (!specifier.startsWith('.')) {
39+
return undefined;
40+
}
41+
42+
const resolved = relative(SRC, join(SRC, fromDirectory, specifier));
43+
const head = resolved.split(sep)[0];
44+
45+
return RESOURCES.includes(head) ? head : undefined;
46+
}
47+
48+
function directoryOf(relativePath: string): string {
49+
const parts = relativePath.split(sep);
50+
return parts.slice(0, -1).join(sep);
51+
}
52+
53+
function crossResourceEdges(relativePath: string): string[] {
54+
const directory = directoryOf(relativePath);
55+
const owner = directory.split(sep)[0];
56+
57+
return importedPaths(relativePath)
58+
.map((specifier) => resourceOf(specifier, directory))
59+
.filter((target): target is string => target !== undefined && target !== owner);
60+
}
61+
62+
function commandEdges(relativePath: string): string[] {
63+
const owner = relativePath.split(sep)[2];
64+
const directory = directoryOf(relativePath);
65+
66+
return importedPaths(relativePath)
67+
.map((specifier) => resourceOf(specifier, directory))
68+
.filter((target): target is string => target !== undefined && target !== owner);
69+
}
70+
1971
function resourceImports(relativePath: string): string[] {
2072
return importedPaths(relativePath).filter((specifier) =>
2173
RESOURCES.some((resource) => specifier.startsWith(`../${resource}/`) || specifier === `../${resource}`),
2274
);
2375
}
2476

2577
describe('layering', () => {
26-
it.each(LAYERS.flatMap((layer) => sourceFilesIn(layer)))('%s imports no resource directly', (relativePath) => {
78+
it.each(LAYERS.flatMap((layer) => filesUnder(layer)))('%s imports no resource directly', (relativePath) => {
2779
expect(resourceImports(relativePath)).toEqual([]);
2880
});
2981

@@ -32,17 +84,91 @@ describe('layering', () => {
3284
expect(sourceFilesIn('transport').length).toBeGreaterThan(0);
3385
});
3486

87+
it('checks the test files in core and transport too, not only the production ones', () => {
88+
const checked = LAYERS.flatMap((layer) => filesUnder(layer));
89+
90+
expect(checked).toContain(join('core', 'layering.test.ts'));
91+
expect(checked.filter((path) => path.endsWith('.test.ts')).length).toBeGreaterThan(0);
92+
});
93+
3594
it('reports a resource import when one is present, so the check cannot pass vacuously', () => {
3695
const contrived = importedPaths(join('projects', 'project.inputs.ts'));
3796

3897
expect(contrived).toContain('../core/resolution');
39-
expect(
40-
contrived.filter((specifier) => RESOURCES.some((resource) => specifier.startsWith(`../${resource}/`))),
41-
).toEqual([]);
4298
expect(
4399
['../projects/types', './types'].filter((specifier) =>
44100
RESOURCES.some((resource) => specifier.startsWith(`../${resource}/`)),
45101
),
46102
).toEqual(['../projects/types']);
47103
});
48104
});
105+
106+
describe('layering between resources', () => {
107+
it.each(RESOURCES.flatMap((resource) => filesUnder(resource)))(
108+
'%s reaches only the resources its allow-list names',
109+
(relativePath) => {
110+
const owner = relativePath.split(sep)[0];
111+
112+
expect(RESOURCE_EDGES[owner]).toBeDefined();
113+
for (const target of crossResourceEdges(relativePath)) {
114+
expect(RESOURCE_EDGES[owner]).toContain(target);
115+
}
116+
},
117+
);
118+
119+
it('declares an allow-list entry for every resource and invents none', () => {
120+
expect(Object.keys(RESOURCE_EDGES).sort()).toEqual([...RESOURCES].sort());
121+
for (const targets of Object.values(RESOURCE_EDGES)) {
122+
for (const target of targets) {
123+
expect(RESOURCES).toContain(target);
124+
}
125+
}
126+
});
127+
128+
it('records the edges POST /projects really creates, and no others', () => {
129+
expect(RESOURCE_EDGES.projects).toEqual(['environments', 'deployments', 'git']);
130+
expect(crossResourceEdges(join('projects', 'project.create.ts')).sort()).toEqual([
131+
'deployments',
132+
'deployments',
133+
'deployments',
134+
'deployments',
135+
'deployments',
136+
'environments',
137+
'environments',
138+
'environments',
139+
'environments',
140+
'git',
141+
]);
142+
});
143+
144+
it('reports a disallowed edge rather than passing vacuously', () => {
145+
const owner = 'git';
146+
147+
expect(crossResourceEdges(join('projects', 'project.create.ts')).length).toBeGreaterThan(0);
148+
expect(RESOURCE_EDGES[owner]).not.toContain('projects');
149+
expect(resourceOf('../projects/types', 'git')).toBe('projects');
150+
});
151+
});
152+
153+
describe('layering of the command files', () => {
154+
it.each(filesUnder(join('commands', 'launch')).filter((path) => path.split(sep).length > 3))(
155+
'%s reaches its own resource and no other',
156+
(relativePath) => {
157+
expect(commandEdges(relativePath)).toEqual([]);
158+
},
159+
);
160+
161+
it('finds the command files it claims to be checking', () => {
162+
const checked = filesUnder(join('commands', 'launch')).filter((path) => path.split(sep).length > 3);
163+
164+
expect(checked).toContain(join('commands', 'launch', 'projects', 'create.ts'));
165+
expect(checked).toContain(join('commands', 'launch', 'functions', 'serve.ts'));
166+
});
167+
168+
it('reports a command reaching another resource rather than passing vacuously', () => {
169+
expect(resourceOf('../../../deployments/deployment.watcher', join('commands', 'launch', 'projects'))).toBe(
170+
'deployments',
171+
);
172+
expect(resourceOf('../../../projects/project.create', join('commands', 'launch', 'projects'))).toBe('projects');
173+
});
174+
});

‎src/core/service-context.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { randomUUID } from 'node:crypto';
22

33
import { HttpClient, authHandler, configHandler } from '@contentstack/cli-utilities';
44

5-
import { ProjectsApi } from '../projects/projects.api';
65
import { SessionExpiredError, UnauthenticatedError } from './errors';
76
import { UxLike } from './render';
87
import { buildServiceContext } from './service-context';
@@ -66,7 +65,8 @@ describe('buildServiceContext', () => {
6665
isTTY: true,
6766
});
6867

69-
expect(built.api.projects).toBeInstanceOf(ProjectsApi);
68+
expect(typeof built.api.projects.list).toBe('function');
69+
expect(typeof built.api.projects.get).toBe('function');
7070
expect(built.ux).toBe(UX);
7171
expect(built.isTTY).toBe(true);
7272
configSpy.mockRestore();

‎src/functions/cloud-functions.test.ts‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { AddressInfo } from 'net';
66
import { tmpdir } from 'os';
77
import { dirname, join } from 'path';
88

9-
import { CloudFunctions } from './cloud-functions';
9+
import { CloudFunctions, listenFailure } from './cloud-functions';
1010
import { PortInUseError } from './function.errors';
1111
import { loadDataURL } from './load-data-url';
1212

@@ -347,6 +347,41 @@ describe('CloudFunctions serve failures', () => {
347347
expect((failure as Error).message).toContain('--port');
348348
});
349349

350+
it('maps a port clash to a port-in-use error naming the port', () => {
351+
const clash = Object.assign(new Error('listen EADDRINUSE'), { code: 'EADDRINUSE' });
352+
353+
const mapped = listenFailure(clash, 4321);
354+
355+
expect(mapped).toBeInstanceOf(PortInUseError);
356+
expect(mapped.message).toContain('4321');
357+
});
358+
359+
it('passes a listen failure that is not a port clash through untouched', () => {
360+
const other = Object.assign(new Error('listen EACCES'), { code: 'EACCES' });
361+
362+
const mapped = listenFailure(other, 4321);
363+
364+
expect(mapped).toBe(other);
365+
expect(mapped).not.toBeInstanceOf(PortInUseError);
366+
});
367+
368+
it('logs an error raised after the server is already listening rather than rejecting', async () => {
369+
writeFunctionFile('hello.js', 'export default function hello(request, response) { response.send("hi"); }');
370+
const logged: unknown[] = [];
371+
const errorSpy = jest.spyOn(console, 'error').mockImplementation((value: unknown) => {
372+
logged.push(value);
373+
});
374+
const server = (await new CloudFunctions(workspace).serve(await freePort())) as Server;
375+
const late = new Error('late failure');
376+
377+
server.emit('error', late);
378+
379+
expect(logged).toEqual([late]);
380+
381+
errorSpy.mockRestore();
382+
await new Promise<void>((resolve) => server.close(() => resolve()));
383+
});
384+
350385
it('propagates a listen failure that is not a port clash', async () => {
351386
writeFunctionFile('hello.js', 'export default function hello(request, response) { response.send("hi"); }');
352387

‎src/functions/cloud-functions.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import commonjs from '@rollup/plugin-commonjs';
2323
import json from '@rollup/plugin-json';
2424
import { loadDataURL } from './load-data-url';
2525

26+
export function listenFailure(error: NodeJS.ErrnoException, servingPort: number): Error {
27+
return error.code === 'EADDRINUSE' ? new PortInUseError(servingPort) : error;
28+
}
29+
2630
export class CloudFunctions {
2731
private cloudFunctionsDirectoryPath: string;
2832
private pathToSourceCode: string;
@@ -35,7 +39,7 @@ export class CloudFunctions {
3539
this.pathToSourceCode = pathToSourceCode;
3640
}
3741

38-
async serve(servingPort: number): Promise<void> {
42+
async serve(servingPort: number): Promise<Server | undefined> {
3943
const directoryExists = checkIfDirectoryExists(
4044
this.cloudFunctionsDirectoryPath
4145
);
@@ -68,7 +72,7 @@ export class CloudFunctions {
6872

6973
dotenv.config({ path: path.join(this.pathToSourceCode, ENV_FILE_NAME) });
7074

71-
await this.startServer(app, servingPort);
75+
return this.startServer(app, servingPort);
7276
}
7377

7478
private startServer(app: Express, servingPort: number): Promise<Server> {
@@ -87,7 +91,7 @@ export class CloudFunctions {
8791
return;
8892
}
8993

90-
reject(error.code === 'EADDRINUSE' ? new PortInUseError(servingPort) : error);
94+
reject(listenFailure(error, servingPort));
9195
});
9296
});
9397
}

‎src/projects/project.create.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ import { PROJECT_TYPE_BY_CHOICE, PROJECT_TYPE_CHOICES, ProjectTypeChoice, projec
2222
import { uploadArchive } from './project.upload';
2323
import type { CreateProjectInput, DetectedFramework, Project } from './types';
2424

25+
export { DEPLOYMENT_WAIT_TIMEOUT_MS, defaultWatchTiming } from '../deployments/deployment.watcher';
26+
export { serverCommandFrameworkGate } from '../environments/environment.inputs';
27+
2528
export const DEFAULT_OUTPUT_DIRECTORY = './';
2629
export const NO_DEPLOYMENT_STATUS = 'NONE';
2730
export const FIRST_LOOKUP_ATTEMPTS = 3;
@@ -170,15 +173,17 @@ export class ProjectCreator {
170173
}
171174

172175
private async appearing<T>(lookup: () => Promise<T | undefined>): Promise<T | undefined> {
173-
for (let attempt = 1; ; attempt += 1) {
176+
for (let attempt = 1; attempt < FIRST_LOOKUP_ATTEMPTS; attempt += 1) {
174177
const found = await lookup();
175178

176-
if (found !== undefined || attempt >= FIRST_LOOKUP_ATTEMPTS) {
179+
if (found !== undefined) {
177180
return found;
178181
}
179182

180183
await this.timing.sleep(this.timing.pollDelayMs);
181184
}
185+
186+
return lookup();
182187
}
183188

184189
private async follow(org: string, project: Project, envName: string): Promise<void> {

0 commit comments

Comments
 (0)