Skip to content

Commit 8d2a69a

Browse files
alexey1312alexey1312
committed
fix(lint): improve error handling, type safety, and Slack notification paths
- Add core.warning() in parseLintOutput for parse failures instead of silent null - Type lintSeverity as union ('error' | 'warning' | 'info') with input validation - Skip parseExFigOutput regex for lint command (use empty metrics) - Fix Slack success branch to check lintErrors > 0 before showing "Lint passed" - Expand Slack failure branch to show "Lint failed" even when JSON parsing fails - Export parseLintOutput and add 6 unit tests (valid, zero, missing fields, invalid) - Add buildCommand test for lint with default inputs (no --rules flag) - Update CLAUDE.md with new command checklist and node24 runtime
1 parent 4c79479 commit 8d2a69a

6 files changed

Lines changed: 199 additions & 34 deletions

File tree

‎AGENTS.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ ExFig Action is a TypeScript-based GitHub Action that exports Figma assets using
3030

3131
**Key Files:**
3232

33-
- `action.yml` - Action definition (inputs/outputs, runs: node22)
33+
- `action.yml` - Action definition (inputs/outputs, runs: node24)
3434
- `src/index.ts` - Main logic (~1050 lines)
3535
- `src/types.ts` - TypeScript interfaces
3636
- `dist/index.js` - Compiled bundle (committed)
@@ -71,9 +71,11 @@ Single Node.js process executes: validate inputs → resolve version → cache/d
7171

7272
**Slack notification:** Uses raw `https.request` (no libraries). All errors are non-fatal (`resolve()`, never `reject()`). Always `res.resume()` response body to avoid holding the socket open.
7373

74+
**New command checklist:** When adding a new ExFig command: (1) add to `ExFigCommand` type + `VALID_COMMANDS`, (2) add command-specific parsing in `run()`, (3) skip `parseExFigOutput` regex if output format differs, (4) add both success AND failure paths in `handleSlackNotification`, (5) export parser function and add tests.
75+
7476
## Testing
7577

76-
- Unit tests: `parseExFigOutput`, `categorizeError`, `formatSlackMention`, `buildCommand`, `getPklBinaryName`, `detectCrash`, `parseReportFile`
78+
- Unit tests: `parseExFigOutput`, `parseLintOutput`, `categorizeError`, `formatSlackMention`, `buildCommand`, `getPklBinaryName`, `detectCrash`, `parseReportFile`
7779
- E2E: `.github/workflows/test.yml` (build, version resolution, binary download)
7880

7981
## Release

‎dist/index.js‎

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39074,6 +39074,7 @@ __nccwpck_require__.d(__webpack_exports__, {
3907439074
getPklBinaryName: () => (/* binding */ getPklBinaryName),
3907539075
isValidCommand: () => (/* binding */ isValidCommand),
3907639076
parseExFigOutput: () => (/* binding */ parseExFigOutput),
39077+
parseLintOutput: () => (/* binding */ parseLintOutput),
3907739078
parseReportFile: () => (/* binding */ parseReportFile),
3907839079
validatePlatform: () => (/* binding */ validatePlatform)
3907939080
});
@@ -86713,6 +86714,14 @@ const REPORT_COMMANDS = [
8671386714
// =============================================================================
8671486715
// Input Parsing
8671586716
// =============================================================================
86717+
const VALID_LINT_SEVERITIES = ['error', 'warning', 'info'];
86718+
function validateLintSeverity(raw) {
86719+
if (!raw)
86720+
return 'info';
86721+
if (VALID_LINT_SEVERITIES.includes(raw))
86722+
return raw;
86723+
throw new Error(`Invalid lint_severity '${raw}'. Must be one of: ${VALID_LINT_SEVERITIES.join(', ')}`);
86724+
}
8671686725
function getInputs() {
8671786726
const command = getInput('command', { required: true });
8671886727
if (!isValidCommand(command)) {
@@ -86738,7 +86747,7 @@ function getInputs() {
8673886747
verbose: getBooleanInput('verbose'),
8673986748
extraArgs: getInput('extra_args'),
8674086749
lintRules: getInput('lint_rules'),
86741-
lintSeverity: getInput('lint_severity') || 'info',
86750+
lintSeverity: validateLintSeverity(getInput('lint_severity')),
8674286751
slackWebhook: getInput('slack_webhook'),
8674386752
slackMention: getInput('slack_mention'),
8674486753
slackTemplates: getInput('slack_templates'),
@@ -87077,9 +87086,7 @@ function buildCommand(inputs) {
8707787086
if (inputs.lintRules) {
8707887087
args.push('--rules', inputs.lintRules);
8707987088
}
87080-
if (inputs.lintSeverity) {
87081-
args.push('--severity', inputs.lintSeverity);
87082-
}
87089+
args.push('--severity', inputs.lintSeverity);
8708387090
}
8708487091
// Add rate limit and retries
8708587092
args.push('--rate-limit', inputs.rateLimit.toString());
@@ -87259,12 +87266,17 @@ function parseSingleReport(report) {
8725987266
function parseLintOutput(stdout) {
8726087267
try {
8726187268
const report = JSON.parse(stdout);
87262-
if (typeof report.diagnosticsCount === 'number') {
87269+
if (typeof report.diagnosticsCount === 'number' && Array.isArray(report.diagnostics)) {
8726387270
return report;
8726487271
}
87272+
warning(`Lint output JSON is missing expected fields. ` +
87273+
`Raw output (first 200 chars): ${stdout.substring(0, 200)}`);
8726587274
return null;
8726687275
}
87267-
catch {
87276+
catch (error) {
87277+
const msg = error instanceof Error ? error.message : String(error);
87278+
warning(`Failed to parse lint JSON output: ${msg}. ` +
87279+
`Raw output (first 200 chars): ${stdout.substring(0, 200)}`);
8726887280
return null;
8726987281
}
8727087282
}
@@ -87469,10 +87481,23 @@ async function run() {
8746987481
outputs.lintWarnings = lintResult.warningsCount;
8747087482
outputs.reportJson = result.stdout;
8747187483
}
87484+
else {
87485+
warning('Could not parse lint output as JSON. Lint results will not be available in outputs.');
87486+
}
8747287487
}
8747387488
// Parse output: prefer structured report when available, fallback to regex
8747487489
let metrics;
87475-
if (REPORT_COMMANDS.includes(inputs.command) && reportPath) {
87490+
if (inputs.command === 'lint') {
87491+
metrics = {
87492+
assetsExported: 0,
87493+
validatedCount: 0,
87494+
exportedConfigs: 0,
87495+
failedCount: 0,
87496+
errorMessage: '',
87497+
errorCategory: '',
87498+
};
87499+
}
87500+
else if (REPORT_COMMANDS.includes(inputs.command) && reportPath) {
8747687501
const reportMetrics = parseReportFile(reportPath);
8747787502
if (reportMetrics) {
8747887503
metrics = reportMetrics;
@@ -87604,11 +87629,21 @@ async function handleSlackNotification(inputs, outputs, context, version, platfo
8760487629
let templateName;
8760587630
if (outputs.exitCode === 0) {
8760687631
if (inputs.command === 'lint') {
87607-
color = '#36a64f';
87608-
icon = '\u2705';
87609-
title = 'Lint passed';
87610-
subtitle = outputs.lintWarnings > 0 ? `${outputs.lintWarnings} warning(s)` : 'All checks passed';
87611-
templateName = 'success.json';
87632+
if (outputs.lintErrors > 0) {
87633+
color = '#dc3545';
87634+
icon = '\u274C';
87635+
title = 'Lint failed';
87636+
subtitle = `${outputs.lintErrors} error(s), ${outputs.lintWarnings} warning(s)`;
87637+
templateName = 'failure.json';
87638+
}
87639+
else {
87640+
color = '#36a64f';
87641+
icon = '\u2705';
87642+
title = 'Lint passed';
87643+
subtitle =
87644+
outputs.lintWarnings > 0 ? `${outputs.lintWarnings} warning(s)` : 'All checks passed';
87645+
templateName = 'success.json';
87646+
}
8761287647
}
8761387648
else if (outputs.validatedCount > 0 && outputs.assetsExported === 0) {
8761487649
color = '#36a64f';
@@ -87639,9 +87674,14 @@ async function handleSlackNotification(inputs, outputs, context, version, platfo
8763987674
subtitle = '[CRASH] Process killed by signal - check workflow logs';
8764087675
}
8764187676
}
87642-
else if (inputs.command === 'lint' && outputs.lintErrors > 0) {
87677+
else if (inputs.command === 'lint') {
8764387678
title = 'Lint failed';
87644-
subtitle = `${outputs.lintErrors} error(s), ${outputs.lintWarnings} warning(s)`;
87679+
if (outputs.lintErrors > 0) {
87680+
subtitle = `${outputs.lintErrors} error(s), ${outputs.lintWarnings} warning(s)`;
87681+
}
87682+
else {
87683+
subtitle = outputs.errorMessage || 'See workflow logs for details';
87684+
}
8764587685
}
8764687686
else if (outputs.failedCount > 0) {
8764787687
title = 'Export failed';

‎dist/index.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/index.test.ts‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
parseExFigOutput,
33
parseReportFile,
4+
parseLintOutput,
45
categorizeError,
56
detectCrash,
67
formatSlackMention,
@@ -104,6 +105,72 @@ Batch complete: 1 succeeded, 1 failed
104105
});
105106
});
106107

108+
describe('parseLintOutput', () => {
109+
it('should parse valid lint JSON', () => {
110+
const json = JSON.stringify({
111+
diagnosticsCount: 2,
112+
errorsCount: 1,
113+
warningsCount: 1,
114+
diagnostics: [
115+
{
116+
ruleId: 'naming-convention',
117+
ruleName: 'Naming Convention',
118+
severity: 'error',
119+
message: 'Bad name',
120+
componentName: 'Button',
121+
nodeId: '1:2',
122+
suggestion: 'Rename it',
123+
},
124+
{
125+
ruleId: 'deleted-variables',
126+
ruleName: 'Deleted Variables',
127+
severity: 'warning',
128+
message: 'Var deleted',
129+
componentName: null,
130+
nodeId: null,
131+
suggestion: null,
132+
},
133+
],
134+
});
135+
const result = parseLintOutput(json);
136+
expect(result).not.toBeNull();
137+
expect(result!.errorsCount).toBe(1);
138+
expect(result!.warningsCount).toBe(1);
139+
expect(result!.diagnostics).toHaveLength(2);
140+
});
141+
142+
it('should parse valid lint JSON with zero diagnostics', () => {
143+
const json = JSON.stringify({
144+
diagnosticsCount: 0,
145+
errorsCount: 0,
146+
warningsCount: 0,
147+
diagnostics: [],
148+
});
149+
const result = parseLintOutput(json);
150+
expect(result).not.toBeNull();
151+
expect(result!.diagnosticsCount).toBe(0);
152+
expect(result!.diagnostics).toHaveLength(0);
153+
});
154+
155+
it('should return null for missing diagnosticsCount', () => {
156+
const json = JSON.stringify({ errorsCount: 0, warningsCount: 0, diagnostics: [] });
157+
expect(parseLintOutput(json)).toBeNull();
158+
});
159+
160+
it('should return null for missing diagnostics array', () => {
161+
const json = JSON.stringify({ diagnosticsCount: 0, errorsCount: 0, warningsCount: 0 });
162+
expect(parseLintOutput(json)).toBeNull();
163+
});
164+
165+
it('should return null for invalid JSON', () => {
166+
expect(parseLintOutput('not json at all')).toBeNull();
167+
});
168+
169+
it('should return null for empty string', () => {
170+
expect(parseLintOutput('')).toBeNull();
171+
});
172+
});
173+
107174
describe('categorizeError', () => {
108175
it('should categorize rate limit errors', () => {
109176
expect(categorizeError('Rate limit exceeded')).toBe('RATE_LIMIT');
@@ -417,6 +484,16 @@ describe('buildCommand', () => {
417484
expect(args).toContain('error');
418485
});
419486

487+
it('should add --format and --severity but not --rules for lint with defaults', () => {
488+
mockExistsSync.mockReturnValue(true);
489+
const { args } = buildCommand({ ...baseInputs, command: 'lint' });
490+
expect(args).toContain('--format');
491+
expect(args).toContain('json');
492+
expect(args).toContain('--severity');
493+
expect(args).toContain('info');
494+
expect(args).not.toContain('--rules');
495+
});
496+
420497
it('should not inject --report for lint command', () => {
421498
mockExistsSync.mockReturnValue(true);
422499
const { args, reportPath } = buildCommand({ ...baseInputs, command: 'lint' });

‎src/index.ts‎

Lines changed: 59 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import type {
1111
ActionInputs,
1212
ActionOutputs,
1313
ExFigCommand,
14+
LintSeverity,
1415
Platform,
1516
RunnerOS,
1617
ExFigResult,
@@ -55,6 +56,16 @@ const REPORT_COMMANDS: readonly ExFigCommand[] = [
5556
// Input Parsing
5657
// =============================================================================
5758

59+
const VALID_LINT_SEVERITIES: readonly LintSeverity[] = ['error', 'warning', 'info'];
60+
61+
function validateLintSeverity(raw: string): LintSeverity {
62+
if (!raw) return 'info';
63+
if (VALID_LINT_SEVERITIES.includes(raw as LintSeverity)) return raw as LintSeverity;
64+
throw new Error(
65+
`Invalid lint_severity '${raw}'. Must be one of: ${VALID_LINT_SEVERITIES.join(', ')}`
66+
);
67+
}
68+
5869
function getInputs(): ActionInputs {
5970
const command = core.getInput('command', { required: true });
6071
if (!isValidCommand(command)) {
@@ -81,7 +92,7 @@ function getInputs(): ActionInputs {
8192
verbose: core.getBooleanInput('verbose'),
8293
extraArgs: core.getInput('extra_args'),
8394
lintRules: core.getInput('lint_rules'),
84-
lintSeverity: core.getInput('lint_severity') || 'info',
95+
lintSeverity: validateLintSeverity(core.getInput('lint_severity')),
8596
slackWebhook: core.getInput('slack_webhook'),
8697
slackMention: core.getInput('slack_mention'),
8798
slackTemplates: core.getInput('slack_templates'),
@@ -489,9 +500,7 @@ export function buildCommand(inputs: ActionInputs): {
489500
if (inputs.lintRules) {
490501
args.push('--rules', inputs.lintRules);
491502
}
492-
if (inputs.lintSeverity) {
493-
args.push('--severity', inputs.lintSeverity);
494-
}
503+
args.push('--severity', inputs.lintSeverity);
495504
}
496505

497506
// Add rate limit and retries
@@ -701,14 +710,23 @@ function parseSingleReport(report: ExportReport): ExFigMetrics {
701710
};
702711
}
703712

704-
function parseLintOutput(stdout: string): LintReport | null {
713+
export function parseLintOutput(stdout: string): LintReport | null {
705714
try {
706715
const report = JSON.parse(stdout) as LintReport;
707-
if (typeof report.diagnosticsCount === 'number') {
716+
if (typeof report.diagnosticsCount === 'number' && Array.isArray(report.diagnostics)) {
708717
return report;
709718
}
719+
core.warning(
720+
`Lint output JSON is missing expected fields. ` +
721+
`Raw output (first 200 chars): ${stdout.substring(0, 200)}`
722+
);
710723
return null;
711-
} catch {
724+
} catch (error) {
725+
const msg = error instanceof Error ? error.message : String(error);
726+
core.warning(
727+
`Failed to parse lint JSON output: ${msg}. ` +
728+
`Raw output (first 200 chars): ${stdout.substring(0, 200)}`
729+
);
712730
return null;
713731
}
714732
}
@@ -945,12 +963,25 @@ async function run(): Promise<void> {
945963
outputs.lintErrors = lintResult.errorsCount;
946964
outputs.lintWarnings = lintResult.warningsCount;
947965
outputs.reportJson = result.stdout;
966+
} else {
967+
core.warning(
968+
'Could not parse lint output as JSON. Lint results will not be available in outputs.'
969+
);
948970
}
949971
}
950972

951973
// Parse output: prefer structured report when available, fallback to regex
952974
let metrics: ExFigMetrics;
953-
if (REPORT_COMMANDS.includes(inputs.command) && reportPath) {
975+
if (inputs.command === 'lint') {
976+
metrics = {
977+
assetsExported: 0,
978+
validatedCount: 0,
979+
exportedConfigs: 0,
980+
failedCount: 0,
981+
errorMessage: '',
982+
errorCategory: '',
983+
};
984+
} else if (REPORT_COMMANDS.includes(inputs.command) && reportPath) {
954985
const reportMetrics = parseReportFile(reportPath);
955986
if (reportMetrics) {
956987
metrics = reportMetrics;
@@ -1100,12 +1131,20 @@ async function handleSlackNotification(
11001131

11011132
if (outputs.exitCode === 0) {
11021133
if (inputs.command === 'lint') {
1103-
color = '#36a64f';
1104-
icon = '\u2705';
1105-
title = 'Lint passed';
1106-
subtitle =
1107-
outputs.lintWarnings > 0 ? `${outputs.lintWarnings} warning(s)` : 'All checks passed';
1108-
templateName = 'success.json';
1134+
if (outputs.lintErrors > 0) {
1135+
color = '#dc3545';
1136+
icon = '\u274C';
1137+
title = 'Lint failed';
1138+
subtitle = `${outputs.lintErrors} error(s), ${outputs.lintWarnings} warning(s)`;
1139+
templateName = 'failure.json';
1140+
} else {
1141+
color = '#36a64f';
1142+
icon = '\u2705';
1143+
title = 'Lint passed';
1144+
subtitle =
1145+
outputs.lintWarnings > 0 ? `${outputs.lintWarnings} warning(s)` : 'All checks passed';
1146+
templateName = 'success.json';
1147+
}
11091148
} else if (outputs.validatedCount > 0 && outputs.assetsExported === 0) {
11101149
color = '#36a64f';
11111150
icon = '\uD83D\uDCA8'; // dash
@@ -1132,9 +1171,13 @@ async function handleSlackNotification(
11321171
} else {
11331172
subtitle = '[CRASH] Process killed by signal - check workflow logs';
11341173
}
1135-
} else if (inputs.command === 'lint' && outputs.lintErrors > 0) {
1174+
} else if (inputs.command === 'lint') {
11361175
title = 'Lint failed';
1137-
subtitle = `${outputs.lintErrors} error(s), ${outputs.lintWarnings} warning(s)`;
1176+
if (outputs.lintErrors > 0) {
1177+
subtitle = `${outputs.lintErrors} error(s), ${outputs.lintWarnings} warning(s)`;
1178+
} else {
1179+
subtitle = outputs.errorMessage || 'See workflow logs for details';
1180+
}
11381181
} else if (outputs.failedCount > 0) {
11391182
title = 'Export failed';
11401183
if (outputs.errorMessage) {

0 commit comments

Comments
 (0)