Skip to content

Commit b82e06b

Browse files
PaulJPhilpclaude
andcommitted
refactor: Code quality remediation for ep-cli and ep-admin
ep-cli fixes: - Fix version mismatch (constants.ts VERSION synced to package.json) - Migrate Install service from Context.GenericTag to Effect.Service pattern - Fix addFinalizer scope issue with Effect.scoped in pattern-repo-commands - Add Schema.decodeUnknown validation for JSON parsing in Install service - Remove dead require("effect-cli-tui") code and getTUISpinner from execution helpers - Create src/errors.ts with tagged error types (LintFailedError, etc.) - Consolidate colorize into utils.ts with NO_COLOR support - Convert skills-commands.ts from tabs to 2-space indentation - Replace forEach with for...of in linter service - Add readonly to linter types, fix TUIModule type, remove unused exports - Fix mutating .sort() in skill-generator.ts - Remove no-op validate-env.ts and unused TUI adapter files ep-admin fixes: - Fix version mismatch (constants.ts VERSION synced to package.json) - Remove dead require("effect-cli-tui") and getTUISpinner from execution helpers - Rewire Execution service to use TUIService instead of dead getTUISpinner - Add NO_COLOR support to colorize in utils.ts - Replace deprecated colors/levelColors re-exports with direct ANSI_COLORS imports - Fix forEach in display service, TUIModule type, @ts-ignore in autofix service - Remove unused EpAdminTUIAdapter and EpAdminExecutionTUIAdapter exports - Fix mutating .sort() in skill-generator.ts - Update tests and production runtime for TUIService dependency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a246464 commit b82e06b

47 files changed

Lines changed: 500 additions & 717 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

packages/ep-admin/src/constants.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
export const CLI = {
1313
NAME: "ep-admin",
14-
VERSION: "0.4.1",
14+
VERSION: "0.2.0",
1515
DESCRIPTION: "Administrative CLI for Effect Patterns maintainers",
1616
RUNNER_NAME: "EffectPatterns Admin CLI",
1717
} as const;

packages/ep-admin/src/runtime/production.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { Execution } from "../services/execution/index.js";
2121
import { EnhancedFileSystem } from "../services/filesystem/index.js";
2222
import { Logger } from "../services/logger/index.js";
2323
import { McpService } from "../services/mcp/service.js";
24+
import { TUIService } from "../services/tui/service.js";
2425
import { ValidationService } from "../services/validation/index.js";
2526

2627
/**
@@ -45,7 +46,8 @@ export const ProductionLayer = Layer.mergeAll(
4546
NodeFileSystemLayer,
4647
Logger.Default,
4748
Layer.provide(Display.Default, Logger.Default),
48-
Layer.provide(Execution.Default, Logger.Default),
49+
TUIService.Default,
50+
Layer.provide(Execution.Default, Layer.mergeAll(Logger.Default, TUIService.Default)),
4951
Layer.provide(EnhancedFileSystem.Default, NodeFileSystemLayer),
5052
Layer.provide(ValidationService.Default, NodeFileSystemLayer),
5153
McpLayer,

packages/ep-admin/src/services/autofix/service.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ const callGemini = (
4444
}
4545

4646
const json = yield* response.json;
47-
// @ts-ignore
48-
const text = json.candidates?.[0]?.content?.parts?.[0]?.text;
49-
return text as string | undefined;
47+
const candidates = (json as Record<string, unknown>).candidates as Array<Record<string, unknown>> | undefined;
48+
const content = candidates?.[0]?.content as Record<string, unknown> | undefined;
49+
const parts = content?.parts as Array<Record<string, unknown>> | undefined;
50+
return parts?.[0]?.text as string | undefined;
5051
});
5152

5253
export const DefaultAutofixService = Layer.effect(
@@ -83,20 +84,20 @@ export const DefaultAutofixService = Layer.effect(
8384

8485
// Handle report structure (adapt as needed)
8586
// Assumes prepublish-check output format
86-
// @ts-ignore
87-
const diagnostics = report.diagnostics || [];
87+
const diagnostics = ((report as Record<string, unknown>).diagnostics ?? []) as Array<Record<string, unknown>>;
8888
if (diagnostics.length === 0) {
8989
yield* display.showInfo("No errors found in report.");
9090
return;
9191
}
9292

9393
// Group by file
94-
const filesMap = new Map<string, any[]>();
94+
const filesMap = new Map<string, Array<Record<string, unknown>>>();
9595
for (const d of diagnostics) {
9696
if (!d.file) continue;
97-
const existing = filesMap.get(d.file) || [];
97+
const filePath = String(d.file);
98+
const existing = filesMap.get(filePath) || [];
9899
existing.push(d);
99-
filesMap.set(d.file, existing);
100+
filesMap.set(filePath, existing);
100101
}
101102

102103
let processed = 0;

packages/ep-admin/src/services/display/__tests__/helpers.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Effect, Layer } from "effect";
66
import { describe, expect, it, vi, beforeEach } from "vitest";
77
import { colorizeWithConfig, getLoggerConfig } from "../helpers.js";
88
import { Logger, LoggerDefault } from "../../logger/index.js";
9-
import { colors } from "../../../utils.js";
9+
import { ANSI_COLORS } from "../../../constants.js";
1010

1111
describe("Display Helpers", () => {
1212
describe("colorizeWithConfig", () => {
@@ -19,8 +19,8 @@ describe("Display Helpers", () => {
1919
};
2020

2121
const result = colorizeWithConfig("text", "GREEN", config);
22-
expect(result).toContain(colors.GREEN);
23-
expect(result).toContain(colors.RESET);
22+
expect(result).toContain(ANSI_COLORS.GREEN);
23+
expect(result).toContain(ANSI_COLORS.RESET);
2424
});
2525

2626
it("should not apply color when colors disabled", () => {
@@ -33,7 +33,7 @@ describe("Display Helpers", () => {
3333

3434
const result = colorizeWithConfig("text", "GREEN", config);
3535
expect(result).toBe("text");
36-
expect(result).not.toContain(colors.GREEN);
36+
expect(result).not.toContain(ANSI_COLORS.GREEN);
3737
});
3838

3939
it("should handle different color keys", () => {
@@ -48,9 +48,9 @@ describe("Display Helpers", () => {
4848
const blueResult = colorizeWithConfig("text", "BLUE", config);
4949
const yellowResult = colorizeWithConfig("text", "YELLOW", config);
5050

51-
expect(redResult).toContain(colors.RED);
52-
expect(blueResult).toContain(colors.BLUE);
53-
expect(yellowResult).toContain(colors.YELLOW);
51+
expect(redResult).toContain(ANSI_COLORS.RED);
52+
expect(blueResult).toContain(ANSI_COLORS.BLUE);
53+
expect(yellowResult).toContain(ANSI_COLORS.YELLOW);
5454
});
5555
});
5656

packages/ep-admin/src/services/display/helpers.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,10 @@
33
*/
44

55
import { Effect } from "effect";
6-
import { colors } from "../../utils.js";
6+
import { ANSI_COLORS } from "../../constants.js";
77
import type { LoggerConfig } from "../logger/index.js";
88
import { Logger } from "../logger/index.js";
99

10-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
11-
type TUIModule = any;
12-
13-
// TUI module state - loaded lazily on first use
14-
// TUI module state - loaded lazily on first use
15-
// Stores: null (not attempted), false (attempted but failed), or the module
16-
// let tuiModuleCache: TUIModule | false | null = null;
17-
18-
1910
/**
2011
* Get logger config from Logger service
2112
*/
@@ -31,9 +22,9 @@ export const getLoggerConfig = (): Effect.Effect<LoggerConfig, never, Logger> =>
3122
*/
3223
export const colorizeWithConfig = (
3324
text: string,
34-
color: keyof typeof colors,
25+
color: keyof typeof ANSI_COLORS,
3526
config: LoggerConfig
3627
): string => {
3728
if (!config.useColors) return text;
38-
return `${colors[color]}${text}${colors.RESET}`;
29+
return `${ANSI_COLORS[color]}${text}${ANSI_COLORS.RESET}`;
3930
};
Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
11
/**
22
* Display service barrel exports
3-
*
4-
* Re-exporting shared Display service with ep-admin specific TUI adapter
3+
*
4+
* Re-exporting shared Display service
55
*/
66

77
// Re-export everything from shared services
88
export * from "@effect-patterns/ep-shared-services/display";
9-
10-
// Export ep-admin specific TUI adapter
11-
export { EpAdminTUIAdapter } from "./tui-adapter.js";
12-

packages/ep-admin/src/services/display/service.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,9 @@ export const Display = Effect.Service<DisplayService>()("Display", {
153153

154154
yield* Console.log(headers);
155155
yield* Console.log("─".repeat(headers.length));
156-
rows.forEach((row) => Console.log(row));
156+
for (const row of rows) {
157+
yield* Console.log(row);
158+
}
157159
}).pipe(
158160
Effect.mapError((error) => DisplayError.make(`Failed to show table`, error))
159161
) as Effect.Effect<void, DisplayServiceError>;

packages/ep-admin/src/services/display/tui-loader.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@
33
*/
44
import { Context, Effect, Layer } from "effect";
55

6-
// Define the shape of the TUI module
7-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
8-
export type TUIModule = any;
6+
// biome-ignore lint/suspicious/noExplicitAny: Dynamic TUI module shape is unknown at compile time
7+
export type TUIModule = Record<string, unknown>;
98

109
export class TUILoader extends Context.Tag("TUILoader")<
1110
TUILoader,

packages/ep-admin/src/services/execution/__tests__/execution.test.ts

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,26 @@ import { Logger } from "../../logger/index.js";
88
import { TUIService } from "../../tui/service.js";
99
import { Execution } from "../service.js";
1010

11+
const MockTUIService = Layer.succeed(
12+
TUIService,
13+
TUIService.of({
14+
load: () => Effect.succeed(null),
15+
isAvailable: () => Effect.succeed(false),
16+
clearCache: () => Effect.void,
17+
})
18+
);
19+
20+
const ExecutionTest = Execution.Default.pipe(
21+
Layer.provide(MockTUIService),
22+
Layer.provide(Logger.Default)
23+
);
24+
25+
const TestLayer = Layer.mergeAll(
26+
ExecutionTest,
27+
Logger.Default,
28+
MockTUIService
29+
);
30+
1131
describe("Execution Service", () => {
1232
describe("Service Methods", () => {
1333
it("should provide all execution methods", () =>
@@ -19,13 +39,7 @@ describe("Execution Service", () => {
1939
expect(execution.executeScriptStream).toBeDefined();
2040
expect(execution.withSpinner).toBeDefined();
2141
}).pipe(
22-
Effect.provide(
23-
Layer.mergeAll(
24-
Execution.Default,
25-
Logger.Default,
26-
TUIService.Default
27-
)
28-
),
42+
Effect.provide(TestLayer),
2943
Effect.runPromise
3044
));
3145
});
@@ -40,13 +54,7 @@ describe("Execution Service", () => {
4054
);
4155
expect(result).toBe("test result");
4256
}).pipe(
43-
Effect.provide(
44-
Layer.mergeAll(
45-
Execution.Default,
46-
Logger.Default,
47-
TUIService.Default
48-
)
49-
),
57+
Effect.provide(TestLayer),
5058
Effect.runPromise
5159
));
5260

@@ -61,15 +69,7 @@ describe("Execution Service", () => {
6169

6270
await expect(
6371
Effect.runPromise(
64-
program.pipe(
65-
Effect.provide(
66-
Layer.mergeAll(
67-
Execution.Default,
68-
Logger.Default,
69-
TUIService.Default
70-
)
71-
)
72-
)
72+
program.pipe(Effect.provide(TestLayer))
7373
)
7474
).rejects.toThrow("Task failed");
7575
});

packages/ep-admin/src/services/execution/__tests__/helpers.test.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import { Effect } from "effect";
66
import { describe, expect, it } from "vitest";
7-
import { getTUISpinner, withSpinner } from "../helpers.js";
7+
import { withSpinner } from "../helpers.js";
88

99
describe("Execution Helpers", () => {
1010
describe("withSpinner", () => {
@@ -25,11 +25,4 @@ describe("Execution Helpers", () => {
2525
});
2626
});
2727

28-
describe("getTUISpinner", () => {
29-
it("should return TUI spinner info", () => {
30-
const result = getTUISpinner();
31-
expect(result).toHaveProperty("spinnerEffectTUI");
32-
expect(result).toHaveProperty("InkService");
33-
});
34-
});
3528
});

0 commit comments

Comments
 (0)