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
11 changes: 11 additions & 0 deletions .github/AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Agent Standards & Documentation

**ATTENTION AGENTS**: Before making changes, you **MUST** read the `README.md` files in the relevant directories (e.g., `.github/workflows/README.md`) to understand the current context and testing procedures.

## Development Workflow for Agents

1. **Read Docs**: Check `.github/workflows/README.md` for testing instructions.
2. **Modify Script**: Edit the Deno script in the workflow directory.
3. **Unit Test**: Run the unit tests using `deno test`.
4. **Integration Test**: Run `bash .github/test/test-workflow-with-act.sh` to verify the workflow configuration.
5. **Commit**: Push changes.
17 changes: 17 additions & 0 deletions .github/test/mock-events/pull_request_opened.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"action": "opened",
"number": 1,
"pull_request": {
"title": "feat(learning-activity): SC-12345-description-of-changes",
"body": "Test PR for acting locally",
"head": {
"ref": "D/conventional-commit-lint-workflow"
},
"base": {
"ref": "main"
}
},
"repository": {
"full_name": "simpleclub/.github"
}
}
50 changes: 50 additions & 0 deletions .github/test/test-workflow-with-act.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/usr/bin/env bash
set -euo pipefail

# Helper to run the PR workflow locally with act
# Requires: act (https://github.com/nektos/act), Docker

WORKFLOW_FILE=".github/workflows/validate-pull-request-title/validate-pull-request-title.yml"
EVENT_FILE=".github/test/mock-events/pull_request_opened.json"
# Use a newer image that supports Node 24 (required by actions/setup-node@v6)
IMAGE="ghcr.io/catthehacker/ubuntu:act-22.04"

echo "Running workflow $WORKFLOW_FILE with event $EVENT_FILE"

# Detect Docker Host if not set
if [[ -z "${DOCKER_HOST-}" ]]; then
if command -v docker >/dev/null 2>&1; then
DETECTED_HOST=$(docker context inspect --format '{{.Endpoints.docker.Host}}' 2>/dev/null || true)
if [[ -n "$DETECTED_HOST" ]]; then
echo "Detected DOCKER_HOST: $DETECTED_HOST"
export DOCKER_HOST="$DETECTED_HOST"
fi
fi
fi

# If the user has a GITHUB_TOKEN set, use it.
# If not, try to get it from gh CLI.
# Otherwise, fall back to a dummy token (which might fail for cloning actions).
if [[ -z "${GITHUB_TOKEN-}" ]]; then
if command -v gh >/dev/null 2>&1; then
TOKEN=$(gh auth token 2>/dev/null || true)
if [[ -n "$TOKEN" ]]; then
echo "Using GITHUB_TOKEN from gh CLI"
export GITHUB_TOKEN="$TOKEN"
fi
fi
fi

if [[ -z "${GITHUB_TOKEN-}" ]]; then
echo "No GITHUB_TOKEN found; using dummy token (warning: this may fail action cloning)"
export GITHUB_TOKEN=dummy_token
fi

# Add architecture flag for Apple Silicon if needed
ARCH_ARGS=""
if [[ "$(uname -m)" == "arm64" ]]; then
ARCH_ARGS="--container-architecture linux/amd64"
fi

# Run act
act pull_request -W "$WORKFLOW_FILE" -e "$EVENT_FILE" -P ubuntu-latest=$IMAGE $ARCH_ARGS --env-file <(echo "GITHUB_TOKEN=$GITHUB_TOKEN")
45 changes: 45 additions & 0 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# GitHub Actions Workflows

This directory contains the shared GitHub Actions workflows used across the organization.

## Available Workflows

- **Validate Pull Request Title** (`validate-pull-request-title/`): Ensures PR titles follow Conventional Commits and include Jira ticket references.
- **Auto Approve** (`auto-approve.yml`): Automatically approves specific types of PRs (configuration details inside the file).

## Testing Workflows

### 1. Unit Tests (Deno)

Scripts used in workflows are written in JavaScript / TypeScript, and run by Deno.

- **Why Deno?**: Deno allows granular security permissions (e.g., explicit network/file access). Since these scripts run in a repository with global impact, this security control is worth the trade-off of deviating from more commonly used tech at simpleclub (Node/Python/Dart).
- **Requirement**: No `package.json`. Use standard Deno imports (e.g., `jsr:@std/...`).
- **Running Tests**:
```bash
# Example for PR title validation
deno test .github/workflows/validate-pull-request-title/validate-pull-request-title.test.js
```

### 2. Integration Tests (act)

We use [act](https://github.com/nektos/act) to simulate GitHub Actions locally using Docker.

**Prerequisites:**

- [Docker Desktop](https://www.docker.com/products/docker-desktop/)
- [act](https://github.com/nektos/act)

**Running Integration Tests:**
A helper script is available to run the workflows with mock events:

```bash
# Run from the root of the repository
bash .github/test/test-workflow-with-act.sh
```

You can also run `act` directly if you prefer:

```bash
act pull_request -W .github/workflows/validate-pull-request-title/validate-pull-request-title.yml --container-architecture linux/amd64 -e .github/test/mock-events/pull_request.json
```
36 changes: 0 additions & 36 deletions .github/workflows/conventional-commit-lint.yaml

This file was deleted.

19 changes: 19 additions & 0 deletions .github/workflows/validate-pull-request-title/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Validate Pull Request Title

This workflow validates that a Pull Request title adheres to the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) specification and includes a Jira ticket reference.

## Validation Rules

1. **Prefix**: Must start with a valid type (e.g., `feat`, `fix`, `chore`) and optional scope.
- Regex: `^(feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert)(\([a-z0-9-]+\))?:`
2. **Ticket**: Must contain a ticket reference (e.g., `SC-1234`, `AI-5678`).
- Regex: `(SC|AI|PT)-[0-9]+`
3. **Format**: The preferred format is `type(scope): TICKET subject`.

## Usage

This workflow is triggered on `pull_request` events (opened, edited, reopened, synchronized).

## Testing

For instructions on how to run unit tests and local integration tests, please refer to the [Workflows README](../README.md).
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* Validates the PR title against Conventional Commits and Jira Ticket requirements.
* @param {string} title
* @returns {{ isValid: boolean, errors: string[], warnings: string[] }}
*/
export function validateTitle(title) {
const errors = [];
const warnings = [];

// Regex Definitions
const REGEX_PREFIX =
/^(feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert)(\([a-z0-9-]+\))?:/;
const REGEX_TICKET = /(SC|AI|PT)-[0-9]+/;
const REGEX_FULL =
/^(feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert)(\([a-z0-9-]+\))?: (SC|AI|PT)-[0-9]+[- ].+$/;

// 1. Fast Path: Check if it matches the perfect format
if (REGEX_FULL.test(title)) {
return { isValid: true, errors, warnings };
}

// 2. Granular Diagnostics
let failed = false;

// Check 1: Prefix
if (!REGEX_PREFIX.test(title)) {
errors.push(
"Title must start with a valid conventional commit prefix (e.g., 'feat:', 'fix(scope):')."
);
errors.push(
"Allowed types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert."
);
failed = true;
}

// Check 2: Ticket Presence & Format
if (REGEX_TICKET.test(title)) {
// Ticket is present but full regex failed -> Warning
warnings.push("Ticket reference found, but the format is incorrect.");
warnings.push("Expected: `type(scope): TICKET subject`");
warnings.push(
"Example: `feat(learning-activity): SC-12345 description of changes`"
);
} else {
// Ticket is missing -> Error
errors.push("Ticket reference (e.g., SC-1234) is missing.");
failed = true;
}

return { isValid: !failed, errors, warnings };
}

// CLI Execution
if (import.meta.main) {
const title = Deno.args[0];
if (!title) {
console.error("Error: No title provided.");
Deno.exit(1);
}

const { isValid, errors, warnings } = validateTitle(title);

// Output Summary Header to stdout
console.log(`## PR Title Validation
Validating title: ${title}
`);

if (isValid) {
console.log("✅ PR title is valid.");
} else {
console.log("❌ PR title validation failed.");
}

// Output Errors
errors.forEach((err) => {
console.error(`::error::${err}`); // Annotation to stderr
console.log(`❌ ${err}`); // Markdown to stdout
});

// Output Warnings
warnings.forEach((warn) => {
console.error(`::warning::${warn}`); // Annotation to stderr
console.log(`⚠️ ${warn}`); // Markdown to stdout
});

console.log(
"See [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) for more details."
);

if (!isValid) {
Deno.exit(1);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { describe, it } from "jsr:@std/testing/bdd";
import { expect } from "jsr:@std/expect";
import { validateTitle } from "./validate-pull-request-title.js";

describe("PR Title Validation", () => {
const validCases = [
{
title: "feat(api): SC-1234 Add new endpoint",
desc: "accepts standard feature with scope and ticket",
},
{
title: "fix: AI-5678 Fix crash on startup",
desc: "accepts fix with ticket and no scope",
},
{
title: "chore: PT-999 Update dependencies",
desc: "accepts chore with PT ticket",
},
{
title: "revert(auth): SC-0000 Revert bad commit",
desc: "accepts revert with scope and ticket",
},
{
title: "feat(api): SC-1234-description-with-hyphens",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I disagree that this should pass!

The changelog should be readable not some identifier.

This is also why I'm in favor of putting the ticket at the end, if you read the changelog the least important information is the ticket.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

we discussed briefly in backend check-in -> we don't care strongly about this -> i'll put it to the end

desc: "accepts hyphen as separator after ticket",
},
];

describe("Valid Titles", () => {
validCases.forEach(({ title, desc }) => {
it(desc, () => {
const result = validateTitle(title);
expect(result.isValid).toBe(true);
expect(result.errors).toEqual([]);
expect(result.warnings).toEqual([]);
});
});
});

const warningCases = [
{
title: "feat: Add feature SC-1234",
desc: "warns when ticket is at the end",
expectedWarningMatch: /format is incorrect/,
},
];

describe("Warnings", () => {
warningCases.forEach(({ title, desc, expectedWarningMatch }) => {
it(desc, () => {
const result = validateTitle(title);
expect(result.isValid).toBe(true);
expect(result.warnings.length).toBeGreaterThan(0);
expect(result.warnings[0]).toMatch(expectedWarningMatch);
});
});
});

const invalidCases = [
{
title: "feat: Add feature",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I also think we should allow no tickets.
There are simply changes where creating a ticket is not useful.
The most important example: Deploying a domain 👀 There we also don't want to have a ticket every time.

Though other changes like updating docs, READMEs, etc. would be too annoying to create an issue just to have an issue.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes fair point -> i'll make it warn, but otherwise succeed

desc: "fails when ticket is missing",
expectedErrorMatch: /Ticket reference.*missing/,
},
{
title: "random: SC-1234 Add feature",
desc: "fails when prefix is invalid",
expectedErrorMatch: /valid conventional commit prefix/,
},
{
title: "feat(api) SC-1234 Missing colon",
desc: "fails when colon is missing",
expectedErrorMatch: /valid conventional commit prefix/,
},
{
title: "Update README",
desc: "fails when completely malformed",
expectedErrorMatch: /valid conventional commit prefix/,
},
{
title: "SC-1234 feat: Add feature",
desc: "fails when ticket is at the start (invalid prefix)",
expectedErrorMatch: /valid conventional commit prefix/,
},
];

describe("Invalid Titles", () => {
invalidCases.forEach(({ title, desc, expectedErrorMatch }) => {
it(desc, () => {
const result = validateTitle(title);
expect(result.isValid).toBe(false);
expect(result.errors.some((e) => expectedErrorMatch.test(e))).toBe(
true
);
});
});
});
});
Loading