Skip to content
Merged
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
14 changes: 9 additions & 5 deletions actions/setup/js/create_discussion.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,22 @@ function resolveCategoryId(categoryConfig, itemCategory, categories) {
const categoryToMatch = itemCategory || categoryConfig;

if (categoryToMatch) {
// Try to match against category IDs first
// Try to match against category IDs first (exact match, case-sensitive)
const categoryById = categories.find(cat => cat.id === categoryToMatch);
if (categoryById) {
return { id: categoryById.id, matchType: "id", name: categoryById.name };
}
// Try to match against category names
const categoryByName = categories.find(cat => cat.name === categoryToMatch);

// Normalize the category to match for case-insensitive comparison
const normalizedCategoryToMatch = categoryToMatch.toLowerCase();

// Try to match against category names (case-insensitive)
const categoryByName = categories.find(cat => cat.name.toLowerCase() === normalizedCategoryToMatch);
if (categoryByName) {
return { id: categoryByName.id, matchType: "name", name: categoryByName.name };
}
// Try to match against category slugs (routes)
const categoryBySlug = categories.find(cat => cat.slug === categoryToMatch);
// Try to match against category slugs (routes, case-insensitive)
const categoryBySlug = categories.find(cat => cat.slug.toLowerCase() === normalizedCategoryToMatch);
Comment on lines 65 to +81
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

resolveCategoryId now calls categoryToMatch.toLowerCase(), which will throw a TypeError if item.category (or config.category) is not a string (agent output can be non-string). Consider coercing to string (e.g., String(categoryToMatch)) or guarding with typeof categoryToMatch === "string" before lowercasing so invalid types fall back gracefully instead of crashing the handler.

See below for a potential fix:

    if (typeof categoryToMatch === "string") {
      // Normalize the category to match for case-insensitive comparison
      const normalizedCategoryToMatch = categoryToMatch.toLowerCase();

      // Try to match against category names (case-insensitive)
      const categoryByName = categories.find(cat => cat.name.toLowerCase() === normalizedCategoryToMatch);
      if (categoryByName) {
        return { id: categoryByName.id, matchType: "name", name: categoryByName.name };
      }
      // Try to match against category slugs (routes, case-insensitive)
      const categoryBySlug = categories.find(cat => cat.slug.toLowerCase() === normalizedCategoryToMatch);
      if (categoryBySlug) {
        return { id: categoryBySlug.id, matchType: "slug", name: categoryBySlug.name };
      }
    }

Copilot uses AI. Check for mistakes.
if (categoryBySlug) {
return { id: categoryBySlug.id, matchType: "slug", name: categoryBySlug.name };
}
Expand Down
274 changes: 274 additions & 0 deletions actions/setup/js/create_discussion_category_normalization.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,274 @@
// @ts-check
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import { createRequire } from "module";

const require = createRequire(import.meta.url);
const { main: createDiscussionMain } = require("./create_discussion.cjs");

describe("create_discussion category normalization", () => {
let mockGithub;
let mockCore;
let mockContext;
let mockExec;
let originalEnv;

beforeEach(() => {
// Save original environment
originalEnv = { ...process.env };

// Mock GitHub API with discussion categories
mockGithub = {
rest: {},
graphql: vi.fn().mockImplementation((query, variables) => {
// Handle repository query (fetch categories)
if (query.includes("discussionCategories")) {
return Promise.resolve({
repository: {
id: "R_test123",
discussionCategories: {
nodes: [
{
id: "DIC_kwDOGFsHUM4BsUn1",
name: "General",
slug: "general",
description: "General discussions",
},
{
id: "DIC_kwDOGFsHUM4BsUn2",
name: "Audits",
slug: "audits",
description: "Audit reports",
},
{
id: "DIC_kwDOGFsHUM4BsUn3",
name: "Research",
slug: "research",
description: "Research discussions",
},
],
},
},
});
}
// Handle create discussion mutation
if (query.includes("createDiscussion")) {
return Promise.resolve({
createDiscussion: {
discussion: {
id: "D_test456",
number: 42,
title: variables.title,
url: "https://github.com/test-owner/test-repo/discussions/42",
},
},
});
}
return Promise.reject(new Error("Unknown GraphQL query"));
}),
};

// Mock Core
mockCore = {
info: vi.fn(),
warning: vi.fn(),
error: vi.fn(),
setOutput: vi.fn(),
};

// Mock Context
mockContext = {
repo: { owner: "test-owner", repo: "test-repo" },
runId: 12345,
payload: {
repository: {
html_url: "https://github.com/test-owner/test-repo",
},
},
};

// Mock Exec
mockExec = {
exec: vi.fn().mockResolvedValue(0),
};

// Set globals
global.github = mockGithub;
global.core = mockCore;
global.context = mockContext;
global.exec = mockExec;

// Set required environment variables
process.env.GH_AW_WORKFLOW_NAME = "Test Workflow";
process.env.GH_AW_WORKFLOW_ID = "test-workflow";
process.env.GH_AW_WORKFLOW_SOURCE_URL = "https://github.com/owner/repo/blob/main/workflow.md";
process.env.GITHUB_SERVER_URL = "https://github.com";
});

afterEach(() => {
// Restore environment
process.env = originalEnv;
vi.clearAllMocks();
});

it("should match category name case-insensitively (lowercase config, capitalized repo)", async () => {
const handler = await createDiscussionMain({
max: 5,
category: "audits", // lowercase config
});

const result = await handler(
{
title: "Test Discussion",
body: "This is a test discussion.",
},
{}
);

expect(result.success).toBe(true);
expect(result.number).toBe(42);

// Verify the correct category ID was used (Audits with capital A)
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
expect(createMutationCall).toBeDefined();
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn2"); // Audits category
});

it("should match category name case-insensitively (capitalized config, capitalized repo)", async () => {
const handler = await createDiscussionMain({
max: 5,
category: "Audits", // Capitalized config (user error)
});

const result = await handler(
{
title: "Test Discussion",
body: "This is a test discussion.",
},
{}
);

expect(result.success).toBe(true);
expect(result.number).toBe(42);

// Verify the correct category ID was used
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
expect(createMutationCall).toBeDefined();
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn2"); // Audits category
});

it("should match category name case-insensitively (mixed case config)", async () => {
const handler = await createDiscussionMain({
max: 5,
category: "AuDiTs", // Mixed case (should still match)
});

const result = await handler(
{
title: "Test Discussion",
body: "This is a test discussion.",
},
{}
);

expect(result.success).toBe(true);
expect(result.number).toBe(42);

// Verify the correct category ID was used
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
expect(createMutationCall).toBeDefined();
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn2"); // Audits category
});

it("should match category slug case-insensitively", async () => {
const handler = await createDiscussionMain({
max: 5,
category: "RESEARCH", // Uppercase slug
});

const result = await handler(
{
title: "Test Discussion",
body: "This is a test discussion.",
},
{}
);

expect(result.success).toBe(true);
expect(result.number).toBe(42);

// Verify the correct category ID was used (Research)
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
expect(createMutationCall).toBeDefined();
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn3"); // Research category
});

it("should preserve category IDs (exact match, case-sensitive)", async () => {
const handler = await createDiscussionMain({
max: 5,
category: "DIC_kwDOGFsHUM4BsUn3", // Direct category ID
});

const result = await handler(
{
title: "Test Discussion",
body: "This is a test discussion.",
},
{}
);

expect(result.success).toBe(true);
expect(result.number).toBe(42);

// Verify the exact category ID was used
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
expect(createMutationCall).toBeDefined();
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn3");
});

it("should use item category over config category (case-insensitive)", async () => {
const handler = await createDiscussionMain({
max: 5,
category: "general", // Config says general
});

const result = await handler(
{
title: "Test Discussion",
body: "This is a test discussion.",
category: "AUDITS", // Item overrides with uppercase
},
{}
);

expect(result.success).toBe(true);
expect(result.number).toBe(42);

// Verify Audits category was used (from item, not config)
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
expect(createMutationCall).toBeDefined();
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn2"); // Audits category
});

it("should fallback to first category when no match found", async () => {
const handler = await createDiscussionMain({
max: 5,
category: "NonExistentCategory",
});

const result = await handler(
{
title: "Test Discussion",
body: "This is a test discussion.",
},
{}
);

expect(result.success).toBe(true);
expect(result.number).toBe(42);

// Verify fallback to first category (General)
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
expect(createMutationCall).toBeDefined();
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn1"); // General (first)
});
});
45 changes: 21 additions & 24 deletions pkg/workflow/create_discussion.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,8 @@ func (c *Compiler) parseDiscussionsConfig(outputMap map[string]any) *CreateDiscu
return nil // Invalid configuration, return nil to cause validation error
}

// Validate category naming convention (lowercase, preferably plural)
if validateDiscussionCategory(config.Category, discussionLog, c.markdownPath) {
return nil // Invalid configuration, return nil to cause validation error
}
// Normalize and validate category naming convention
Copy link

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

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

The comment says "Normalize and validate category naming convention", but normalizeDiscussionCategory no longer validates/rejects categories (it always returns a string). Update the comment to avoid implying that invalid categories are still rejected at compile time.

Suggested change
// Normalize and validate category naming convention
// Normalize category naming convention for consistent handling and logging

Copilot uses AI. Check for mistakes.
config.Category = normalizeDiscussionCategory(config.Category, discussionLog, c.markdownPath)

// Log configured values
if config.TitlePrefix != "" {
Expand Down Expand Up @@ -201,18 +199,18 @@ func (c *Compiler) buildCreateOutputDiscussionJob(data *WorkflowData, mainJobNam
})
}

// validateDiscussionCategory validates discussion category naming conventions
// Categories should be lowercase and preferably plural
// Returns true if validation fails (invalid), false if valid
func validateDiscussionCategory(category string, log *logger.Logger, markdownPath string) bool {
// normalizeDiscussionCategory normalizes discussion category to lowercase
// and provides warnings about naming conventions
// Returns normalized category (or original if it's a category ID)
func normalizeDiscussionCategory(category string, log *logger.Logger, markdownPath string) string {
// Empty category is allowed (GitHub Discussions will use default)
if category == "" {
return false
return category
}

// GitHub Discussion category IDs start with "DIC_" - these are valid
// GitHub Discussion category IDs start with "DIC_" - don't normalize these
if strings.HasPrefix(category, "DIC_") {
return false
return category
}

// List of known category naming issues and their corrections
Expand All @@ -223,26 +221,25 @@ func validateDiscussionCategory(category string, log *logger.Logger, markdownPat
"Research": "research",
}

// Check if category has uppercase letters
if category != strings.ToLower(category) {
// Check if category has uppercase letters and normalize
normalizedCategory := strings.ToLower(category)
if category != normalizedCategory {
var message string
// Check if we have a known correction
if corrected, exists := categoryCorrections[category]; exists {
message = fmt.Sprintf("Discussion category %q should use lowercase: %q", category, corrected)
message = fmt.Sprintf("Discussion category %q normalized to lowercase: %q", category, corrected)
if log != nil {
log.Printf("Invalid discussion category %q: should use lowercase: %q", category, corrected)
log.Printf("Normalized discussion category %q to lowercase: %q", category, corrected)
}
} else {
message = fmt.Sprintf("Discussion category %q should use lowercase", category)
message = fmt.Sprintf("Discussion category %q normalized to lowercase: %q", category, normalizedCategory)
if log != nil {
log.Printf("Invalid discussion category %q: should use lowercase", category)
log.Printf("Normalized discussion category %q to lowercase: %q", category, normalizedCategory)
}
}

// Print formatted warning to stderr
fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", message))

return true // Validation failed
// Print formatted info message to stderr
fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "info", message))
}

// Warn about singular forms of common categories
Expand All @@ -251,11 +248,11 @@ func validateDiscussionCategory(category string, log *logger.Logger, markdownPat
"report": "reports",
}

if plural, isSingular := singularToPlural[category]; isSingular {
if plural, isSingular := singularToPlural[normalizedCategory]; isSingular {
if log != nil {
log.Printf("⚠ Discussion category %q is singular; consider using plural form %q for consistency", category, plural)
log.Printf("⚠ Discussion category %q is singular; consider using plural form %q for consistency", normalizedCategory, plural)
}
}

return false // Validation passed
return normalizedCategory
}
Loading
Loading