Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion scopes/harmony/diagnostic/diagnostic.graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ export class DiagnosticGraphql implements Schema {
resolvers = {
Query: {
_diagnostic: () => {
return this.diagnosticMain.getDiagnosticData();
// return this.diagnosticMain.getDiagnosticData();
return {};
Comment on lines +18 to +19
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The GraphQL resolver for _diagnostic has been commented out and now returns an empty object. This breaks the GraphQL API functionality. If the intention is to disable this temporarily, it should either be removed completely or have a clear explanation/TODO comment. If this is intentional and permanent, the schema and resolver should be removed entirely.

This change is not mentioned in the PR description and appears to be unrelated to adding the getProcessInfo diagnostic function.

Copilot uses AI. Check for mistakes.
},
},
};
Expand Down
36 changes: 35 additions & 1 deletion scopes/harmony/diagnostic/diagnostic.main.runtime.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { getBitVersion } from '@teambit/bit.get-bit-version';
import os from 'os';

import type { SlotRegistry } from '@teambit/harmony';
import { Slot } from '@teambit/harmony';
Expand Down Expand Up @@ -45,13 +46,46 @@ export class DiagnosticMain {
return { version };
}

static getProcessInfo() {
const memUsage = process.memoryUsage();
const cpuUsage = process.cpuUsage();
return {
uptime: process.uptime(),
pid: process.pid,
nodeVersion: process.version,
platform: process.platform,
arch: process.arch,
memory: {
rss: memUsage.rss,
heapTotal: memUsage.heapTotal,
heapUsed: memUsage.heapUsed,
external: memUsage.external,
arrayBuffers: memUsage.arrayBuffers,
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

process.memoryUsage().arrayBuffers is not available on all supported Node versions. This repo’s root package.json allows Node >=12.22.0, and on Node 12 memoryUsage() may not include arrayBuffers, making this field undefined at runtime. Consider making arrayBuffers optional/conditional (only include when present) or provide a safe fallback to avoid runtime incompatibility.

Suggested change
arrayBuffers: memUsage.arrayBuffers,
...(typeof (memUsage as any).arrayBuffers === 'number'
? { arrayBuffers: (memUsage as any).arrayBuffers }
: {}),

Copilot uses AI. Check for mistakes.
},
cpu: {
user: cpuUsage.user,
system: cpuUsage.system,
},
Comment on lines +49 to +68
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

getProcessInfo() exposes CPU usage as the raw output of process.cpuUsage() (user/system in microseconds). Without units in the field names, this is easy to misinterpret as percentages or milliseconds. Consider either converting to a clearer unit (e.g. milliseconds) or renaming/annotating the fields to include the unit so downstream consumers don’t misread the numbers.

Copilot uses AI. Check for mistakes.
system: {
totalMemory: os.totalmem(),
freeMemory: os.freemem(),
cpuCount: os.cpus().length,
loadAverage: os.loadavg(),
hostname: os.hostname(),
},
};
}

static async provider(
[express, graphql]: [ExpressMain, GraphqlMain],
config: any,
[diagnosticSlot]: [DiagnosticSlot]
) {
const diagnosticMain = new DiagnosticMain(diagnosticSlot);
diagnosticMain.register({ diagnosticFn: DiagnosticMain.getBitVersion });
diagnosticMain.register(
{ diagnosticFn: DiagnosticMain.getBitVersion },
{ diagnosticFn: DiagnosticMain.getProcessInfo }
);
express.register([new DiagnosticRoute(diagnosticMain)]);
graphql.register(() => new DiagnosticGraphql(diagnosticMain));
return diagnosticMain;
Expand Down
8 changes: 4 additions & 4 deletions scopes/harmony/diagnostic/diagnostic.route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Route, Request, Response } from '@teambit/express';
import type { Route, Request, Response, Middleware } from '@teambit/express';
import { Verb } from '@teambit/express';
import type { DiagnosticMain } from './diagnostic.main.runtime';

Expand All @@ -7,12 +7,12 @@ export class DiagnosticRoute implements Route {

method = 'GET';
route = '/_diagnostic';
verb = Verb.READ;
verb = Verb.WRITE;
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The verb should be Verb.READ instead of Verb.WRITE. The /_diagnostic endpoint only reads diagnostic data without modifying any state. Based on the codebase patterns:

  • Verb.READ is used for GET requests that query data without side effects (e.g., fetch routes, search routes, query routes)
  • Verb.WRITE is used for POST requests that modify state (e.g., create, delete, put routes)

This is a GET request that only reads diagnostic information, so it should use Verb.READ.

Suggested change
verb = Verb.WRITE;
verb = Verb.READ;

Copilot uses AI. Check for mistakes.

middlewares = [
middlewares: Middleware[] = [
async (req: Request, res: Response) => {
const diagnosticData = this.diagnosticMain.getDiagnosticData();
res.json(diagnosticData);
return res.json(diagnosticData);
},
];
}
76 changes: 76 additions & 0 deletions scopes/harmony/diagnostic/diagnostic.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import os from 'os';
import { expect } from 'chai';
import { DiagnosticMain } from './diagnostic.main.runtime';

describe('DiagnosticMain', () => {
describe('getBitVersion', () => {
it('should return an object with a version string', () => {
const result = DiagnosticMain.getBitVersion();
expect(result).to.have.property('version');
expect(result.version).to.be.a('string');
});
});

describe('getProcessInfo', () => {
it('should return process uptime as a positive number', () => {
const result = DiagnosticMain.getProcessInfo();
expect(result.uptime).to.be.a('number');
expect(result.uptime).to.be.greaterThan(0);
});
Comment on lines +15 to +19
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

Some assertions in these unit tests may be brittle across environments (e.g. process.uptime() can be extremely close to 0 early in the process lifetime; some memory fields can be 0 depending on runtime/Node/V8 behavior). To reduce flakiness, consider asserting >= 0 for uptime/memory and focusing on shape/type guarantees rather than strict positivity.

Copilot uses AI. Check for mistakes.

it('should return the current process pid', () => {
const result = DiagnosticMain.getProcessInfo();
expect(result.pid).to.equal(process.pid);
});

it('should return node version, platform, and arch', () => {
const result = DiagnosticMain.getProcessInfo();
expect(result.nodeVersion).to.equal(process.version);
expect(result.platform).to.equal(process.platform);
expect(result.arch).to.equal(process.arch);
});

it('should return memory usage with all expected fields', () => {
const result = DiagnosticMain.getProcessInfo();
expect(result.memory).to.have.property('rss').that.is.a('number');
expect(result.memory).to.have.property('heapTotal').that.is.a('number');
expect(result.memory).to.have.property('heapUsed').that.is.a('number');
expect(result.memory).to.have.property('external').that.is.a('number');
expect(result.memory).to.have.property('arrayBuffers').that.is.a('number');
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

This test expects memory.arrayBuffers to exist and be a number. Since the repo supports Node >=12.22.0, and process.memoryUsage() may not provide arrayBuffers on older Node versions, the test can fail even if the diagnostic is otherwise correct. Consider making the assertion conditional (only when the field exists) or treating it as optional.

Suggested change
expect(result.memory).to.have.property('arrayBuffers').that.is.a('number');
if ('arrayBuffers' in result.memory && result.memory.arrayBuffers !== undefined) {
expect(result.memory.arrayBuffers).to.be.a('number');
}

Copilot uses AI. Check for mistakes.
});

it('should return memory values that are positive', () => {
const result = DiagnosticMain.getProcessInfo();
expect(result.memory.rss).to.be.greaterThan(0);
expect(result.memory.heapTotal).to.be.greaterThan(0);
expect(result.memory.heapUsed).to.be.greaterThan(0);
Comment on lines +44 to +46
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

These tests assume several memory fields are strictly > 0 (rss, heapTotal, heapUsed). Depending on Node/V8 and the runtime environment, some of these can be 0 (especially in minimal/embedded environments), which can cause unnecessary flakes. Prefer >= 0 (or only assert presence/type) unless strict positivity is required by the contract.

Suggested change
expect(result.memory.rss).to.be.greaterThan(0);
expect(result.memory.heapTotal).to.be.greaterThan(0);
expect(result.memory.heapUsed).to.be.greaterThan(0);
expect(result.memory.rss).to.be.at.least(0);
expect(result.memory.heapTotal).to.be.at.least(0);
expect(result.memory.heapUsed).to.be.at.least(0);

Copilot uses AI. Check for mistakes.
});

it('should return cpu usage with user and system fields', () => {
const result = DiagnosticMain.getProcessInfo();
expect(result.cpu).to.have.property('user').that.is.a('number');
expect(result.cpu).to.have.property('system').that.is.a('number');
});

it('should return system info matching os module values', () => {
const result = DiagnosticMain.getProcessInfo();
expect(result.system.totalMemory).to.equal(os.totalmem());
expect(result.system.cpuCount).to.equal(os.cpus().length);
expect(result.system.hostname).to.equal(os.hostname());
});

it('should return load average as an array of 3 numbers', () => {
const result = DiagnosticMain.getProcessInfo();
expect(result.system.loadAverage).to.be.an('array').with.lengthOf(3);
result.system.loadAverage.forEach((val) => {
expect(val).to.be.a('number');
});
});

it('should return free memory less than or equal to total memory', () => {
const result = DiagnosticMain.getProcessInfo();
expect(result.system.freeMemory).to.be.at.most(result.system.totalMemory);
expect(result.system.freeMemory).to.be.greaterThan(0);
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

This test asserts freeMemory is > 0. In some constrained/containerized environments it may be possible to observe 0 (or near-0) free memory, which would make the test flaky even though the implementation is fine. Consider asserting >= 0 and <= totalMemory only.

Suggested change
expect(result.system.freeMemory).to.be.greaterThan(0);
expect(result.system.freeMemory).to.be.at.least(0);

Copilot uses AI. Check for mistakes.
});
});
});
3 changes: 1 addition & 2 deletions scopes/harmony/express/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
export type { RouteSlot } from './express.main.runtime';
export type { Route } from './types';
export { Verb } from './types';
export type { Request, Response, NextFunction, Middleware } from './types';
export type { Route, Request, Response, NextFunction, Middleware } from './types';
export type { MiddlewareManifest } from './middleware-manifest';
export type { ExpressMain } from './express.main.runtime';
export { ExpressAspect } from './express.aspect';
9 changes: 5 additions & 4 deletions scopes/harmony/express/types/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { Request } from './request';
export { Response } from './response';
export { NextFunction } from './next';
export { Route, Middleware, Verb } from './route';
export type { Request } from './request';
export type { Response } from './response';
export type { NextFunction } from './next';
export type { Route, Middleware } from './route';
export { Verb } from './route';
2 changes: 1 addition & 1 deletion scopes/harmony/graphql/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export { GraphqlAspect, GraphqlAspect as default } from './graphql.aspect';

export type { Schema } from './schema';
export type { Schema, DocumentNode, SchemaDirectives } from './schema';
export type { GraphqlMain, SchemaSlot } from './graphql.main.runtime';
export type { GraphqlUI, GraphQLClient } from './graphql.ui.runtime';
export type { GraphQLServer } from './graphql-server';
Expand Down
3 changes: 3 additions & 0 deletions scopes/harmony/graphql/schema.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import type { DocumentNode } from 'graphql';
import type { SchemaDirectives } from '@graphql-modules/core';

export type { DocumentNode } from 'graphql';
export type { SchemaDirectives } from '@graphql-modules/core';
Comment on lines +4 to +5
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

schema.ts now both imports DocumentNode/SchemaDirectives for local use and re-exports them from their original modules. This duplicates module references and can be simplified by exporting the already-imported types (or by using import('...').TypeName in the Schema type and keeping only the re-export). Simplifying keeps the file clearer and avoids redundant statements.

Suggested change
export type { DocumentNode } from 'graphql';
export type { SchemaDirectives } from '@graphql-modules/core';
export type { DocumentNode, SchemaDirectives };

Copilot uses AI. Check for mistakes.

/**
* graphql schema for an extension.
*/
Expand Down