diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml deleted file mode 100644 index 3edca739d..000000000 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ /dev/null @@ -1,239 +0,0 @@ ---- -name: Bug Report -description: Report something that is not working correctly -labels: ["bug", "needs-triage"] -body: - - type: markdown - attributes: - value: | - Thanks for taking the time to report a bug. The more detail you provide, the faster we can fix it. - - > **⚠️ Privacy Notice:** Some fields below ask for logs or config files that may contain **personally identifiable information (PII)** such as file paths with your username, API keys, project names, or system details. Before pasting any output, please: - > 1. Review it for sensitive data - > 2. Redact usernames, paths, and API keys (e.g., replace `/Users/yourname/` with `/Users/REDACTED/`) - > 3. Or run your logs through an anonymizer — we recommend **[presidio-anonymizer](https://microsoft.github.io/presidio/)** (open-source, local-only) or **[scrub](https://github.com/dssg/scrub)** before pasting - - - type: checkboxes - id: preflight - attributes: - label: Pre-submission checklist - options: - - label: I have searched existing issues and this bug has not already been reported - required: true - - - type: input - id: version - attributes: - label: MSD Version - description: "Check the installed version: `cat ~/.claude/msd-core/msd-file-manifest.json` (look for `\"version\"`) or `npx @golem15/msd-core --version` (if installed via npm global)" - placeholder: "e.g., 1.18.0" - validations: - required: true - - - type: dropdown - id: runtime - attributes: - label: Runtime - description: Which AI coding tool are you using MSD with? - options: - - Claude Code - - OpenCode - - Codex - - Antigravity - - Cursor - - ZCode (Z.ai) - - Multiple (specify in description) - validations: - required: true - - - type: dropdown - id: os - attributes: - label: Operating System - options: - - macOS - - Windows - - Linux (Ubuntu/Debian) - - Linux (Fedora/RHEL) - - Linux (Arch) - - Linux (Other) - - WSL - validations: - required: true - - - type: input - id: node_version - attributes: - label: Node.js Version - description: "Run: `node --version`" - placeholder: "e.g., v20.11.0" - validations: - required: true - - - type: input - id: shell - attributes: - label: Shell - description: "Run: `echo $SHELL` (macOS/Linux) or `echo %COMSPEC%` (Windows)" - placeholder: "e.g., /bin/zsh, /bin/bash, PowerShell 7" - validations: - required: false - - - type: dropdown - id: install_method - attributes: - label: Installation Method - options: - - npx @golem15/msd-core@latest (fresh run) - - npm install -g @golem15/msd-core - - Updated from a previous version - validations: - required: true - - - type: textarea - id: description - attributes: - label: What happened? - description: Describe what went wrong. Be specific about which MSD command you were running. - placeholder: | - When I ran `/msd-plan`, the system... - validations: - required: true - - - type: textarea - id: expected - attributes: - label: What did you expect? - description: Describe what you expected to happen instead. - validations: - required: true - - - type: textarea - id: reproduce - attributes: - label: Steps to reproduce - description: | - Exact steps to reproduce the issue. Include the MSD command used. - placeholder: | - 1. Install MSD with `npx @golem15/msd-core@latest` - 2. Select runtime: Claude Code - 3. Run `/msd-init` with a new project - 4. Run `/msd-plan` - 5. Error appears at step... - validations: - required: true - - - type: textarea - id: logs - attributes: - label: Error output / logs - description: | - Paste any error messages from the terminal. This will be rendered as code. - - **⚠️ PII Warning:** Terminal output often contains your system username in file paths (e.g., `/Users/yourname/.claude/...`). Please redact before pasting. - render: shell - validations: - required: false - - - type: textarea - id: config - attributes: - label: MSD Configuration - description: | - If the bug is related to planning, phases, or workflow behavior, paste your `.planning/config.json`. - - **How to retrieve:** `cat .planning/config.json` - - **⚠️ PII Warning:** This file may contain project-specific names. Redact if sensitive. - render: json - validations: - required: false - - - type: textarea - id: state - attributes: - label: MSD State (if relevant) - description: | - If the bug involves incorrect state tracking or phase progression, include your `.planning/STATE.md`. - - **How to retrieve:** `cat .planning/STATE.md` - - **⚠️ PII Warning:** This file contains project names, phase descriptions, and timestamps. Redact any project names or details you don't want public. - render: markdown - validations: - required: false - - - type: textarea - id: settings_json - attributes: - label: Runtime settings.json (if relevant) - description: | - If the bug involves hooks, statusline, or runtime integration, include your runtime's settings.json. - - **How to retrieve:** - - Claude Code: `cat ~/.claude/settings.json` - - OpenCode: `cat ~/.config/opencode/opencode.json` or `opencode.jsonc` - - **⚠️ PII Warning:** This file may contain API keys, tokens, or custom paths. **Remove all API keys and tokens before pasting.** We recommend running through [presidio-anonymizer](https://microsoft.github.io/presidio/) or manually redacting any line containing "key", "token", or "secret". - render: json - validations: - required: false - - - type: dropdown - id: frequency - attributes: - label: How often does this happen? - options: - - Every time (100% reproducible) - - Most of the time - - Sometimes / intermittent - - Only happened once - validations: - required: true - - - type: dropdown - id: severity - attributes: - label: Impact - description: How much does this affect your workflow? - options: - - Blocker — Cannot use MSD at all - - Major — Core feature is broken, no workaround - - Moderate — Feature is broken but I have a workaround - - Minor — Cosmetic or edge case - validations: - required: true - - - type: textarea - id: workaround - attributes: - label: Workaround (if any) - description: Have you found any way to work around this issue? - validations: - required: false - - - type: textarea - id: additional - attributes: - label: Additional context - description: | - Anything else — screenshots, screen recordings, related issues, or links. - - **Useful diagnostics to include (if applicable):** - - `cat ~/.claude/msd-core/msd-file-manifest.json` — confirms installed version (the `version` field tracks what `/msd-update` installs; `npm list -g` does NOT reflect runtime-home installs) - - `ls -la ~/.claude/msd-core/` — confirms installation files (Claude Code) - - `cat ~/.claude/msd-core/msd-file-manifest.json` — file manifest for debugging install issues - - `ls -la .planning/` — confirms planning directory state - - **⚠️ PII Warning:** File listings and manifests contain your home directory path. Replace your username with `REDACTED`. - validations: - required: false - - - type: checkboxes - id: pii_check - attributes: - label: Privacy Checklist - description: Please confirm you've reviewed your submission for sensitive data. - options: - - label: I have reviewed all pasted output for PII (usernames, paths, API keys) and redacted where necessary - required: true diff --git a/.github/ISSUE_TEMPLATE/chore.yml b/.github/ISSUE_TEMPLATE/chore.yml deleted file mode 100644 index 92125a339..000000000 --- a/.github/ISSUE_TEMPLATE/chore.yml +++ /dev/null @@ -1,118 +0,0 @@ ---- -name: Chore / Maintenance -description: Internal improvements — refactoring, test quality, CI/CD, dependency updates, tech debt. -labels: ["type: chore", "needs-triage"] -body: - - type: markdown - attributes: - value: | - ## Internal maintenance work - - Use this template for work that improves the **project's health** without changing user-facing behavior. Examples: - - Test suite refactoring or standardization - - CI/CD pipeline improvements - - Dependency updates - - Code quality or linting changes - - Build system or tooling updates - - Documentation infrastructure (not content — use Docs Issue for content) - - Tech debt paydown - - If this changes how MSD **works** for users, use [Enhancement](./enhancement.yml) or [Feature Request](./feature_request.yml) instead. - - - type: checkboxes - id: preflight - attributes: - label: Pre-submission checklist - options: - - label: This does not change user-facing behavior (commands, output, file formats, config) - required: true - - label: I have searched existing issues — this has not already been filed - required: true - - - type: input - id: chore_title - attributes: - label: What is the maintenance task? - description: A short, concrete description of what needs to happen. - placeholder: "e.g., Migrate test suite to node:assert/strict, Update c8 to v12, Add Windows CI matrix entry" - validations: - required: true - - - type: dropdown - id: chore_type - attributes: - label: Type of maintenance - options: - - Test quality (coverage, patterns, runner) - - CI/CD pipeline - - Dependency update - - Refactoring / code quality - - Build system / tooling - - Documentation infrastructure - - Tech debt - - Other - validations: - required: true - - - type: textarea - id: current_state - attributes: - label: Current state - description: | - Describe the current situation. What is the problem or debt? Include numbers where possible (test count, coverage %, build time, dependency age). - placeholder: | - 73 of 89 test files use `require('node:assert')` instead of `require('node:assert/strict')`. - CONTRIBUTING.md requires strict mode. Non-strict assert allows type coercion in `deepEqual`, - masking potential bugs. - validations: - required: true - - - type: textarea - id: proposed_work - attributes: - label: Proposed work - description: | - What changes will be made? List files, patterns, or systems affected. - placeholder: | - - Replace `require('node:assert')` with `require('node:assert/strict')` across all 73 test files - - Replace `try/finally` cleanup with `t.after()` hooks per CONTRIBUTING.md standards - - Verify all 2148 tests still pass - validations: - required: true - - - type: textarea - id: acceptance_criteria - attributes: - label: Done when - description: | - List the specific conditions that mean this work is complete. These should be verifiable. - placeholder: | - - [ ] All test files use `node:assert/strict` - - [ ] Zero `try/finally` cleanup blocks in test lifecycle code - - [ ] CI green on all matrix entries (Node 24, Ubuntu/macOS/Windows) - - [ ] No change to user-facing behavior - validations: - required: true - - - type: dropdown - id: area - attributes: - label: Area affected - options: - - Test suite - - CI/CD - - Build system - - Core library code - - Installer - - Documentation tooling - - Multiple areas - validations: - required: true - - - type: textarea - id: additional_context - attributes: - label: Additional context - description: Related issues, prior art, or anything else that helps scope this work. - validations: - required: false diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml deleted file mode 100644 index 0219418dd..000000000 --- a/.github/ISSUE_TEMPLATE/config.yml +++ /dev/null @@ -1,11 +0,0 @@ -blank_issues_enabled: false -contact_links: - - name: "⚠️ v1.31.0 not on npm yet (known issue — workaround inside)" - url: https://github.com/open-gsd/gsd-core/discussions - about: v1.31.0 was not published to npm due to a hardware failure. Read the pinned announcement for the workaround before opening an issue. - - name: Discord Community - url: https://discord.gg/mYgfVNfA2r - about: Ask questions and get help from the community - - name: Discussions - url: https://github.com/open-gsd/gsd-core/discussions - about: Share ideas or ask general questions diff --git a/.github/ISSUE_TEMPLATE/docs_issue.yml b/.github/ISSUE_TEMPLATE/docs_issue.yml deleted file mode 100644 index ce1e324a4..000000000 --- a/.github/ISSUE_TEMPLATE/docs_issue.yml +++ /dev/null @@ -1,55 +0,0 @@ ---- -name: Documentation Issue -description: Report incorrect, missing, or unclear documentation -labels: ["documentation"] -body: - - type: markdown - attributes: - value: | - Help us improve the docs. Point us to what's wrong or missing. - - - type: checkboxes - id: preflight - attributes: - label: Pre-submission checklist - options: - - label: I have searched existing issues and this documentation problem has not already been reported - required: true - - - type: dropdown - id: type - attributes: - label: Issue type - options: - - Incorrect information - - Missing documentation - - Unclear or confusing - - Outdated (no longer matches behavior) - - Typo or formatting - validations: - required: true - - - type: input - id: location - attributes: - label: Where is the issue? - description: File path, URL, or section name - placeholder: "e.g., docs/USER-GUIDE.md, README.md#getting-started" - validations: - required: true - - - type: textarea - id: description - attributes: - label: What's wrong? - description: Describe the documentation issue. - validations: - required: true - - - type: textarea - id: suggestion - attributes: - label: Suggested fix - description: If you know what the correct information should be, include it here. - validations: - required: false diff --git a/.github/ISSUE_TEMPLATE/enhancement.yml b/.github/ISSUE_TEMPLATE/enhancement.yml deleted file mode 100644 index c51642201..000000000 --- a/.github/ISSUE_TEMPLATE/enhancement.yml +++ /dev/null @@ -1,160 +0,0 @@ ---- -name: Enhancement Proposal -description: Propose an improvement to an existing feature. Read the full instructions before opening this issue. -labels: ["enhancement", "needs-review"] -body: - - type: markdown - attributes: - value: | - ## ⚠️ Read this before you fill anything out - - An enhancement improves something that already exists — better output, expanded edge-case handling, improved performance, cleaner UX. It does **not** add new commands, new workflows, or new concepts. If you are proposing something new, use the [Feature Request](./feature_request.yml) template instead. - - **Before opening this issue:** - - Confirm the thing you want to improve actually exists and works today. - - Read [CONTRIBUTING.md](../../CONTRIBUTING.md#-enhancement) — understand what `approved-enhancement` means and why you must wait for it before writing any code. - - **What happens after you submit:** - A maintainer will review this proposal. If it is incomplete or out of scope, it will be **closed**. If approved, it will be labeled `approved-enhancement` and you may begin coding. - - **Do not open a PR until this issue is labeled `approved-enhancement`.** - - - type: checkboxes - id: preflight - attributes: - label: Pre-submission checklist - description: You must check every box. Unchecked boxes are an immediate close. - options: - - label: I have confirmed this improves existing behavior — it does not add a new command, workflow, or concept - required: true - - label: I have searched existing issues and this enhancement has not already been proposed - required: true - - label: I have read CONTRIBUTING.md and understand I must wait for `approved-enhancement` before writing any code - required: true - - label: I can clearly describe the concrete benefit — not just "it would be nicer" - required: true - - - type: input - id: what_is_being_improved - attributes: - label: What existing feature or behavior does this improve? - description: Name the specific command, workflow, output, or behavior you are enhancing. - placeholder: "e.g., `/msd-plan` output, phase status display in statusline, context summary format" - validations: - required: true - - - type: textarea - id: current_behavior - attributes: - label: Current behavior - description: | - Describe exactly how the thing works today. Be specific. Include example output or commands if helpful. - placeholder: | - Currently, `/msd-status` shows: - ``` - Phase 2/5 — In Progress - ``` - It does not show the phase name, making it hard to know what phase you are actually in without - opening STATE.md. - validations: - required: true - - - type: textarea - id: proposed_behavior - attributes: - label: Proposed behavior - description: | - Describe exactly how it should work after the enhancement. Be specific. Include example output or commands. - placeholder: | - After the enhancement, `/msd-status` would show: - ``` - Phase 2/5 — In Progress — "Implement core auth module" - ``` - The phase name is pulled from STATE.md and appended to the existing output. - validations: - required: true - - - type: textarea - id: reason_and_benefit - attributes: - label: Reason and benefit - description: | - Answer both of these clearly: - - 1. **Why is the current behavior a problem?** (Not just inconvenient — what goes wrong, what is harder than it should be, or what is confusing?) - 2. **What is the concrete benefit of the proposed behavior?** (What becomes easier, faster, less error-prone, or clearer?) - - Vague answers like "it would be better" or "it's more user-friendly" are not sufficient. - placeholder: | - **Why the current behavior is a problem:** - When working in a long session, the AI agent frequently loses track of which phase is active - and must re-read STATE.md. The numeric-only status gives no semantic context. - - **Concrete benefit:** - Showing the phase name means the agent can confirm the active phase from the status output - alone, without an extra file read. This reduces context consumption in long sessions. - validations: - required: true - - - type: textarea - id: scope - attributes: - label: Scope of changes - description: | - List the files and systems this enhancement would touch. Be complete. - An enhancement should have a narrow, well-defined scope. If your list is long, this might be a feature, not an enhancement. - placeholder: | - Files modified: - - `msd-core/commands/msd/status.md` — update output format description - - `msd-core/bin/lib/state.cjs` — expose phase name in status() return value - - `tests/status.test.cjs` — update snapshot and add test for phase name in output - - `CHANGELOG.md` — user-facing change entry - - No new files created. No new dependencies. - validations: - required: true - - - type: textarea - id: breaking_changes - attributes: - label: Breaking changes - description: | - Does this change existing command output, file formats, or behavior that users or AI agents might depend on? - If yes, describe exactly what changes and how it stays backward compatible (or why it cannot). - Write "None" only if you are certain. - validations: - required: true - - - type: textarea - id: alternatives - attributes: - label: Alternatives considered - description: | - What other ways could this be improved? Why is your proposed approach the right one? - If you haven't considered alternatives, take a moment before submitting. - validations: - required: true - - - type: dropdown - id: area - attributes: - label: Area affected - options: - - Core workflow (init, plan, build, verify) - - Planning system (phases, roadmap, state) - - Context management (context engineering, summaries) - - Runtime integration (hooks, statusline, settings) - - Installation / setup - - Output / formatting - - Documentation - - Other - validations: - required: true - - - type: textarea - id: additional_context - attributes: - label: Additional context - description: Screenshots, related issues, or anything else that helps explain the proposal. - validations: - required: false diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml deleted file mode 100644 index ca6e195fc..000000000 --- a/.github/ISSUE_TEMPLATE/feature_request.yml +++ /dev/null @@ -1,247 +0,0 @@ ---- -name: Feature Request -description: Propose a new feature. Read the full instructions before opening this issue. -labels: ["feature-request", "needs-review"] -body: - - type: markdown - attributes: - value: | - ## ⚠️ Read this before you fill anything out - - A feature adds something new to MSD — a new command, workflow, concept, or integration. Features have the **highest bar** for acceptance because every feature adds permanent maintenance burden to a project built for solo developers. - - **Before opening this issue:** - - Check [Discussions](https://github.com/open-gsd/gsd-core/discussions) — has this been proposed and declined before? - - Read [CONTRIBUTING.md](../../CONTRIBUTING.md#-feature) — understand what "approved-feature" means and why you must wait for it before writing code. - - Ask yourself: *does this solve a real problem for a solo developer working with an AI coding tool, or is it a feature I personally want?* - - **What happens after you submit:** - A maintainer will review this spec. If it is incomplete, it will be **closed**, not revised. If it conflicts with MSD's design philosophy, it will be declined. If it is approved, it will be labeled `approved-feature` and you may begin coding. - - **Do not open a PR until this issue is labeled `approved-feature`.** - - - type: checkboxes - id: preflight - attributes: - label: Pre-submission checklist - description: You must check every box. Unchecked boxes are an immediate close. - options: - - label: I have searched existing issues and discussions — this has not been proposed and declined before - required: true - - label: I have read CONTRIBUTING.md and understand that I must wait for `approved-feature` before writing any code - required: true - - label: I have read the existing MSD commands and workflows and confirmed this feature does not duplicate existing behavior - required: true - - label: This feature solves a problem for solo developers using AI coding tools, not a personal preference or workflow I happen to like - required: true - - - type: input - id: feature_name - attributes: - label: Feature name - description: A short, concrete name for this feature (not a sales pitch — just what it is). - placeholder: "e.g., Phase rollback command, Auto-archive completed phases, Cross-project state sync" - validations: - required: true - - - type: dropdown - id: feature_type - attributes: - label: Type of addition - description: What kind of thing is this feature adding? - options: - - New command (slash command or CLI flag) - - New workflow (multi-step process) - - New runtime integration - - New planning concept (phase type, state, etc.) - - New installation/setup behavior - - New output or reporting format - - Other (describe in spec) - validations: - required: true - - - type: textarea - id: problem_statement - attributes: - label: The solo developer problem - description: | - Describe the concrete problem this solves for a solo developer using an AI coding tool. Be specific. - - Good: "When a phase fails mid-way, there is no way to roll back state without manually editing STATE.md. This causes the AI agent to continue from a corrupted state, producing wrong plans." - - Bad: "It would be nice to have a rollback feature." / "Other tools have this." / "I need this for my workflow." - placeholder: | - When [specific situation], the developer cannot [specific thing], which causes [specific negative outcome]. - validations: - required: true - - - type: textarea - id: what_is_added - attributes: - label: What this feature adds - description: | - Describe exactly what is being added. Be specific about commands, output, behavior, and user interaction. - Include example commands or example output where possible. - placeholder: | - A new command `/msd-rollback` that: - 1. Reads the current phase from STATE.md - 2. Reverts STATE.md to the previous phase's snapshot - 3. Outputs a confirmation with the rolled-back state - - Example usage: - ``` - /msd-rollback - > Rolled back from Phase 3 (failed) to Phase 2 (completed) - ``` - validations: - required: true - - - type: textarea - id: full_scope - attributes: - label: Full scope of changes - description: | - List every file, system, and area of the codebase this feature would touch. Be exhaustive. - If you cannot fill this out, you do not understand the codebase well enough to propose this feature yet. - placeholder: | - Files that would be created: - - `msd-core/commands/msd/rollback.md` — new slash command definition - - Files that would be modified: - - `msd-core/bin/lib/state.cjs` — add rollback() function - - `msd-core/bin/lib/phases.cjs` — expose phase snapshot API - - `tests/rollback.test.cjs` — new test file - - `docs/COMMANDS.md` — document new command - - `CHANGELOG.md` — entry for this feature - - Systems affected: - - STATE.md schema (must remain backward compatible) - - Phase lifecycle state machine - validations: - required: true - - - type: textarea - id: user_stories - attributes: - label: User stories - description: Write at least two user stories in the format "As a [user], I want [thing] so that [outcome]." - placeholder: | - 1. As a solo developer, I want to roll back a failed phase so that I can re-attempt it without corrupting my project state. - 2. As a solo developer, I want rollback to be undoable so that I don't accidentally lose completed work. - validations: - required: true - - - type: textarea - id: acceptance_criteria - attributes: - label: Acceptance criteria - description: | - List the specific, testable conditions that must be true for this feature to be considered complete. - These become the basis for reviewer sign-off. Vague criteria ("it works") are not acceptable. - placeholder: | - - [ ] `/msd-rollback` reverts STATE.md to the previous phase when current phase status is `failed` - - [ ] `/msd-rollback` exits with an error if there is no previous phase to roll back to - - [ ] `/msd-rollback` outputs the before/after phase names in its confirmation message - - [ ] Rollback is logged in the phase history so the AI agent can see it happened - - [ ] All existing tests still pass - - [ ] New tests cover the happy path, no-previous-phase case, and STATE.md corruption case - validations: - required: true - - - type: dropdown - id: scope - attributes: - label: Which area does this primarily affect? - options: - - Core workflow (init, plan, build, verify) - - Planning system (phases, roadmap, state) - - Context management (context engineering, summaries) - - Runtime integration (hooks, statusline, settings) - - Installation / setup - - Documentation only - - Multiple areas (describe in scope section above) - validations: - required: true - - - type: checkboxes - id: runtimes - attributes: - label: Applicable runtimes - description: Which runtimes must this work with? Check all that apply. - options: - - label: Claude Code - - label: OpenCode - - label: Codex - - label: Antigravity - - label: Cursor - - label: All runtimes - - - type: textarea - id: breaking_changes - attributes: - label: Breaking changes assessment - description: | - Does this feature change existing behavior, command output, file formats, or APIs? - If yes, describe exactly what breaks and how existing users would migrate. - Write "None" only if you are certain. - placeholder: | - None — this adds a new command and does not modify any existing command behavior or file schemas. - - OR: - - STATE.md will gain a new `phase_history` array field. Existing STATE.md files without this field - will be treated as having an empty history (backward compatible). The rollback command will - decline gracefully if history is empty. - validations: - required: true - - - type: textarea - id: maintenance_burden - attributes: - label: Maintenance burden - description: | - Every feature is code that must be maintained forever. Describe the ongoing cost: - - How does this interact with future changes to phases, state, or commands? - - Does this add external dependencies? - - Does this require documentation updates across multiple files? - - Will this create edge cases or interactions with other features? - placeholder: | - - No new dependencies - - The rollback function must be updated if the STATE.md schema ever changes - - Will need to be tested on each new Node.js LTS release - - The command definition must be kept in sync with any future command format changes - validations: - required: true - - - type: textarea - id: alternatives - attributes: - label: Alternatives considered - description: | - What other approaches did you consider? Why did you reject them? - If the answer is "I didn't consider any alternatives", this issue will be closed. - placeholder: | - 1. Manual STATE.md editing — rejected because it requires the developer to understand the schema - and is error-prone. The AI agent cannot reliably guide this. - 2. A `/msd-reset` command that wipes all state — rejected because it is too destructive and - loses all completed phase history. - validations: - required: true - - - type: textarea - id: prior_art - attributes: - label: Prior art and references - description: | - Does any other tool, project, or MSD discussion address this? Link to anything relevant. - If you are aware of a prior declined proposal for this feature, explain why this proposal is different. - validations: - required: false - - - type: textarea - id: additional_context - attributes: - label: Additional context - description: Anything else — screenshots, recordings, related issues, or links. - validations: - required: false diff --git a/.github/PULL_REQUEST_TEMPLATE/enhancement.md b/.github/PULL_REQUEST_TEMPLATE/enhancement.md deleted file mode 100644 index 1d8cbde85..000000000 --- a/.github/PULL_REQUEST_TEMPLATE/enhancement.md +++ /dev/null @@ -1,101 +0,0 @@ -## Enhancement PR - -> **Using the wrong template?** -> — Bug fix: use [fix.md](?template=fix.md) -> — New feature: use [feature.md](?template=feature.md) - ---- - -## Linked Issue - -> **Required.** This PR will be auto-closed if no valid issue link is found. -> The linked issue **must** have the `approved-enhancement` label. If it does not, this PR will be closed without review. - -Closes # - -> ⛔ **No `approved-enhancement` label on the issue = immediate close.** -> Do not open this PR if a maintainer has not yet approved the enhancement proposal. - ---- - -## What this enhancement improves - - - -## Before / After - -**Before:** - - -**After:** - - -## How it was implemented - - - -## Testing - -### How I verified the enhancement works - - - -### Platforms tested - -- [ ] macOS -- [ ] Windows (including backslash path handling) -- [ ] Linux -- [ ] N/A (not platform-specific) - -### Runtimes tested - -- [ ] Claude Code -- [ ] Antigravity -- [ ] OpenCode -- [ ] Other: ___ -- [ ] N/A (not runtime-specific) - ---- - -## Scope confirmation - - - -- [ ] The implementation matches the scope approved in the linked issue — no additions or removals -- [ ] If scope changed during implementation, I updated the issue and got re-approval before continuing - ---- - -## Documentation - -> CI enforces this — `lint:docs` fails any PR with an `Added` / `Changed` / `Deprecated` / `Removed` -> changeset fragment that does not also touch at least one file under `docs/`. -> See [CONTRIBUTING.md → Documentation Updates](../../CONTRIBUTING.md#documentation-updates-update-the-relevant-docs). - -- [ ] Updated the relevant file(s) under `docs/` to reflect this change - - Behavior or output change → `docs/USER-GUIDE.md` and/or `docs/COMMANDS.md` - - Configuration / schema change → `docs/CONFIGURATION.md` - - Architectural change → `docs/ARCHITECTURE.md` and/or `docs/adr/` - - Agent or skill change → `docs/AGENTS.md` -- [ ] All `docs/` content added in this PR is written in English -- [ ] If genuinely no user-facing docs impact (infrastructure / internal refactor / test-only), - apply the `no-docs` label **or** add `` inside each - triggering changeset fragment and leave a comment explaining why. - -## Checklist - -- [ ] Issue linked above with `Closes #NNN` — **PR will be auto-closed if missing** -- [ ] Linked issue has the `approved-enhancement` label — **PR will be closed if missing** -- [ ] Changes are scoped to the approved enhancement — nothing extra included -- [ ] All existing tests pass (`npm test`) -- [ ] New or updated tests cover the enhanced behavior -- [ ] `.changeset/` fragment added (`npm run changeset -- --type Changed --pr --body "..."`) — or `no-changelog` label applied if not user-facing -- [ ] No unnecessary dependencies added - -## Breaking changes - - - -None diff --git a/.github/PULL_REQUEST_TEMPLATE/feature.md b/.github/PULL_REQUEST_TEMPLATE/feature.md deleted file mode 100644 index 9f744c788..000000000 --- a/.github/PULL_REQUEST_TEMPLATE/feature.md +++ /dev/null @@ -1,131 +0,0 @@ -## Feature PR - -> **Using the wrong template?** -> — Bug fix: use [fix.md](?template=fix.md) -> — Enhancement to existing behavior: use [enhancement.md](?template=enhancement.md) - ---- - -## Linked Issue - -> **Required.** This PR will be auto-closed if no valid issue link is found. -> The linked issue **must** have the `approved-feature` label. If it does not, this PR will be closed without review — no exceptions. - -Closes # - -> ⛔ **No `approved-feature` label on the issue = immediate close.** -> Do not open this PR if a maintainer has not yet approved the feature spec. -> Do not open this PR if you wrote code before the issue was approved. - ---- - -## Feature summary - - - -## What changed - -### New files - - - -| File | Purpose | -|------|---------| -| | | - -### Modified files - - - -| File | What changed | -|------|-------------| -| | | - -## Implementation notes - - - -## Spec compliance - - - -- [ ] -- [ ] -- [ ] - -## Testing - -### Test coverage - - - -### Platforms tested - -- [ ] macOS -- [ ] Windows (including backslash path handling) -- [ ] Linux - -### Runtimes tested - -- [ ] Claude Code -- [ ] Antigravity -- [ ] OpenCode -- [ ] Codex -- [ ] Other: ___ -- [ ] N/A — specify which runtimes are supported and why others are excluded - ---- - -## Scope confirmation - -- [ ] The implementation matches the scope approved in the linked issue exactly -- [ ] No additional features, commands, or behaviors were added beyond what was approved -- [ ] If scope changed during implementation, I updated the issue spec and received re-approval - ---- - -## Documentation - -> CI enforces this — `lint:docs` fails any PR with an `Added` / `Changed` / `Deprecated` / `Removed` -> changeset fragment that does not also touch at least one file under `docs/`. Features almost -> always trigger `Added`. See -> [CONTRIBUTING.md → Documentation Updates](../../CONTRIBUTING.md#documentation-updates-update-the-relevant-docs). - -- [ ] Updated the relevant file(s) under `docs/` to reflect this feature - - New command or flag → `docs/COMMANDS.md` and `docs/FEATURES.md` - - New workflow or behavior → `docs/USER-GUIDE.md` - - Configuration / schema change → `docs/CONFIGURATION.md` - - Architectural change → `docs/ARCHITECTURE.md` and/or `docs/adr/` - - Agent or skill change → `docs/AGENTS.md` -- [ ] All `docs/` content added in this PR is written in English - (translated READMEs `README.pt-BR.md` / `README.zh-CN.md` / `README.ja-JP.md` / `README.ko-KR.md` - are community-maintained and do not need to be updated in this PR) -- [ ] If genuinely no user-facing docs impact (rare for features — explain in PR), apply the - `no-docs` label **or** add `` inside each triggering - changeset fragment. - -## Checklist - -- [ ] Issue linked above with `Closes #NNN` — **PR will be auto-closed if missing** -- [ ] Linked issue has the `approved-feature` label — **PR will be closed if missing** -- [ ] All acceptance criteria from the issue are met (listed above) -- [ ] Implementation scope matches the approved spec exactly -- [ ] All existing tests pass (`npm test`) -- [ ] New tests cover the happy path, error cases, and edge cases -- [ ] `.changeset/` fragment added with a user-facing description of the feature (`npm run changeset -- --type Added --pr --body "..."`) -- [ ] No unnecessary external dependencies added -- [ ] Works on Windows (backslash paths handled) - -## Breaking changes - - - -None - -## Screenshots / recordings - - diff --git a/.github/PULL_REQUEST_TEMPLATE/fix.md b/.github/PULL_REQUEST_TEMPLATE/fix.md deleted file mode 100644 index fe5154782..000000000 --- a/.github/PULL_REQUEST_TEMPLATE/fix.md +++ /dev/null @@ -1,74 +0,0 @@ -## Fix PR - -> **Using the wrong template?** -> — Enhancement: use [enhancement.md](?template=enhancement.md) -> — Feature: use [feature.md](?template=feature.md) - ---- - -## Linked Issue - -> **Required.** This PR will be auto-closed if no valid issue link is found. - -Fixes # - -> The linked issue must have the `confirmed-bug` label. If it doesn't, ask a maintainer to confirm the bug before continuing. - ---- - -## What was broken - - - -## What this fix does - - - -## Root cause - - - -## Testing - -### How I verified the fix - - - -### Regression test added? - -- [ ] Yes — added a test that would have caught this bug -- [ ] No — explain why: - -### Platforms tested - -- [ ] macOS -- [ ] Windows (including backslash path handling) -- [ ] Linux -- [ ] N/A (not platform-specific) - -### Runtimes tested - -- [ ] Claude Code -- [ ] Antigravity -- [ ] OpenCode -- [ ] Other: ___ -- [ ] N/A (not runtime-specific) - ---- - -## Checklist - -- [ ] Issue linked above with `Fixes #NNN` — **PR will be auto-closed if missing** -- [ ] Linked issue has the `confirmed-bug` label -- [ ] Fix is scoped to the reported bug — no unrelated changes included -- [ ] Regression test added (or explained why not) -- [ ] All existing tests pass (`npm test`) -- [ ] `.changeset/` fragment added if this is a user-facing fix (`npm run changeset -- --type Fixed --pr --body "..."`) — or `no-changelog` label applied -- [ ] No unnecessary dependencies added - -## Breaking changes - - - -None diff --git a/.github/PULL_REQUEST_TEMPLATE/registry-entry.md b/.github/PULL_REQUEST_TEMPLATE/registry-entry.md deleted file mode 100644 index fd2734fc9..000000000 --- a/.github/PULL_REQUEST_TEMPLATE/registry-entry.md +++ /dev/null @@ -1,107 +0,0 @@ -## Registry Entry PR - -> **Using the wrong template?** -> — Bug fix: use [fix.md](?template=fix.md) -> — New feature (not a registry listing): use [feature.md](?template=feature.md) -> — Enhancement to existing behavior: use [enhancement.md](?template=enhancement.md) - -Full schema and process: [docs/registries/README.md](../../docs/registries/README.md). - ---- - -## Registry type - - - -- [ ] Capability Registry entry — adds/updates one object in `docs/registries/capabilities.json` -- [ ] EoS Registry entry — adds/updates one object in `docs/registries/eos.json` -- [ ] Reviewer Lane Registry entry — adds/updates one object in `docs/registries/reviewers.json` - -## The entry - - - -```json -{ - "id": "", - "name": "", - "type": "", - "repo": "", - "description": "", - "author": "", - "license": "", - "enginesMsd": "", - "install": "", - "uninstall": "", - "interactions": {}, - "discussion": "" -} -``` - ---- - -## Required-field checklist - -- [ ] `id`, `name`, `type`, `repo`, `description`, `author`, `license`, `enginesMsd`, `install`, `uninstall`, `interactions`, `discussion` are all present and non-empty -- [ ] **(Capability entries only)** `interactions.loopExtensionPoints` is a non-empty subset of the 12 Loop Extension Points, `interactions.hookKinds` ⊆ `{step, contribution, gate}`, and `interactions.configKeys` / `requires` / `runtimeCompat` / `produces` / `consumes` are present (empty arrays are fine where nothing applies) -- [ ] **(EoS entries only)** `protocolVersion` is an integer ≥ 1, `interactions.interfacePoints` is a non-empty subset of the six interface points, `interactions.profile` is one of `programmatic-cli` / `declarative-cli` / `ide`, and `interactions.axes` has exactly the eight required axis keys plus, optionally, `effortSurface` (`argv` / `none`) -- [ ] **(Reviewer entries only)** `interactions.slug` matches the lane slug grammar `^[a-z0-9][a-z0-9_-]*$`, `interactions.flags` is a non-empty array matching `^--[a-z0-9][a-z0-9-]*$`, `interactions.transport` is `spawn` or `openai-http`, `interactions.evidenceClass` is `source-grounded` or `diff-only`, `interactions.reviewsSection` is a non-empty string (max 200 characters), and `interactions.requiresBinaries` / `configKeys` / `runtimeCompat` are present (empty arrays are fine where nothing applies) - -## Ownership & non-endorsement - -- [ ] `repo` links to a repository **I own or am the primary maintainer of** — not a fork, mirror, or someone else's project -- [ ] I understand that inclusion in this registry means only that a maintainer merged this PR — it is **not** an endorsement, and MSD has not reviewed, tested, audited, or verified my solution or its claimed MSD interactions -- [ ] I understand this entry is removed only for illegal content, malware, spam, or a dead/non-functional link — never for quality — and a maintainer may remove it on that narrow basis without further notice - -## One entry, one PR - -- [ ] This PR adds or updates exactly **one** entry, in exactly one of `capabilities.json` / `eos.json` / `reviewers.json` -- [ ] I have not bundled any other registry entry, code change, or unrelated docs change into this PR - -## Generated file in sync - -- [ ] I ran `npm run gen:registry` after editing the JSON source, and this PR includes the regenerated `docs/registries/capability-registry.md`, `docs/registries/eos-registry.md`, or `docs/registries/reviewer-registry.md` -- [ ] I did **not** hand-edit the generated `.md` file directly — all edits were made to the JSON source - -## Documentation - -> CI enforces `lint:docs` for any changeset fragment typed `Added` / `Changed` / `Deprecated` / `Removed` — it must also touch a file under `docs/`. The JSON source and its regenerated markdown, both under `docs/registries/`, satisfy this. - -- [ ] This PR includes both the JSON source file and the regenerated markdown file under `docs/registries/` - -## Checklist - -- [ ] `npm run validate:registry` passes locally against my entry -- [ ] `discussion` links to a GitHub Discussion in the `EoS Registry` category — which, despite its name, carries threads for all three registries (or notes that one will be created on merge, per [docs/registries/README.md](../../docs/registries/README.md)) -- [ ] `.changeset/` fragment added with an `Added` type describing the new listing - ---- - -## Example filled entry - - - -```json -{ - "id": "linear-issue-sync", - "name": "Linear Issue Sync", - "type": "capability", - "repo": "some-org/msd-cap-linear-sync", - "description": "Mirrors ROADMAP.md items to Linear issues as a ship:post contribution.", - "author": "Some Org ", - "license": "MIT", - "enginesMsd": ">=1.6.0", - "install": "msd capability install https://github.com/some-org/msd-cap-linear-sync.git#v1.0.0", - "uninstall": "msd capability remove linear-issue-sync", - "interactions": { - "loopExtensionPoints": ["ship:post"], - "hookKinds": ["contribution"], - "configKeys": ["linear-issue-sync.enabled"], - "requires": [], - "runtimeCompat": ["all"], - "produces": ["linear-issue-links"], - "consumes": ["ROADMAP.md"] - }, - "discussion": "https://github.com/open-gsd/gsd-core/discussions/1234" -} -``` diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md deleted file mode 100644 index 51a550488..000000000 --- a/.github/pull_request_template.md +++ /dev/null @@ -1,41 +0,0 @@ -## ⚠️ Wrong template — please use the correct one for your PR type - -Every PR must use a typed template. Using this default template is a reason for rejection. - -Select the template that matches your PR: - -| PR Type | When to use | Template link | -|---------|-------------|---------------| -| **Fix** | Correcting a bug, crash, or behavior that doesn't match documentation | [Use fix template](?template=PULL_REQUEST_TEMPLATE/fix.md) | -| **Enhancement** | Improving an existing feature — better output, expanded edge cases, performance | [Use enhancement template](?template=PULL_REQUEST_TEMPLATE/enhancement.md) | -| **Feature** | Adding something new — new command, workflow, concept, or integration | [Use feature template](?template=PULL_REQUEST_TEMPLATE/feature.md) | - ---- - -### Not sure which type applies? - -- If it **corrects broken behavior** → Fix -- If it **improves existing behavior** without adding new commands or concepts → Enhancement -- If it **adds something that doesn't exist today** → Feature -- If you are not sure → open a [Discussion](https://github.com/open-gsd/gsd-core/discussions) first - ---- - -### Reminder: Issues must be approved before PRs - -For **enhancements**: the linked issue must have the `approved-enhancement` label before you open this PR. - -For **features**: the linked issue must have the `approved-feature` label before you open this PR. - -PRs that arrive without a labeled, approved issue are closed without review. - -> **No draft PRs.** Draft PRs are automatically closed. Only open a PR when your code is complete, tests pass, and the correct template is used. See [CONTRIBUTING.md](../CONTRIBUTING.md). - -See [CONTRIBUTING.md](../CONTRIBUTING.md) for the full process. - ---- - - - diff --git a/.github/workflows/auto-branch.yml b/.github/workflows/auto-branch.yml deleted file mode 100644 index d1318dec2..000000000 --- a/.github/workflows/auto-branch.yml +++ /dev/null @@ -1,102 +0,0 @@ -name: Auto-Branch from Issue Label - -on: - issues: - types: [labeled] - -concurrency: - group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true - -permissions: - contents: write - issues: write - -jobs: - create-branch: - runs-on: ubuntu-latest - timeout-minutes: 2 - if: >- - contains(fromJSON('["bug", "enhancement", "priority: critical", "type: chore", "area: docs"]'), - github.event.label.name) - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - - name: Create branch - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - script: | - const label = context.payload.label.name; - const issue = context.payload.issue; - const number = issue.number; - - // Generate slug from title - const slug = issue.title - .toLowerCase() - .replace(/[^a-z0-9]+/g, '-') - .replace(/^-+|-+$/g, '') - .substring(0, 40); - - // Map label to branch prefix - const prefixMap = { - 'bug': 'fix', - 'enhancement': 'feat', - 'priority: critical': 'fix', - 'type: chore': 'chore', - 'area: docs': 'docs', - }; - const prefix = prefixMap[label]; - if (!prefix) return; - - // For priority: critical, use fix/critical-NNN-slug to avoid - // colliding with the hotfix workflow's hotfix/X.Y.Z naming. - const branch = label === 'priority: critical' - ? `fix/critical-${number}-${slug}` - : `${prefix}/${number}-${slug}`; - - // Check if branch already exists - try { - await github.rest.git.getRef({ - owner: context.repo.owner, - repo: context.repo.repo, - ref: `heads/${branch}`, - }); - core.info(`Branch ${branch} already exists`); - return; - } catch (e) { - if (e.status !== 404) throw e; - } - - // Create branch from next HEAD (the integration branch under the - // next-branch model). Fall back to main if next doesn't exist — - // covers the brief transition window when this workflow was first - // deployed and any legacy single-branch state. - let baseRef; - try { - baseRef = await github.rest.git.getRef({ - owner: context.repo.owner, - repo: context.repo.repo, - ref: 'heads/next', - }); - } catch (e) { - if (e.status !== 404) throw e; - baseRef = await github.rest.git.getRef({ - owner: context.repo.owner, - repo: context.repo.repo, - ref: 'heads/main', - }); - } - - await github.rest.git.createRef({ - owner: context.repo.owner, - repo: context.repo.repo, - ref: `refs/heads/${branch}`, - sha: baseRef.data.object.sha, - }); - - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: number, - body: `Branch \`${branch}\` created.\n\n\`\`\`bash\ngit fetch origin && git checkout ${branch}\n\`\`\``, - }); diff --git a/.github/workflows/auto-close-unsolicited-prs.yml b/.github/workflows/auto-close-unsolicited-prs.yml deleted file mode 100644 index 41849063f..000000000 --- a/.github/workflows/auto-close-unsolicited-prs.yml +++ /dev/null @@ -1,159 +0,0 @@ -name: Auto-Close Unsolicited PRs - -# pull_request_target (not pull_request) so the job runs in the base-repo -# context with a write-capable token even for PRs from forks. Without this, -# fork PRs from first-time/external contributors get a read-only GITHUB_TOKEN -# (the repo default is `read`) and the close/comment API calls 403. Worse, the -# equivalent `pull_request`-triggered gate (require-issue-link) is held behind -# GitHub's first-time-contributor approval policy and never runs at all, so a -# no-issue drive-by PR sits open until a maintainer closes it by hand. -# Safe because this job only reads event metadata and the linked issue's -# labels via the API; it never checks out or executes PR-supplied code. -# Residual platform limitation: GitHub deliberately does NOT trigger -# pull_request_target for fork branches whose names look like a Git SHA, so a -# contributor could still evade this by naming their head branch like a commit -# hash. Fully closing that gap needs a scheduled base-context sweep (tracked as -# a follow-up); a PR that evades this still cannot merge and still fails the -# other gates. -on: - # opened only — NOT reopened. A maintainer who reopens an external PR - # "closed in error" must not have it immediately re-closed (the job checks - # the PR author's association, which is unchanged on reopen). Re-closing is - # the maintainer's call; this workflow only acts at open. - pull_request_target: - types: [opened] - -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -permissions: - pull-requests: write - issues: read - -jobs: - close-if-unapproved: - name: Reject PRs without a pre-approved issue - # Skip maintainers (they may open internal coordination PRs) and drafts - # (handled by close-draft-prs.yml). Only non-member, ready-for-review PRs - # are evaluated. - if: >- - github.event.pull_request.draft == false && - github.event.pull_request.user.login != 'dependabot[bot]' && - contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.pull_request.author_association) == false - runs-on: ubuntu-latest - timeout-minutes: 2 - env: - # Maintainer-applied approval labels. A linked issue must carry one of - # these for an external PR to be accepted. Anyone can open an issue or - # cite a number, but only users with triage/write can apply labels — so - # requiring a label defeats forged or self-opened "approval" issues. - APPROVAL_LABELS: 'approved-feature,approved-enhancement,confirmed-bug' - steps: - - name: Close unless PR links a pre-approved issue - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - script: | - const pr = context.payload.pull_request; - const body = pr.body || ''; - const approvalLabels = process.env.APPROVAL_LABELS - .split(',').map(s => s.trim()).filter(Boolean); - - // Collect SAME-REPO issue numbers referenced with a GitHub closing - // keyword. Cross-repo and URL refs are resolved only when they point - // back at this repo — an approving label must live on an issue here. - const owner = context.repo.owner; - const repo = context.repo.repo; - // Ignore references inside fenced/inline code so example or template - // snippets (e.g. a documented `Closes #123`) don't count as a link. - const scanBody = body - .replace(/```[\s\S]*?```/g, '') - .replace(/`[^`]*`/g, ''); - const refRe = /\b(?:close[sd]?|fix(?:es|ed)?|resolve[sd]?)\b[\s:]*(?:#(\d+)|([\w.-]+)\/([\w.-]+)#(\d+)|https?:\/\/github\.com\/([\w.-]+)\/([\w.-]+)\/issues\/(\d+))/gi; - const numbers = new Set(); - for (const m of scanBody.matchAll(refRe)) { - if (m[1]) { - numbers.add(Number(m[1])); - } else if (m[2] && m[2].toLowerCase() === owner.toLowerCase() && m[3].toLowerCase() === repo.toLowerCase()) { - numbers.add(Number(m[4])); - } else if (m[5] && m[5].toLowerCase() === owner.toLowerCase() && m[6].toLowerCase() === repo.toLowerCase()) { - numbers.add(Number(m[7])); - } - } - - // Accept the PR only if it links at least one issue in this repo - // that carries a maintainer-applied approval label. - // Cap the number of issues we fetch: a body stuffed with hundreds - // of refs must not stall the job past its timeout (which would fail - // open and leave an unapproved PR unclosed). - const MAX_ISSUE_CHECKS = 20; - let approved = false; - for (const number of [...numbers].slice(0, MAX_ISSUE_CHECKS)) { - let issue; - try { - issue = await github.rest.issues.get({ owner, repo, issue_number: number }); - } catch (err) { - if (err.status === 404) { - core.info(`Referenced #${number} not found — ignoring.`); - continue; - } - // Indeterminate failure (rate limit / 5xx / network). Do NOT - // close — failing open here avoids wrongly closing a PR whose - // only linked issue is genuinely approved but momentarily - // unreadable. A re-run or the maintainer can resolve it. - core.setFailed(`Could not verify issue #${number} (${err.status || err.message}); leaving PR #${pr.number} open.`); - return; - } - if (issue.data.pull_request) { - core.info(`#${number} is a pull request, not an issue — ignoring.`); - continue; - } - const labels = (issue.data.labels || []) - .map(l => (typeof l === 'string' ? l : l.name)); - if (labels.some(l => approvalLabels.includes(l))) { - core.info(`#${number} carries an approval label — leaving PR #${pr.number} open.`); - approved = true; - break; - } - core.info(`#${number} has no approval label.`); - } - - if (approved) { - return; - } - - const reason = numbers.size === 0 - ? 'it does not link an issue' - : 'the linked issue is not approved'; - const repoUrl = `${owner}/${repo}`; - const marker = ''; - const message = [ - marker, - '## Closing — no pre-approved issue', - '', - `Thanks for your interest in MSD! This PR was closed automatically because ${reason}.`, - '', - '**MSD requires a pre-approved issue before any PR.** The PR must link an issue in this repository that carries a maintainer-applied approval label — `approved-feature`, `approved-enhancement`, or `confirmed-bug`. Opening your own issue or citing an unrelated number is not enough: the label is applied by maintainers after triage.', - '', - '### What to do', - '', - `1. [Open an issue](https://github.com/${repoUrl}/issues/new/choose) describing the change (bug, enhancement, or feature).`, - '2. Wait for a maintainer to approve it (`confirmed-bug`, `approved-enhancement`, or `approved-feature`).', - '3. Open a new PR using the matching template, with `Closes #` in the body.', - '', - `See [CONTRIBUTING.md](https://github.com/${repoUrl}/blob/main/CONTRIBUTING.md) for the full process. If you believe this was closed in error, comment here and a maintainer can reopen it.`, - ].join('\n'); - - // Upsert a sticky comment so a reopen-then-reclose doesn't spam. - const comments = await github.paginate(github.rest.issues.listComments, { - owner, repo, issue_number: pr.number, per_page: 100, - }); - const existing = comments.find(c => c.body && c.body.includes(marker)); - if (existing) { - await github.rest.issues.updateComment({ owner, repo, comment_id: existing.id, body: message }); - } else { - await github.rest.issues.createComment({ owner, repo, issue_number: pr.number, body: message }); - } - - await github.rest.pulls.update({ owner, repo, pull_number: pr.number, state: 'closed' }); - core.info(`Closed PR #${pr.number} (${reason}): ${pr.title}`); diff --git a/.github/workflows/auto-label-issues.yml b/.github/workflows/auto-label-issues.yml deleted file mode 100644 index 109a25c2b..000000000 --- a/.github/workflows/auto-label-issues.yml +++ /dev/null @@ -1,25 +0,0 @@ -name: Auto-label new issues - -on: - issues: - types: [opened] - -concurrency: - group: ${{ github.workflow }}-${{ github.event.issue.number }} - cancel-in-progress: true - -jobs: - add-triage-label: - runs-on: ubuntu-latest - permissions: - issues: write - steps: - - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - script: | - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.issue.number, - labels: ["needs-triage"] - }) diff --git a/.github/workflows/close-draft-prs-sweep.yml b/.github/workflows/close-draft-prs-sweep.yml deleted file mode 100644 index 2df574e57..000000000 --- a/.github/workflows/close-draft-prs-sweep.yml +++ /dev/null @@ -1,112 +0,0 @@ -name: Close Draft PRs (sweep) - -# Companion to close-draft-prs.yml. That workflow runs per-PR on -# pull_request_target and is the fast path. GitHub does NOT dispatch -# pull_request_target for fork branches whose names look like a Git SHA, so a -# fork draft PR on a SHA-named branch can never be closed by any PR-triggered -# event (a pull_request run from a fork gets a read-only token). This scheduled -# sweep runs in the base-repo context with a write-capable token and enforces -# the identical policy on a timer, catching that evasion. See issue #761. - -on: - schedule: - - cron: '0 */6 * * *' - workflow_dispatch: - -concurrency: - group: close-draft-prs-sweep - cancel-in-progress: false - -permissions: - pull-requests: write - -jobs: - sweep-draft-prs: - name: Sweep open draft PRs - runs-on: ubuntu-latest - steps: - - name: Close non-maintainer draft PRs - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - script: | - // Maintainers may use draft PRs for internal coordination — same - // carve-out as close-draft-prs.yml. A scheduled run has no - // github.event.pull_request, so the carve-out is applied here as a - // Set membership test over author_association. - const MAINTAINER_ASSOCIATIONS = new Set(['OWNER', 'MEMBER', 'COLLABORATOR']); - const repoUrl = context.repo.owner + '/' + context.repo.repo; - - const commentBody = [ - '## Draft PRs are not accepted', - '', - 'This project only accepts completed pull requests. Draft PRs are automatically closed.', - '', - '**Why?** MSD requires all PRs to be ready for review when opened \u2014 with tests passing, the correct PR template used, and a linked approved issue. Draft PRs bypass these quality gates and create review overhead.', - '', - '### What to do instead', - '', - '1. Finish your implementation locally', - '2. Run `npm run test:coverage` and confirm all tests pass', - '3. Open a **non-draft** PR using the [correct template](https://github.com/' + repoUrl + '/blob/main/CONTRIBUTING.md#pull-request-guidelines)', - '', - 'See [CONTRIBUTING.md](https://github.com/' + repoUrl + '/blob/main/CONTRIBUTING.md) for the full process.', - ].join('\n'); - - const isEnforceableDraft = (pr) => - !!pr && pr.state === 'open' && pr.draft === true && - !MAINTAINER_ASSOCIATIONS.has(pr.author_association); - - const openPulls = await github.paginate(github.rest.pulls.list, { - owner: context.repo.owner, - repo: context.repo.repo, - state: 'open', - per_page: 100, - }); - - const candidates = openPulls.filter(isEnforceableDraft); - core.info('Sweep found ' + candidates.length + ' non-maintainer draft PR(s) of ' + openPulls.length + ' open.'); - - const failures = []; - - for (const candidate of candidates) { - try { - // Re-fetch immediately before mutating: the contributor may have - // marked the PR ready (or it may have closed) since pagination. - const { data: pr } = await github.rest.pulls.get({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: candidate.number, - }); - - if (!isEnforceableDraft(pr)) { - core.info('Skipping PR #' + candidate.number + ' — no longer an open non-maintainer draft.'); - continue; - } - - // Close FIRST so enforcement (the primary action) is never gated - // on the explanatory comment. A closed PR is never revisited by a - // later sweep, so this also prevents duplicate comments. - await github.rest.pulls.update({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: pr.number, - state: 'closed' - }); - - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - body: commentBody - }); - - core.info('Closed draft PR #' + pr.number + ': ' + pr.title); - } catch (err) { - failures.push(candidate.number); - core.warning('Failed to process draft PR #' + candidate.number + ': ' + err.message); - } - } - - if (failures.length > 0) { - core.setFailed('Sweep encountered errors on ' + failures.length + ' draft PR(s): ' + failures.join(', ')); - } diff --git a/.github/workflows/close-draft-prs.yml b/.github/workflows/close-draft-prs.yml deleted file mode 100644 index ec30a0668..000000000 --- a/.github/workflows/close-draft-prs.yml +++ /dev/null @@ -1,69 +0,0 @@ -name: Close Draft PRs - -# pull_request_target (not pull_request) so the job runs in the base-repo -# context with a write-capable token even for PRs from forks. Without this, -# fork PRs from first-time/external contributors get a read-only GITHUB_TOKEN -# and the close/comment API calls 403 — letting them bypass the auto-close. -# Safe because this job only reads event metadata and never checks out or -# executes PR-supplied code. -# Residual platform limitation: GitHub deliberately does NOT trigger -# pull_request_target for fork branches whose names look like a Git SHA, so a -# contributor could still evade this by naming their head branch like a commit -# hash. Fully closing that gap needs a scheduled base-context sweep (separate -# concern); a draft PR that evades this still cannot merge and still fails the other gates. -on: - pull_request_target: - types: [opened, reopened, converted_to_draft] - -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -permissions: - pull-requests: write - -jobs: - close-if-draft: - name: Reject draft PRs - # Maintainers may use draft PRs for internal coordination. - if: >- - github.event.pull_request.draft == true && - contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.pull_request.author_association) == false - runs-on: ubuntu-latest - steps: - - name: Comment and close draft PR - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - script: | - const pr = context.payload.pull_request; - const repoUrl = context.repo.owner + '/' + context.repo.repo; - - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - body: [ - '## Draft PRs are not accepted', - '', - 'This project only accepts completed pull requests. Draft PRs are automatically closed.', - '', - '**Why?** MSD requires all PRs to be ready for review when opened \u2014 with tests passing, the correct PR template used, and a linked approved issue. Draft PRs bypass these quality gates and create review overhead.', - '', - '### What to do instead', - '', - '1. Finish your implementation locally', - '2. Run `npm run test:coverage` and confirm all tests pass', - '3. Open a **non-draft** PR using the [correct template](https://github.com/' + repoUrl + '/blob/main/CONTRIBUTING.md#pull-request-guidelines)', - '', - 'See [CONTRIBUTING.md](https://github.com/' + repoUrl + '/blob/main/CONTRIBUTING.md) for the full process.', - ].join('\n') - }); - - await github.rest.pulls.update({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: pr.number, - state: 'closed' - }); - - core.info('Closed draft PR #' + pr.number + ': ' + pr.title); diff --git a/.github/workflows/duplicate-check.yml b/.github/workflows/duplicate-check.yml deleted file mode 100644 index 0931c4575..000000000 --- a/.github/workflows/duplicate-check.yml +++ /dev/null @@ -1,54 +0,0 @@ -name: Duplicate check - -on: - issues: - types: [opened] - -concurrency: - group: ${{ github.workflow }}-${{ github.event.issue.number }} - cancel-in-progress: true - -permissions: - issues: write - contents: read - -jobs: - detect: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - script: | - const dedupe = require(`${process.env.GITHUB_WORKSPACE}/scripts/issue-dedupe.cjs`); - const issue = context.payload.issue; - if (issue.pull_request) return; - const existing = (issue.labels || []).map((l) => (typeof l === 'string' ? l : l.name)); - if (existing.includes(dedupe.POSSIBLE_DUPLICATE_LABEL)) return; - const open = await github.paginate(github.rest.issues.listForRepo, { - owner: context.repo.owner, - repo: context.repo.repo, - state: 'open', - per_page: 100, - }); - const candidates = open - .filter((i) => !i.pull_request && i.number !== issue.number) - .map((i) => ({ number: i.number, title: i.title })); - const matches = dedupe.scoreCandidates(issue.title, candidates, { excludeNumber: issue.number }); - if (!matches.length) { - core.info('No similar open issues found.'); - return; - } - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: issue.number, - body: dedupe.renderChallengeComment(matches, { windowHours: dedupe.DEFAULT_WINDOW_HOURS }), - }); - await github.rest.issues.addLabels({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: issue.number, - labels: [dedupe.POSSIBLE_DUPLICATE_LABEL], - }); - core.info(`Flagged #${issue.number} as possible duplicate of: ${matches.map((m) => '#' + m.number).join(', ')}`); diff --git a/.github/workflows/duplicate-sweep.yml b/.github/workflows/duplicate-sweep.yml deleted file mode 100644 index 083c32118..000000000 --- a/.github/workflows/duplicate-sweep.yml +++ /dev/null @@ -1,91 +0,0 @@ -name: Duplicate auto-close sweep - -on: - schedule: - - cron: '0 7 * * *' - workflow_dispatch: - -permissions: - issues: write - contents: read - -jobs: - sweep: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - script: | - const dedupe = require(`${process.env.GITHUB_WORKSPACE}/scripts/issue-dedupe.cjs`); - const { owner, repo } = context.repo; - const issues = await github.paginate(github.rest.issues.listForRepo, { - owner, - repo, - state: 'open', - labels: dedupe.POSSIBLE_DUPLICATE_LABEL, - per_page: 100, - }); - const now = Date.now(); - for (const issue of issues) { - if (issue.pull_request) continue; - const comments = await github.paginate(github.rest.issues.listComments, { - owner, - repo, - issue_number: issue.number, - per_page: 100, - }); - const challenges = comments.filter((c) => dedupe.isChallengeComment(c.body)); - const challenge = challenges[challenges.length - 1]; - let challengeComment = null; - let laterUserComments = 0; - if (challenge) { - const challengeAt = new Date(challenge.created_at).getTime(); - laterUserComments = comments.filter( - (c) => new Date(c.created_at).getTime() > challengeAt && c.user && c.user.type !== 'Bot', - ).length; - const reactions = await github.paginate(github.rest.reactions.listForIssueComment, { - owner, - repo, - comment_id: challenge.id, - per_page: 100, - }); - const downvoted = reactions.some((r) => r.content === '-1'); - challengeComment = { createdAt: challenge.created_at, downvoted }; - } - const decision = dedupe.shouldClose({ - now, - labels: issue.labels, - challengeComment, - laterUserComments, - windowHours: dedupe.DEFAULT_WINDOW_HOURS, - }); - core.info(`#${issue.number}: ${decision.reason}`); - if (!decision.close) continue; - const fresh = await github.rest.issues.get({ owner, repo, issue_number: issue.number }); - if (fresh.data.state !== 'open') continue; - const freshLabels = (fresh.data.labels || []).map((l) => (typeof l === 'string' ? l : l.name)); - if (!freshLabels.includes(dedupe.POSSIBLE_DUPLICATE_LABEL)) { - core.info(`#${issue.number}: possible-duplicate cleared since snapshot, skipping close`); - continue; - } - await github.rest.issues.createComment({ - owner, - repo, - issue_number: issue.number, - body: `Closing as a likely duplicate — no response within ${dedupe.DEFAULT_WINDOW_HOURS}h of the duplicate check. If this was a mistake, comment and a maintainer will reopen it.`, - }); - await github.rest.issues.update({ - owner, - repo, - issue_number: issue.number, - state: 'closed', - state_reason: 'duplicate', - }); - await github.rest.issues.removeLabel({ - owner, - repo, - issue_number: issue.number, - name: dedupe.POSSIBLE_DUPLICATE_LABEL, - }).catch((e) => core.info(`removeLabel after close: ${e.message}`)); - } diff --git a/.github/workflows/pr-template-format.yml b/.github/workflows/pr-template-format.yml deleted file mode 100644 index bfca5da64..000000000 --- a/.github/workflows/pr-template-format.yml +++ /dev/null @@ -1,126 +0,0 @@ -name: PR Template Format - -on: - pull_request_target: - types: [opened, edited, reopened, synchronize, ready_for_review] - -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -permissions: - contents: read - issues: write - pull-requests: write - -jobs: - check-template-format: - name: Pull request template format - runs-on: ubuntu-latest - timeout-minutes: 2 - steps: - - name: Check out policy from base branch - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - - name: Get changed files - id: changed_files - env: - GH_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ github.event.pull_request.number }} - run: | - # The delimiter must be unguessable: every path in `files` is - # attacker-controlled on a fork PR, and a file named after a fixed - # delimiter (e.g. a file literally named `EOF`, the weakest - # possible choice here) would terminate the heredoc value early, - # truncating everything after it — this is GitHub's own documented - # guidance for untrusted multiline output - # (https://docs.github.com/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings). - # The length-vs-total check in the policy (fileListIsComplete) is - # the second, independent layer. - delim="MSD_EOF_$(openssl rand -hex 16)" - files=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json files --jq '.files[].path') - echo "files<<$delim" >> "$GITHUB_OUTPUT" - echo "$files" >> "$GITHUB_OUTPUT" - echo "$delim" >> "$GITHUB_OUTPUT" - - - name: Evaluate PR template format - id: policy - env: - PR_BODY: ${{ github.event.pull_request.body }} - AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association }} - CHANGED_FILES: ${{ steps.changed_files.outputs.files }} - CHANGED_FILES_TOTAL: ${{ github.event.pull_request.changed_files }} - run: node scripts/pr-template-policy.cjs - - - name: Warn trusted contributor about missing template - if: steps.policy.outputs.action == 'warn' - uses: actions/github-script@d746ffe35508b1917358783b479e04febd2b8f71 # v9.0.0 - with: - script: | - const marker = ''; - const pr = context.payload.pull_request; - const policy = JSON.parse(${{ toJSON(steps.policy.outputs.result) }}); - const comments = await github.paginate(github.rest.issues.listComments, { - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - per_page: 100, - }); - if (!comments.some((comment) => comment.body && comment.body.includes(marker))) { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - body: [ - marker, - '## Please use the PR template format', - '', - 'This PR is allowed to stay open because the author is a contributor or higher, but the body does not follow one of the required PR templates.', - '', - 'Please update the PR description to use the fix, enhancement, or feature template format from `CONTRIBUTING.md` so the automated documentation modules can do their job.', - '', - `Detected problem: ${policy.reason}`, - ].join('\n'), - }); - } - core.warning(policy.reason); - - - name: Comment and fail untrusted malformed PR - if: steps.policy.outputs.action == 'close' - uses: actions/github-script@d746ffe35508b1917358783b479e04febd2b8f71 # v9.0.0 - with: - script: | - const marker = ''; - const pr = context.payload.pull_request; - const repoUrl = `https://github.com/${context.repo.owner}/${context.repo.repo}`; - const policy = JSON.parse(${{ toJSON(steps.policy.outputs.result) }}); - const comments = await github.paginate(github.rest.issues.listComments, { - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - per_page: 100, - }); - if (!comments.some((comment) => comment.body && comment.body.includes(marker))) { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - body: [ - marker, - '## PR template required', - '', - 'This PR does not follow one of the required pull request templates from the contribution guidelines.', - '', - 'Please update the PR description to use the correct typed template:', - '', - `- [Fix PR template](${repoUrl}/blob/main/.github/PULL_REQUEST_TEMPLATE/fix.md)`, - `- [Enhancement PR template](${repoUrl}/blob/main/.github/PULL_REQUEST_TEMPLATE/enhancement.md)`, - `- [Feature PR template](${repoUrl}/blob/main/.github/PULL_REQUEST_TEMPLATE/feature.md)`, - '', - 'This check only validates template format. It does not close PRs for an unfilled issue number such as `Fixes #`; the issue-link workflow handles issue references separately after the PR exists.', - '', - `Detected problem: ${policy.reason}`, - ].join('\n'), - }); - } - core.setFailed(policy.reason); diff --git a/.github/workflows/pr-title-validator.yml b/.github/workflows/pr-title-validator.yml deleted file mode 100644 index 92a7da896..000000000 --- a/.github/workflows/pr-title-validator.yml +++ /dev/null @@ -1,204 +0,0 @@ -name: PR Title Validator - -# Enforce the PR-title convention the release changelog depends on (#1549). -# -# The changelog is entirely title-driven: release.yml runs -# `gh release create --generate-notes` (GitHub builds "What's Changed" from PR -# titles) and scripts/release-notes/format-github-release-notes.cjs reformats -# it. Two independent rules are read off the title: -# 1. Bucket — classifyBucket() anchors on the leading type (^feat / ^fix / -# else Enhancement). A leading tag (e.g. `[security] `) defeats -# the anchor and silently mis-files the entry. -# 2. Issue link — the `(#)` in the title is what renders as a link to -# the issue in the changelog line. -# -# This gate reuses the SAME matcher the changelog uses -# (scripts/release-notes/conventional-title.cjs) — not a forked regex — so a title that -# passes here cannot mis-bucket in the changelog. -# -# Trust boundary: the matcher is loaded from a BASE-branch checkout (the -# already-merged, reviewed copy on the PR's target), exactly as -# pr-target-validator loads its policy. The PR cannot edit the ruler that -# measures its own title, so the gate is not self-bypassable. Until this -# matcher lands on the base branch it does not exist there — the introducing -# PR is skipped (bootstrap); every PR after merge is fully gated. -# -# Trigger (#2331): pull_request_target, NOT pull_request. A `pull_request` -# event raised from a fork hands the job a read-only GITHUB_TOKEN regardless of -# the `permissions:` block below, so the sticky-comment call 403s, the -# unhandled rejection kills this step, and core.setFailed never runs — the -# contributor sees an API stack trace instead of the retitle instructions. -# pull_request_target runs in the base-repo context with a write-capable token. -# This is safe here for the same reason it is safe in pr-template-format.yml: -# the only checkout is the BASE branch (persist-credentials: false) and the -# only PR-controlled input is `pr.title`, read as data. No head code executes. -# The trust boundary above is reinforced, not weakened — pull_request_target -# checks out base by definition. -# -# Unlike pr-target-validator, this runs for ALL authors (including members): -# the changelog drift that motivated #1549 came from member PRs. -# -# Phase-1 rollout: set WARN_ONLY=true to comment without failing the check. -# Shipped enforcing (WARN_ONLY=false); flip to 'true' for a grace period. -# -# See: scripts/release-notes/conventional-title.cjs, CONTRIBUTING.md, issue #1549. - -on: - pull_request_target: - types: [opened, edited, reopened, synchronize] - -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number }} - cancel-in-progress: true - -# Scope unchanged by #2331 — deliberately. `pull-requests: write` alone already -# authorizes github.rest.issues.createComment on a PR (GitHub accepts EITHER -# `issues` or `pull-requests` write for the issue-comments endpoint when the -# target is a PR). Verified against this repo's own history rather than the -# docs: this workflow has only ever declared `pull-requests: write` and its -# sticky comment has posted 26 times; require-issue-link.yml declares only -# `issues: write` and its comment posts too. The 403 was the fork token -# downgrade, NOT a missing scope — so widening the scope here would add -# privilege on a pull_request_target workflow while fixing nothing. -permissions: - contents: read - pull-requests: write - -jobs: - validate-title: - runs-on: ubuntu-latest - timeout-minutes: 2 - env: - # Phase-1: set to 'true' to warn only. Shipped enforcing. - WARN_ONLY: 'false' - steps: - # Check out the BASE branch (the PR's merge target) as the trusted policy - # source — not the PR head. The matcher that judges the title must be - # already-merged, reviewed code so a PR cannot bypass the gate by editing - # conventional-title.cjs to accept its own malformed title. Mirrors - # pr-target-validator.yml. The introducing PR is handled by the bootstrap - # guard in the script below (the matcher isn't on base yet). - - name: Checkout base branch (trusted policy source) - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - ref: ${{ github.event.pull_request.base.ref }} - persist-credentials: false - - - name: Validate PR title - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - env: - WARN_ONLY: ${{ env.WARN_ONLY }} - with: - script: | - const fs = require('fs'); - const matcherPath = `${process.env.GITHUB_WORKSPACE}/scripts/release-notes/conventional-title.cjs`; - - const pr = context.payload.pull_request; - - // #4196: Dependabot PR titles (e.g. "chore(deps): bump ...") - // never carry `(#)` — there's no issue to link (its - // alerts live in the Security tab, not as repo issues). Exempt - // by author login, not title/branch shape, since that field is - // authenticated by GitHub and not forgeable. - if (pr.user && pr.user.login === 'dependabot[bot]') { - core.info('PR opened by dependabot[bot] — skipping title convention check.'); - return; - } - - const title = pr.title || ''; - const warnOnly = process.env.WARN_ONLY === 'true'; - - // Bootstrap: the matcher is loaded from the base-branch checkout, so it - // is absent on the PR that first introduces it. Skip rather than fail — - // once this lands on the base branch, every subsequent PR is gated. - if (!fs.existsSync(matcherPath)) { - core.info('conventional-title.cjs not on the base branch yet — bootstrap PR, skipping title check.'); - return; - } - const { evaluatePrTitle } = require(matcherPath); - - const result = evaluatePrTitle({ title }); - - if (result.valid) { - core.info(`PR title OK: ${title}`); - return; - } - - // #2331: `title` is attacker-controlled free text (a PR title has no - // charset restriction) and the comment below is posted by - // github-actions[bot] with a write token. Echoed raw into an inline-code - // span, a single backtick in the title closes the span early and the - // remainder renders as live Markdown — GFM autolinks a bare URL, so a - // fork author could make our own bot post an arbitrary clickable link - // into the PR thread (phishing that borrows the bot's credibility). - // Stripping the backtick is sufficient and complete: it is the only - // character that can break out of an inline-code span, and everything - // else is inert once it cannot. Only the RENDERED body needs this; - // core.warning/core.setFailed below go to the job log, not Markdown, - // and @actions/core already escapes workflow-command sequences there. - const titleForMarkdown = String(title).replace(/`/g, "'"); - - const msg = [ - `### PR title needs the issue-ref convention`, - ``, - `\`${titleForMarkdown}\``, - ``, - result.message, - ``, - `**How to fix:** click "Edit" next to the PR title above and retitle it`, - `as \`type(#): summary\`. No need to recreate the PR — this check`, - `re-runs when you edit the title.`, - ``, - `
Why this is enforced`, - ``, - `The release changelog is built from PR titles. A leading tag mis-files`, - `the entry into the wrong section, and a scope without \`(#)\` leaves`, - `the changelog line with no link back to the issue. See issue #1549.`, - ``, - `
`, - ].join('\n'); - - // Post or update a sticky comment. - // - // #2331: the comment is a COURTESY, the verdict below is the GATE. - // Never let a failure here suppress the verdict — that inversion is - // the bug this guard exists to prevent (a 403 on the createComment - // call used to kill the step before core.setFailed ran, replacing - // "retitle as type(#issue): summary" with an API stack trace). - // pull_request_target should make the 403 impossible; this catch is - // defense in depth so a future permission change degrades the - // diagnostic rather than the gate. - try { - const { data: comments } = await github.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - }); - const marker = ''; - const existing = comments.find(c => c.body && c.body.includes(marker)); - const body = `${marker}\n${msg}`; - if (existing) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: existing.id, - body, - }); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - body, - }); - } - } catch (err) { - // Surface the guidance in the job log so it is not lost entirely. - core.warning(`Could not post the PR-title comment (${err.status || err.message}). Guidance follows:\n${msg}`); - } - - if (warnOnly) { - core.warning(`PR title convention (warning-only mode): ${result.reason} — ${title}`); - } else { - core.setFailed(`PR title does not follow the convention (${result.reason}): ${title}`); - } diff --git a/.github/workflows/remove-duplicate-label.yml b/.github/workflows/remove-duplicate-label.yml deleted file mode 100644 index af71abd9e..000000000 --- a/.github/workflows/remove-duplicate-label.yml +++ /dev/null @@ -1,39 +0,0 @@ -name: Clear possible-duplicate on response - -on: - issue_comment: - types: [created] - -permissions: - issues: write - contents: read - -jobs: - clear: - if: ${{ !github.event.issue.pull_request }} - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - with: - script: | - const dedupe = require(`${process.env.GITHUB_WORKSPACE}/scripts/issue-dedupe.cjs`); - const { owner, repo } = context.repo; - const comment = context.payload.comment; - const issue = context.payload.issue; - if (comment.user && comment.user.type === 'Bot') return; - const existing = (issue.labels || []).map((l) => (typeof l === 'string' ? l : l.name)); - if (!existing.includes(dedupe.POSSIBLE_DUPLICATE_LABEL)) return; - await github.rest.issues.removeLabel({ - owner, - repo, - issue_number: issue.number, - name: dedupe.POSSIBLE_DUPLICATE_LABEL, - }).catch((e) => core.info(`removeLabel: ${e.message}`)); - await github.rest.issues.addLabels({ - owner, - repo, - issue_number: issue.number, - labels: [dedupe.HUMAN_REVIEW_LABEL], - }); - core.info(`Cleared possible-duplicate on #${issue.number}; routed to ${dedupe.HUMAN_REVIEW_LABEL}.`); diff --git a/.github/workflows/require-issue-link.yml b/.github/workflows/require-issue-link.yml deleted file mode 100644 index a67674219..000000000 --- a/.github/workflows/require-issue-link.yml +++ /dev/null @@ -1,228 +0,0 @@ -name: Require Issue Link - -# Trigger (#2331): pull_request_target, NOT pull_request. A `pull_request` event -# raised from a fork hands the job a read-only GITHUB_TOKEN regardless of the -# `permissions:` block, so the createComment call below 403s, the unhandled -# rejection kills the step, and the core.setFailed on the last line never runs — -# the contributor sees an API stack trace instead of "add Closes #NNN". -# pull_request_target runs in the base-repo context with a write-capable token. -# Safe here: the only checkout is the BASE branch (persist-credentials: false), -# and every PR-controlled input — body, head ref, changed-file list — is read as -# data via env vars, never interpolated into a shell. No head code executes. The -# #1389 fork-forgery carve-out still holds — it is keyed on -# head.repo.full_name == github.repository, not on the branch name alone. -# -# #3211: the gate used to model exactly one PR→issue relationship ("this PR -# closes that issue"), so a test-only or docs-only FOLLOW-UP PR — which -# references an issue without closing it — had no way to pass. A fork could not -# use the backmerge carve-out either (it requires same-repo identity), so the -# only green path was a knowingly-inert `Closes #N` against an already-closed -# issue. The verdict now lives in scripts/require-issue-link-policy.cjs, which -# additionally accepts a non-closing reference (`Refs #N`, `Follow-up to #N`, …) -# but ONLY when every changed file is under tests/, under docs/, or a root-level -# *.md — the same doc-only shape pre-pr-gate.sh:111 already recognizes, minus -# CHANGELOG.md, which changeset/lint.cjs classes as user-facing. Ordinary -# source-touching PRs still require a closing keyword, so gate strength is -# unchanged. The exemption is keyed on the server-computed diff shape, which a -# fork author cannot forge — preserving the #1389 property. -# -# Trust boundary: the policy module is loaded from a BASE-branch checkout (the -# already-merged, reviewed copy on the PR's target), exactly as -# pr-title-validator.yml and pr-template-format.yml load theirs. A PR cannot -# edit the ruler that measures it. Until the module lands on the base branch it -# does not exist there — the introducing PR falls back to the legacy -# closing-keyword grep (see the bootstrap branch below) rather than skipping, -# so the gate is never silently open. -on: - pull_request_target: - types: [opened, edited, reopened, synchronize] - -concurrency: - group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} - cancel-in-progress: true - -# `issues: write` alone already authorizes this workflow's issues.createComment -# call on a PR: its sticky comment has posted on same-repo PRs (#106, #164, -# #232, #259) on exactly this scope. The #2331 403 was the fork token -# downgrade, not a missing scope, so no `pull-requests: WRITE` is added — this -# job still calls no mutating pulls.* API. #3211 adds exactly two read scopes, -# each for one concrete need: `contents: read` for the base-branch checkout -# that supplies the policy module, and `pull-requests: read` for the -# `gh pr view --json files,changedFiles` call that supplies the diff shape. -permissions: - contents: read - issues: write - pull-requests: read - -jobs: - check-issue-link: - name: Issue link required - runs-on: ubuntu-latest - timeout-minutes: 2 - steps: - # BASE branch only — the trusted policy source. Never the PR head. - - name: Check out policy from base branch - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - ref: ${{ github.event.pull_request.base.ref }} - persist-credentials: false - - # The authoritative diff shape. `gh pr view --json files` returns at most - # 100 paths and does NOT paginate, so `changedFiles` (the true total) is - # fetched alongside it: the policy compares the two and fails closed when - # they disagree. Without that check a >100-file fork PR could present a - # falsely tests-only list and forge the follow-up exemption (#3211). - - name: Get changed files - id: changed_files - env: - GH_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ github.event.pull_request.number }} - run: | - # The delimiter must be unguessable: every path in `files` is - # attacker-controlled on a fork PR, and a file named after a fixed - # delimiter (e.g. a file literally named `MSD_EOF`) would terminate - # the heredoc value early, truncating everything after it — this is - # GitHub's own documented guidance for untrusted multiline output - # (https://docs.github.com/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings). - # The length-vs-total check in the policy (fileListIsComplete) is - # the second, independent layer: even if a delimiter collision slid - # past this, a truncated or inflated list would still fail closed. - delim="MSD_EOF_$(openssl rand -hex 16)" - files=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json files --jq '.files[].path') - { - echo "files<<$delim" - echo "$files" - echo "$delim" - } >> "$GITHUB_OUTPUT" - - - name: Evaluate issue link - id: policy - env: - # Every PR-controlled value is bound to an env var and read as data — - # never interpolated into the shell or into a JS template literal. - PR_BODY: ${{ github.event.pull_request.body }} - HEAD_REF: ${{ github.head_ref }} - SAME_REPO: ${{ github.event.pull_request.head.repo.full_name == github.repository }} - PR_AUTHOR_LOGIN: ${{ github.event.pull_request.user.login }} - CHANGED_FILES: ${{ steps.changed_files.outputs.files }} - CHANGED_FILES_TOTAL: ${{ github.event.pull_request.changed_files }} - run: | - # Bootstrap: the policy module is loaded from the BASE checkout, so it - # is absent on the PR that first introduces it. Fall back to the legacy - # closing-keyword grep — enforcing, not skipping — so the gate is never - # silently open during the changeover. Mirrors the bootstrap guard in - # pr-title-validator.yml, which skips; this one keeps enforcing because - # the legacy behavior is a strict subset of the new policy. - if [ ! -f scripts/require-issue-link-policy.cjs ]; then - if echo "$PR_BODY" | grep -qiE '(closes|fixes|resolves)\s+#[0-9]+'; then - printf 'ok=true\nreason=ok_closing_keyword\n' >> "$GITHUB_OUTPUT" - else - printf 'ok=false\nreason=fail_no_issue_reference\n' >> "$GITHUB_OUTPUT" - fi - exit 0 - fi - - # Exit 0 = satisfied, 1 = not satisfied (both are verdicts, and the - # step must survive to let the comment step run). Anything else is a - # real error in the policy script and must fail the job loudly rather - # than be mistaken for "no issue link". - set +e - node scripts/require-issue-link-policy.cjs - status=$? - set -e - if [ "$status" -ne 0 ] && [ "$status" -ne 1 ]; then - echo "::error::require-issue-link-policy.cjs exited with status $status" >&2 - exit "$status" - fi - - - name: Comment and fail if no issue link - # Step-level (not job-level) so the required "Issue link required" check - # still reports SUCCESS rather than a branch-protection-blocking - # "skipped" when a PR is exempt. See #1389 — that placement is load - # bearing and every exemption added since must preserve it. - if: steps.policy.outputs.ok != 'true' - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 - env: - REASON: ${{ steps.policy.outputs.reason }} - with: - # Uses GitHub API SDK — no shell string interpolation of untrusted input - script: | - const marker = ''; - const repoUrl = `https://github.com/${context.repo.owner}/${context.repo.repo}`; - const prNumber = context.payload.pull_request.number; - // #3211: the verdict is a typed reason, so the guidance can name the - // rule that actually fired instead of always saying "no issue link". - // Read from an env var — never interpolated into this script body. - const reason = process.env.REASON || 'fail_no_issue_reference'; - const explanation = { - fail_reference_needs_closing: [ - 'This PR references an issue without a closing keyword, but its diff touches files outside `tests/`, `docs/`, and root-level `*.md`.', - '', - 'The non-closing reference form is accepted **only** for test-only or docs-only follow-up PRs. Because this PR changes other files, it needs a closing keyword:', - ], - fail_file_list_incomplete: [ - 'This PR references an issue without a closing keyword, but its changed-file list could not be confirmed as complete, so it cannot be verified as a test-only or docs-only follow-up.', - '', - 'This happens on very large PRs (more than 100 files). Use a closing keyword instead:', - ], - fail_no_issue_reference: [ - 'This PR does not reference an issue. **All PRs must link to an issue** in the PR body:', - ], - }[reason] || [ - 'This PR does not reference an issue. **All PRs must link to an issue** in the PR body:', - ]; - const body = [ - marker, - '## Missing issue link', - '', - ...explanation, - '', - '```', - 'Closes #123', - '```', - '', - 'Accepted closing keywords are `Closes #NNN`, `Fixes #NNN`, and `Resolves #NNN`.', - '', - '**Test-only or docs-only follow-up PR?** If every file your PR changes is under `tests/`, under `docs/`, or a root-level `*.md` (`README.md`, `CONTRIBUTING.md`, … but not `CHANGELOG.md`), and there is no open issue for it to close, reference the related issue without closing it:', - '', - '```', - 'Refs #123', - '```', - '', - '`Ref`, `Refs`, `References`, `Relates to`, `Related to`, and `Follow-up to` are all accepted in that position. See CONTRIBUTING.md → "Link with a closing keyword".', - '', - `If no issue exists for this change, [open one first](${repoUrl}/issues/new/choose), then update this PR body with the reference.`, - '', - 'This check will re-evaluate on the next PR update.', - ].join('\n'); - // #2331: the comment is a COURTESY, the setFailed below is the GATE. - // A failure here must never suppress the verdict. pull_request_target - // should make the 403 impossible; this catch is defense in depth so a - // future permission change degrades the diagnostic, not the gate. - try { - const comments = await github.paginate(github.rest.issues.listComments, { - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, - per_page: 100, - }); - const existing = comments.find(comment => comment.body && comment.body.includes(marker)); - if (existing) { - await github.rest.issues.updateComment({ - owner: context.repo.owner, - repo: context.repo.repo, - comment_id: existing.id, - body, - }); - } else { - await github.rest.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: prNumber, - body, - }); - } - } catch (err) { - core.warning(`Could not post the missing-issue-link comment (${err.status || err.message}). Guidance follows:\n${body}`); - } - core.setFailed('PR body must contain a closing issue reference (e.g. "Closes #123").'); diff --git a/CONTEXT.md b/CONTEXT.md index e7908f559..ddcc60ec5 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -870,11 +870,6 @@ The prompt-level data/instruction isolation seam for untrusted web/document ingr `PRED.k327.cooldown-normal=[5s, 410s]` `PRED.k327.cooldown-throttled=k322` -`PRED.k328.signal=pr-template-typed-heading-required` -`PRED.k328.canonical-source=CONTRIBUTING.md L48,L64,L81 (template links) + .github/PULL_REQUEST_TEMPLATE/{fix,enhancement,feature}.md L1 (heading text)` -`PRED.k328.k100-restatement=heading must match issue class: bug→## Fix PR, enhancement→## Enhancement PR, feature→## Feature PR` -`PRED.k328.audit-list=[heading-matches-class, closing-keyword-present, changeset-fragment-or-no-changelog-label]` - `PRED.k329.signal=changeset-fragment-canonical-shape` `PRED.k329.canonical-source=CONTRIBUTING.md L196-202 + .changeset/README.md` `PRED.k329.filename=.changeset/--.md` @@ -963,8 +958,6 @@ Dependency posture: this seam OWNS its Windows binary resolution and cmd.exe med `RULESET.PR-FLOW.docker-before-push=before ANY git push of any fix to any PR, run msd-test (docker on the remote, mirrors ubuntu CI) and confirm exit 0. macOS-local node --test is NOT a substitute — many failures are platform-specific (path separators, case sensitivity, locale, fs semantics). Watchdog with Monitor on the output log; never set a sleep/timer and walk away. Source: user feedback 2026-05-16 — "we don't set a timer we actively watch and record results in real time as possible". SUPERSEDED 2026-07-17: 'confirm exit 0' is a false-green trap — piping/backgrounding can report exit 0 on a failed suite; gate on the verdict-line outcome:"passed" for the exact HEAD sha instead. See CLAUDE.md's msd-test rule and the msd-test-is-ref-based-commit-first predicate for the current, correct gating contract.` -`RULESET.PR-FLOW.templates-mandatory=every gh pr create|edit|gh issue create|edit MUST first invoke the gh-templates-first skill and Read (Read tool, not Bash cat — k321 read-tracking) the matching template in .github/. Apply ALL required sections; never write freeform bodies. Repo enforces this via msd-pr-template-policy GitHub Action which flags any non-templated body — the bot allows the PR to stay open only because authors are contributors-or-higher, but the warning is a real complaint that must be cured. Source: user feedback 2026-05-16 (multi-message escalation) — "the whole reason i have that github action is because you fucking blow through and ignore using the templates"` - --- ## Executor failure classification (#3095 / PR #3490) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f37ac6bdf..bcc9fbca6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,95 +29,39 @@ environment validator, daily commands, and troubleshooting. --- -## Types of Contributions +## How Changes Land -MSD accepts three types of contributions. Each type has a different process and a different bar for acceptance. **Read this section before opening anything.** +MSD is maintained by a small team and does not use an issue tracker. Work happens on a feature branch and lands through a pull request against `next` on [git.golem15.com](https://git.golem15.com/golem15/msd-core): -### 🐛 Fix (Bug Report) +1. Branch from `next` (`feat/`, `fix/`, `chore/`, …). +2. Make the change, with tests. Add a `.changeset/` fragment if it is user-facing (see below). +3. Open a PR against `next`. Describe what changed and why, and how you tested it. No issue link or PR template is required. +4. Merge once CI is green and the change is reviewed. -A fix corrects something that is broken, crashes, produces wrong output, or behaves contrary to documented behavior. - -**Process:** -1. Open a [Bug Report issue](https://github.com/open-gsd/gsd-core/issues/new?template=bug_report.yml) — fill it out completely. -2. Wait for a maintainer to confirm it is a bug (label: `confirmed-bug`). For obvious, reproducible bugs this is typically fast. -3. Fix it. Write a test that would have caught the bug. -4. Open a PR using the [Fix PR template](.github/PULL_REQUEST_TEMPLATE/fix.md) — link the confirmed issue. - -**Rejection reasons:** Not reproducible, works-as-designed, duplicate of an existing issue. - ---- - -### ⚡ Enhancement - -An enhancement improves an existing feature — better output, faster execution, cleaner UX, expanded edge-case handling. It does **not** add new commands, new workflows, or new concepts. - -**The bar:** Enhancements must have a scoped written proposal approved by a maintainer before any code is written. A PR for an enhancement will be closed without review if the linked issue does not carry the `approved-enhancement` label. - -**Process:** -1. Open an [Enhancement issue](https://github.com/open-gsd/gsd-core/issues/new?template=enhancement.yml) with the full proposal. The issue template requires: the problem being solved, the concrete benefit, the scope of changes, and alternatives considered. -2. **Wait for maintainer approval.** A maintainer must label the issue `approved-enhancement` before you write a single line of code. Do not open a PR against an unapproved enhancement issue — it will be closed. -3. Write the code. Keep the scope exactly as approved. If scope creep occurs, comment on the issue and get re-approval before continuing. -4. Open a PR using the [Enhancement PR template](.github/PULL_REQUEST_TEMPLATE/enhancement.md) — link the approved issue. - -**Rejection reasons:** Issue not labeled `approved-enhancement`, scope exceeds what was approved, no written proposal, duplicate of existing behavior. - ---- - -### ✨ Feature - -A feature adds something new — a new command, a new workflow, a new concept, a new integration. Features have the highest bar because they add permanent maintenance burden to a solo-developer tool maintained by a small team. - -**The bar:** Features require a complete written specification approved by a maintainer before any code is written. A PR for a feature will be closed without review if the linked issue does not carry the `approved-feature` label. Incomplete specs are closed, not revised by maintainers. - -**Process:** -1. **Discuss first** — check [Discussions](https://github.com/open-gsd/gsd-core/discussions) to see if the idea has been raised. If it has and was declined, don't open a new issue. -2. Open a [Feature Request issue](https://github.com/open-gsd/gsd-core/issues/new?template=feature_request.yml) with the complete spec. The template requires: the solo-developer problem being solved, what is being added, full scope of affected files and systems, user stories, acceptance criteria, and assessment of maintenance burden. -3. **Wait for maintainer approval.** A maintainer must label the issue `approved-feature` before you write a single line of code. Approval is not guaranteed — MSD is intentionally lean and many valid ideas are declined because they conflict with the project's design philosophy. -4. Write the code. Implement exactly the approved spec. Changes to scope require re-approval. -5. Open a PR using the [Feature PR template](.github/PULL_REQUEST_TEMPLATE/feature.md) — link the approved issue. - -**Rejection reasons:** Issue not labeled `approved-feature`, spec is incomplete, scope exceeds what was approved, feature conflicts with MSD's solo-developer focus, maintenance burden too high. +Releases are cut from `next` as `vX.Y.Z` tags; the `stable` branch follows the latest release (see `CLAUDE.md`, "Distribution and releases"). --- ### 📐 Proposing an ADR or PRD -An ADR (Architecture Decision Record) documents a significant architectural decision. A PRD (Product Requirements Document) captures the what and why of a feature before implementation. Both are governed by the same issue-first rule as everything else. +An ADR (Architecture Decision Record) documents a significant architectural decision. A PRD (Product Requirements Document) captures the what and why of a feature before implementation. **Process:** -1. Open an issue of the appropriate type (enhancement for an ADR revisiting an existing area, feature for a new architectural surface, chore for policy/docs decisions). Fill it out completely. -2. **Wait for maintainer approval.** A maintainer must label the issue `approved-enhancement`, `approved-feature`, or confirm the chore before any file is created. -3. The GitHub-assigned issue number becomes your filename prefix. Create the file on a branch named after the issue: - - `docs/adr/-.md` for ADRs - - `docs/prd/-.md` for PRDs - - Branch: `docs/-` -4. Open a PR using the appropriate template and close the issue with `Closes #` in the PR body. +1. Open the PR that adds the file (it can start as just the document) so it gets a number. +2. Use that PR number as the filename prefix: + - `docs/adr/-.md` for ADRs + - `docs/prd/-.md` for PRDs +3. Agree on the decision in the PR before building on it. -**One issue = one ADR-or-PRD = one PR.** Do not batch multiple decisions into one file or one PR. +**One ADR-or-PRD per PR.** Do not batch multiple decisions into one file or one PR. -**Do not compute a "next number" locally.** Any PR that uses the legacy `NNNN-*` sequential pattern for a *new* ADR or PRD will be asked to rename the file to the `-.md` format before merge. - -**Example:** Issue #2264 was opened, approved, and its number became the prefix: `docs/adr/2264-golden-parity-redesign.md`. - -**Rejection reasons:** Issue not approved before file was created, filename uses local-compute sequential number instead of issue#, multiple decisions bundled in one PR, file placed in wrong directory (`docs/adr/` vs `docs/prd/`). +**Do not compute a "next number" locally** — the PR number is unique, a locally guessed sequence is not. Existing files keep the upstream issue numbers they were created with. **This process is for a *new* ADR file.** An accepted ADR is never rewritten from scratch — check `docs/adr/` first for a broader ADR that already owns the area. Amending one is a separate, lighter-weight path: see **[`docs/contributor-standards.md` — "Amending an accepted ADR"](docs/contributor-standards.md#amending-an-accepted-adr)** for the two established patterns (an in-place dated section, or a new ADR that declares `Amends`/gets the reciprocal `Amended by` back-link). --- -## The Issue-First Rule — No Exceptions - -> **No code before approval.** - -For **fixes**: open the issue, confirm it's a bug, then fix it. -For **enhancements**: open the issue, get `approved-enhancement`, then code. -For **features**: open the issue, get `approved-feature`, then code. - -PRs that arrive without a properly-labeled linked issue are closed automatically. This is not a bureaucratic hurdle — it protects you from spending time on work that will be rejected, and it protects maintainers from reviewing code for changes that were never agreed to. - ---- - ## Where Do I Open My PR? (Branching Model) MSD uses two long-lived branches: `main` (production, what's on npm `@latest`) @@ -182,23 +126,15 @@ Contributor requirements (summary): - Read `CONTEXT.md` before naming or refactoring modules/interfaces/seams. - Use `CONTEXT.md` vocabulary consistently in code comments, tests, issue/PR text, and docs for the touched area. - Check relevant ADRs in `docs/adr/` before proposing or implementing architectural changes. -- If a change intentionally revisits an ADR decision, call it out explicitly in the linked issue and PR rationale. +- If a change intentionally revisits an ADR decision, call it out explicitly in the PR rationale. - Do not rewrite maintainer intent in `CONTEXT.md`/ADRs as part of drive-by cleanup; propose focused updates tied to approved scope. - If using an AI assistant, prompt it to read `CONTEXT.md` and the relevant ADRs before writing any code or docs, and verify it used the correct vocabulary before opening the PR. -**Every PR must link to an approved issue.** PRs without a linked issue are closed without review, no exceptions. - -- **No draft PRs** — draft PRs are automatically closed. Only open a PR when it is complete, tested, and ready for review. If your work is not finished, keep it on your local branch until it is. -- **Use the correct PR template** — there are separate templates for [Fix](.github/PULL_REQUEST_TEMPLATE/fix.md), [Enhancement](.github/PULL_REQUEST_TEMPLATE/enhancement.md), and [Feature](.github/PULL_REQUEST_TEMPLATE/feature.md). Using the wrong template or using the default template for a feature is a rejection reason. -- **Link with a closing keyword** — use `Closes #123`, `Fixes #123`, or `Resolves #123` in the PR body. The CI check will fail and the PR will be auto-closed if no valid issue reference is found. - - **Test-only and docs-only follow-up PRs may reference without closing.** If your PR is documentation or regression coverage only — say, a repo-wide guard for a fix that already shipped — and there is no open issue for it to close, use a non-closing reference instead: `Refs #123`. `Ref`, `Refs`, `References`, `Relates to`, `Related to`, and `Follow-up to` are all accepted in that position. Do **not** write a closing keyword against an already-closed issue to satisfy the check; on merge it closes nothing, and it trains readers to treat closing keywords as decorative. - - **Qualifying diff shape:** every changed file must be under `tests/`, under `docs/`, or a root-level `*.md` (`README.md`, `CONTRIBUTING.md`, …). This mirrors the doc-only classification the push gate already uses, and it is deliberately root-only — markdown under a subdirectory (`msd-core/workflows/*.md`, `agents/*.md`, `commands/**/*.md`) is runtime-loaded text, not documentation, so it still requires a closing keyword. `CHANGELOG.md` is excluded too: edit it through a `.changeset/` fragment, never directly. - - This weaker form is accepted **only** for that diff shape. A PR touching anything else still needs a closing keyword, and a PR with no issue reference at all still fails. On a very large PR (more than 100 changed files) the check cannot confirm the diff shape and falls back to requiring a closing keyword. - **One concern per PR** — bug fixes, enhancements, and features must be separate PRs - **No drive-by formatting** — don't reformat code unrelated to your change - **Don't bundle test-fixture updates into `docs:` or unrelated commits** — when a production change makes an existing test assertion stale, the test correction MUST land as its own `test:` (or `fix:`) commit, not bundled into a `docs:` commit that also updates the explanation. The release-sdk hotfix cherry-pick filter routes by commit-subject prefix (`fix:`, `chore:`, `test:`); a test-fixture correction packed under a `docs:` prefix is invisible to the picker and ships a half-state to the hotfix branch — production code changed, test assertion stale. v1.42.3 hit this exact mode (#3621). The fix is upstream: keep the test-fixture commit separate. - **CI must pass** — all configured matrix jobs must be green. Node 24 is the compatibility floor and primary target; Node 26 compatibility must be preserved for code and tests even when a Node 26 CI lane is not yet available. -- **Scope matches the approved issue** — if your PR does more than what the issue describes, the extra changes will be asked to be removed or moved to a new issue +- **Scope matches the PR description** — unrelated changes go in their own PR ## CHANGELOG Entries — Drop a Fragment @@ -244,32 +180,18 @@ node scripts/release-notes/format-github-release-notes.cjs \ Omit `--apply` to print the reformatted body to stdout for review without publishing. -### PR title convention (enforced at open time) +### PR title convention -Because the changelog is built from PR titles, your **PR title** must follow: +Because the GitHub-style release notes are grouped by PR title, start the title with a conventional type: ``` -type(#): short summary +type: short summary ``` -- **Start with the type** — `feat`, `fix`, or any other conventional type - (`chore`, `docs`, `refactor`, …). No leading tags or prefixes: a title like - `[security] fix(config): …` defeats the `^fix` bucket anchor and silently - files the entry under the wrong changelog section. -- **Put the linked issue ref in the scope** — `(#)`. This is what - renders as a link to the issue in the changelog line. `fix(core): …` buckets - correctly but produces a changelog entry with **no issue link**. -- A breaking-change marker is fine: `feat(#42)!: …`. +- **Start with the type** — `feat`, `fix`, or any other conventional type (`chore`, `docs`, `refactor`, …). No leading tags: a title like `[security] fix: …` defeats the `^fix` anchor and files the entry under the wrong section. +- A scope and a breaking-change marker are fine: `fix(installer): …`, `feat!: …`. -Examples: `fix(#1542): roadmap rollback`, `feat(#39): milestone-prefixed phase IDs`, -`enhance(#1549): add PR-title validator`. - -**CI enforcement:** `pr-title-validator.yml` checks the title on open/edit and -fails with the required format if it doesn't conform. It reuses the same matcher -the changelog classifier uses (`scripts/release-notes/conventional-title.cjs`), so a title -that passes the check is guaranteed to bucket and link correctly. Fix a flagged -title by editing it in place — the check re-runs on edit, no need to recreate -the PR. +Nothing enforces this in CI; `scripts/release-notes/conventional-title.cjs` is the matcher the release notes use. ## Documentation Updates — Update the Relevant Docs @@ -347,7 +269,7 @@ shared file, so there is nothing to collide on. 1. **Any unique `id` is legal.** It does not have to be contiguous or maximal — 58, 113 and 131 are already absent, and `6.5`, `27a` and `27b` are live - non-integer ids. **Use your issue number** and you never have to revisit the + non-integer ids. **Use your PR number** and you never have to revisit the choice after a rebase. `--check` rejects a duplicate with an `id_duplicate` violation naming both fragments, so a collision is a loud one-line fix in your own file, not a merge conflict. @@ -373,7 +295,7 @@ shared file, so there is nothing to collide on. rejects it (`body_heading_too_shallow`). **Fork contributors:** there is nothing to coordinate and nothing to chase. Pick -your issue number, add your file, regenerate. If `docs/FEATURES.md` conflicts on +your PR number, add your file, regenerate. If `docs/FEATURES.md` conflicts on a rebase, discard your side and re-run `--write` — it is a derived artifact. **Agents:** no Fleet allocation lease is needed for a feature number any more. @@ -1361,7 +1283,7 @@ Reviewers do not rely solely on CI to verify correctness. Before approving a PR, - Build locally (`npm run build` if applicable) - Run the full test suite locally (`npm test`) - Confirm regression tests exist for bug fixes and that they would fail without the fix -- Validate that the implementation matches what the linked issue described — green CI on the wrong implementation is not an approval signal +- Validate that the implementation matches what the PR description says — green CI on the wrong implementation is not an approval signal **"Tests pass in CI" is not sufficient for merge.** The implementation must correctly solve the problem described in the linked issue. diff --git a/docs/CONTEXT-INDEX.json b/docs/CONTEXT-INDEX.json index 7af4465a8..a875eafe6 100644 --- a/docs/CONTEXT-INDEX.json +++ b/docs/CONTEXT-INDEX.json @@ -1,6 +1,6 @@ { "schemaVersion": 1, - "count": 285, + "count": 280, "classes": { "ARCH": 1, "CI": 2, @@ -13,12 +13,12 @@ "PHASE": 8, "PLANNING": 3, "PR": 2, - "PRED": 68, + "PRED": 64, "PROBE": 12, "PROC": 14, "PROHIB": 10, "RELEASE-NOTES": 31, - "RULESET": 60, + "RULESET": 59, "SEAM": 14, "SESSION": 9, "WAVE": 5, @@ -496,26 +496,6 @@ "klass": "PRED", "value": "cr-ack-vs-real-review" }, - { - "id": "PRED.k328.audit-list", - "klass": "PRED", - "value": "[heading-matches-class, closing-keyword-present, changeset-fragment-or-no-changelog-label]" - }, - { - "id": "PRED.k328.canonical-source", - "klass": "PRED", - "value": "CONTRIBUTING.md L48,L64,L81 (template links) + .github/PULL_REQUEST_TEMPLATE/{fix,enhancement,feature}.md L1 (heading text)" - }, - { - "id": "PRED.k328.k100-restatement", - "klass": "PRED", - "value": "heading must match issue class: bug→## Fix PR, enhancement→## Enhancement PR, feature→## Feature PR" - }, - { - "id": "PRED.k328.signal", - "klass": "PRED", - "value": "pr-template-typed-heading-required" - }, { "id": "PRED.k329.body", "klass": "PRED", @@ -1061,11 +1041,6 @@ "klass": "RULESET", "value": "before ANY git push of any fix to any PR, run msd-test (docker on the remote, mirrors ubuntu CI) and confirm exit 0. macOS-local node --test is NOT a substitute — many failures are platform-specific (path separators, case sensitivity, locale, fs semantics). Watchdog with Monitor on the output log; never set a sleep/timer and walk away. Source: user feedback 2026-05-16 — \"we don't set a timer we actively watch and record results in real time as possible\". SUPERSEDED 2026-07-17: 'confirm exit 0' is a false-green trap — piping/backgrounding can report exit 0 on a failed suite; gate on the verdict-line outcome:\"passed\" for the exact HEAD sha instead. See CLAUDE.md's msd-test rule and the msd-test-is-ref-based-commit-first predicate for the current, correct gating contract." }, - { - "id": "RULESET.PR-FLOW.templates-mandatory", - "klass": "RULESET", - "value": "every gh pr create|edit|gh issue create|edit MUST first invoke the gh-templates-first skill and Read (Read tool, not Bash cat — k321 read-tracking) the matching template in .github/. Apply ALL required sections; never write freeform bodies. Repo enforces this via msd-pr-template-policy GitHub Action which flags any non-templated body — the bot allows the PR to stay open only because authors are contributors-or-higher, but the warning is a real complaint that must be cured. Source: user feedback 2026-05-16 (multi-message escalation) — \"the whole reason i have that github action is because you fucking blow through and ignore using the templates\"" - }, { "id": "RULESET.PR-SCOPE.one-concern-per-pr", "klass": "RULESET", diff --git a/scripts/ci-test-scope.cjs b/scripts/ci-test-scope.cjs index a0c769395..449301154 100644 --- a/scripts/ci-test-scope.cjs +++ b/scripts/ci-test-scope.cjs @@ -17,14 +17,9 @@ const INERT_WORKFLOWS = new Set([ 'stale.yml', 'branch-cleanup.yml', 'branch-naming.yml', - 'auto-label-issues.yml', - 'auto-branch.yml', 'auto-backmerge.yml', - 'close-draft-prs.yml', 'dismiss-unauthorized-pr-approvals.yml', 'pr-target-validator.yml', - 'pr-template-format.yml', - 'require-issue-link.yml', 'changeset-required.yml', 'docs-required.yml', 'discord-changelog.yml', @@ -65,7 +60,6 @@ function isInertCi(filePath) { // Tests shared by both the 'workflow automation' and 'inert CI' rules. const WORKFLOW_LINT_TESTS = [ 'tests/workflow-shell-pinning.test.cjs', - 'tests/pr-template-policy.test.cjs', 'tests/lint-pr-check-project-dir.test.cjs', ]; diff --git a/scripts/lib/macos-conformance-tier.generated.cjs b/scripts/lib/macos-conformance-tier.generated.cjs index 4e5d4873b..4022d167c 100644 --- a/scripts/lib/macos-conformance-tier.generated.cjs +++ b/scripts/lib/macos-conformance-tier.generated.cjs @@ -163,7 +163,6 @@ module.exports = { "tests/quick-research.test.cjs", "tests/reapply-verify-hunks.test.cjs", "tests/refactor-1390-t3-characterization.test.cjs", - "tests/require-issue-link-policy.test.cjs", "tests/response-language-coverage.test.cjs", "tests/retired-artifact-cleanup.test.cjs", "tests/review-build-prompt-optional-sections.test.cjs", diff --git a/scripts/lint-docs-guard-registration.exempt-baseline.cjs b/scripts/lint-docs-guard-registration.exempt-baseline.cjs index bb7a795a9..dcd3c1306 100644 --- a/scripts/lint-docs-guard-registration.exempt-baseline.cjs +++ b/scripts/lint-docs-guard-registration.exempt-baseline.cjs @@ -75,7 +75,6 @@ const DOCS_GUARD_EXEMPT_BASELINE = [ 'precommit-alias-drift-hook.test.cjs', 'removed-but-needed-lint.test.cjs', 'repo-invariants.test.cjs', - 'require-issue-link-policy.test.cjs', 'reviewer-manifest-body.test.cjs', 'reviewer-step-dispatch.test.cjs', 'run-tests-harness.test.cjs', @@ -211,7 +210,6 @@ const DOCS_GUARD_EXEMPT_DOCS_PATHS = { 'docs/README.md', 'docs/getting-started.md', 'docs/msd-new-workspace.md', 'docs/setup.md', 'docs/some-doc.md', ], 'repo-invariants.test.cjs': ['docs/FEATURES.md', 'docs/workflows/README'], - 'require-issue-link-policy.test.cjs': ['docs/-prefixed', 'docs/CONFIGURATION.md', 'docs/a.md', 'docs/b.md', 'docs/guide.md'], 'reviewer-manifest-body.test.cjs': ['docs/how-to/ship-a-reviewer-lane.md'], // #4209: 'docs/spec.md' is a synthetic, never-read fake path proving dispatchReviewerLanes // has no code-review-specific special-casing; 'docs/adr/456-test-rigor-architecture.md' is a diff --git a/scripts/lint-pr-check-project-dir.cjs b/scripts/lint-pr-check-project-dir.cjs index e9a65bfac..850e14863 100644 --- a/scripts/lint-pr-check-project-dir.cjs +++ b/scripts/lint-pr-check-project-dir.cjs @@ -10,13 +10,11 @@ const ROOT = path.join(__dirname, '..'); const DEFAULT_RELATIVE_FILES = [ '.github/workflows/test.yml', - '.github/workflows/pr-template-format.yml', '.github/workflows/changeset-required.yml', 'scripts/lint-command-contract.cjs', 'scripts/lint-skill-deps.cjs', 'scripts/lint-descriptions.cjs', 'scripts/lint-shell-command-projection-drift.cjs', - 'scripts/pr-template-policy.cjs', 'scripts/changeset/lint.cjs', ]; diff --git a/scripts/pr-changed-files.cjs b/scripts/pr-changed-files.cjs deleted file mode 100644 index b38488818..000000000 --- a/scripts/pr-changed-files.cjs +++ /dev/null @@ -1,63 +0,0 @@ -'use strict'; - -/** - * Shared truncation-detection helper for `gh pr view --json files` (#3211). - * - * GitHub's GraphQL `files` connection on a pull request is a paginated - * connection. `gh pr view --json files` requests it with a single page at - * `first: 100` and does NOT paginate through the rest — so any PR touching - * more than 100 files silently returns only the first 100, with no error and - * no indication in the `files` array itself that entries are missing. - * `changedFiles` (a separate, non-paginated scalar field) reports the true - * total. A policy that relaxes a gate based on "every changed file matches - * pattern X" must treat a `files` list it cannot confirm is complete as - * incomplete — i.e. fail closed — rather than silently approving a PR whose - * 101st+ file might violate the policy. - * - * This file lives at the top level of scripts/ rather than in scripts/lib/ - * because scripts/lib/** is enumerated in bin/install.js and ships to users - * on install; this is CI-only tooling with no reason to be installed. - */ - -// GitHub's GraphQL `files` connection is requested at `first: 100` by -// `gh pr view --json files`; `gh` does not paginate this field. -const FILE_LIST_PAGE_LIMIT = 100; - -/** - * Returns true iff `changedFiles` can be trusted as the COMPLETE list of - * files changed in the PR (i.e. it was not silently truncated). - * - * - An empty/non-array list cannot confirm anything about the PR — mirror - * the fail-closed stance `allPathsAreTooling` already takes on an empty - * list in scripts/pr-template-policy.cjs. - * - When the true total is known it is the AUTHORITY, at every list size — - * not just when the list has hit the page cap. The 100-entry page cap is - * only ONE way a list can be short of the truth; a `$GITHUB_OUTPUT` - * heredoc terminated early by an attacker-named file, or a path - * containing a newline, truncates or inflates the list just as - * effectively, and at any length. Comparing against the total catches all - * of them without enumerating the mechanisms. - * - Only when no total is supplied do we fall back to the page-cap - * heuristic: a list below the cap cannot have been truncated BY THE CAP, - * which is the only mechanism a caller without a total can rule out. - */ -function fileListIsComplete(changedFiles, changedFilesTotal) { - if (!Array.isArray(changedFiles) || changedFiles.length === 0) return false; - if (Number.isInteger(changedFilesTotal)) return changedFilesTotal === changedFiles.length; - return changedFiles.length < FILE_LIST_PAGE_LIMIT; -} - -/** - * Parses a newline-delimited env var (as produced by `git diff --name-only` - * or `gh pr view --json files -q '.files[].path'`) into a trimmed, - * empty-line-free array. Returns [] for null/undefined/empty input. - */ -function parseChangedFilesEnv(value) { - if (!value) return []; - return String(value) - .split(/\r?\n/) - .map((line) => line.trim()) - .filter(Boolean); -} - -module.exports = { fileListIsComplete, FILE_LIST_PAGE_LIMIT, parseChangedFilesEnv }; diff --git a/scripts/pr-template-policy.cjs b/scripts/pr-template-policy.cjs deleted file mode 100644 index 59579776a..000000000 --- a/scripts/pr-template-policy.cjs +++ /dev/null @@ -1,278 +0,0 @@ -#!/usr/bin/env node - -const { matchesGlob } = require('path'); -const { fileListIsComplete, parseChangedFilesEnv } = require('./pr-changed-files.cjs'); - -const TRUSTED_AUTHOR_ASSOCIATIONS = new Set([ - 'CONTRIBUTOR', - 'COLLABORATOR', - 'MEMBER', - 'OWNER', -]); - -const DEFAULT_TEMPLATE_MARKERS = [ - 'Wrong template', - 'Every PR must use a typed template', - 'Select the template that matches your PR', -]; - -/** - * Glob patterns for files that are considered CI/tooling/docs scope. - * If ALL changed files in a PR match at least one of these patterns, - * template enforcement is skipped automatically. - */ -const TOOLING_PATH_ALLOWLIST = [ - '.github/**', - 'scripts/**', - 'docs/**', - '*.md', - '.changeset/**', - 'package.json', - 'package-lock.json', - 'pnpm-lock.yaml', - 'yarn.lock', - 'Pipfile', - 'Pipfile.lock', - 'requirements.txt', - 'requirements*.txt', - 'poetry.lock', - 'Gemfile', - 'Gemfile.lock', - 'go.sum', - 'go.mod', -]; - -/** - * Matches an explicit PR-template exemption marker of the form: - * - * - * Capture group 1 is the reason (trimmed). Empty/whitespace-only reasons - * do NOT match because the pattern requires at least one non-whitespace - * character in the captured body. - */ -const EXEMPT_MARKER_REGEX = //; - -const TEMPLATES = [ - { - name: 'fix', - heading: 'Fix PR', - requiredHeadings: [ - 'Fix PR', - 'Linked Issue', - 'What was broken', - 'What this fix does', - 'Testing', - 'Checklist', - ], - }, - { - name: 'enhancement', - heading: 'Enhancement PR', - requiredHeadings: [ - 'Enhancement PR', - 'Linked Issue', - 'What this enhancement improves', - 'Before / After', - 'How it was implemented', - 'Testing', - 'Scope confirmation', - 'Checklist', - ], - }, - { - name: 'feature', - heading: 'Feature PR', - requiredHeadings: [ - 'Feature PR', - 'Linked Issue', - 'Feature summary', - 'What changed', - 'Implementation notes', - 'Spec compliance', - 'Testing', - 'Scope confirmation', - 'Checklist', - ], - }, -]; - -function stripMarkdownDecoration(value) { - return value - .replace(/^\s*#+\s*/, '') - .replace(/\s*#+\s*$/, '') - .replace(/\*\*/g, '') - .trim() - .toLowerCase(); -} - -function extractHeadings(body) { - const headings = new Set(); - for (const line of String(body || '').split(/\r?\n/)) { - if (/^\s*#{1,6}\s+\S/.test(line)) { - headings.add(stripMarkdownDecoration(line)); - } - } - return headings; -} - -function includesDefaultTemplate(body) { - const text = String(body || '').toLowerCase(); - return DEFAULT_TEMPLATE_MARKERS.some((marker) => text.includes(marker.toLowerCase())); -} - -function matchingTemplate(body) { - const headings = extractHeadings(body); - for (const template of TEMPLATES) { - if (!headings.has(stripMarkdownDecoration(template.heading))) continue; - const missingHeadings = template.requiredHeadings.filter((heading) => { - return !headings.has(stripMarkdownDecoration(heading)); - }); - return { - template: template.name, - missingHeadings, - }; - } - return { - template: null, - missingHeadings: [], - }; -} - -/** - * Returns true iff every path in changedFiles matches at least one glob - * pattern in the allowlist. Returns false for an empty file list (no - * files means we cannot confirm it is a tooling-only PR). - */ -function allPathsAreTooling(changedFiles, allowlist, changedFilesTotal) { - if (!Array.isArray(changedFiles) || changedFiles.length === 0) return false; - // #3211: `gh pr view --json files` truncates at 100 entries with no - // in-band signal. A truncated list must not relax enforcement — an unseen - // 101st+ file could fall outside the tooling allowlist entirely. - if (!fileListIsComplete(changedFiles, changedFilesTotal)) return false; - return changedFiles.every((file) => - allowlist.some((pattern) => matchesGlob(file, pattern)), - ); -} - -/** - * Returns true iff the body contains an explicit exemption marker with a - * non-empty (non-whitespace) reason. - */ -function hasExemptMarker(body, regex) { - const match = regex.exec(String(body || '')); - if (!match) return false; - return match[1].trim().length > 0; -} - -function evaluatePrTemplate(body, authorAssociation, changedFiles, changedFilesTotal) { - const association = String(authorAssociation || '').toUpperCase(); - const trusted = TRUSTED_AUTHOR_ASSOCIATIONS.has(association); - const normalizedBody = String(body || '').trim(); - - // --- Carve-out 1: all changed files are in the tooling allowlist --- - if (allPathsAreTooling(changedFiles, TOOLING_PATH_ALLOWLIST, changedFilesTotal)) { - return { - valid: true, - action: 'pass', - trusted, - authorAssociation: association || 'UNKNOWN', - template: null, - reason: null, - missingHeadings: [], - skipped: 'tooling-paths', - }; - } - - // --- Carve-out 2: explicit exemption marker in the PR body --- - if (hasExemptMarker(normalizedBody, EXEMPT_MARKER_REGEX)) { - return { - valid: true, - action: 'pass', - trusted, - authorAssociation: association || 'UNKNOWN', - template: null, - reason: null, - missingHeadings: [], - skipped: 'exempt-marker', - }; - } - - let valid = true; - let reason = 'PR body uses a typed pull request template.'; - let template = null; - let missingHeadings = []; - - if (!normalizedBody) { - valid = false; - reason = 'PR body is empty; a typed pull request template is required.'; - } else { - const match = matchingTemplate(normalizedBody); - template = match.template; - missingHeadings = match.missingHeadings; - if (template && missingHeadings.length === 0) { - valid = true; - } else if (template && missingHeadings.length > 0) { - valid = false; - reason = `PR body appears to use the ${template} template but is missing required headings.`; - } else if (includesDefaultTemplate(normalizedBody)) { - valid = false; - reason = 'PR body still contains the default wrong-template guidance.'; - } else { - valid = false; - reason = 'PR body does not match the fix, enhancement, or feature template.'; - } - } - - let action = 'pass'; - if (!valid) { - action = trusted ? 'warn' : 'close'; - } - - return { - valid, - action, - trusted, - authorAssociation: association || 'UNKNOWN', - template, - reason, - missingHeadings, - }; -} - -function main() { - // Preserve the existing distinction between "unset" (undefined) and - // "set but empty" ([]) — allPathsAreTooling treats them differently. - const changedFiles = process.env.CHANGED_FILES - ? parseChangedFilesEnv(process.env.CHANGED_FILES) - : undefined; - const parsedTotal = Number.parseInt(process.env.CHANGED_FILES_TOTAL, 10); - const changedFilesTotal = Number.isNaN(parsedTotal) ? undefined : parsedTotal; - const result = evaluatePrTemplate( - process.env.PR_BODY || '', - process.env.AUTHOR_ASSOCIATION || '', - changedFiles, - changedFilesTotal, - ); - process.stdout.write(`${JSON.stringify(result)}\n`); - if (process.env.GITHUB_OUTPUT) { - const fs = require('fs'); - fs.appendFileSync(process.env.GITHUB_OUTPUT, `result=${JSON.stringify(result)}\n`); - fs.appendFileSync(process.env.GITHUB_OUTPUT, `action=${result.action}\n`); - fs.appendFileSync(process.env.GITHUB_OUTPUT, `valid=${result.valid ? 'true' : 'false'}\n`); - } -} - -if (require.main === module) { - main(); -} - -module.exports = { - evaluatePrTemplate, - extractHeadings, - includesDefaultTemplate, - matchingTemplate, - allPathsAreTooling, - hasExemptMarker, - TOOLING_PATH_ALLOWLIST, - EXEMPT_MARKER_REGEX, -}; diff --git a/scripts/release-notes/conventional-title.cjs b/scripts/release-notes/conventional-title.cjs index 0625fce66..3a88e2df0 100644 --- a/scripts/release-notes/conventional-title.cjs +++ b/scripts/release-notes/conventional-title.cjs @@ -3,15 +3,11 @@ /** * Single source of truth for conventional-commit PR-title parsing. * - * Consumed by BOTH: - * - the release-notes changelog classifier - * (scripts/release-notes/format-github-release-notes.cjs), and - * - the PR-title CI gate (.github/workflows/pr-title-validator.yml, via - * evaluatePrTitle). - * - * Keeping one matcher here is the point of #1549: a forked copy of the regex - * would let the gate accept a title that the changelog then mis-buckets. Both - * the bucket anchors and the gate must read the title the same way. + * Consumed by the release-notes changelog classifier + * (scripts/release-notes/format-github-release-notes.cjs). `evaluatePrTitle` + * backed the upstream PR-title CI gate (#1549), which MSD does not run; it is + * kept so a title can still be checked against the same matcher the + * changelog buckets on. */ // Bucket anchors. The leading `^` is load-bearing: the changelog buckets on the diff --git a/scripts/require-issue-link-policy.cjs b/scripts/require-issue-link-policy.cjs deleted file mode 100644 index 8c73099bb..000000000 --- a/scripts/require-issue-link-policy.cjs +++ /dev/null @@ -1,207 +0,0 @@ -#!/usr/bin/env node -'use strict'; - -/** - * Require-issue-link policy (#3211, preserving #1389). - * - * Replaces the shell-only `grep -qiE '(closes|fixes|resolves)\s+#[0-9]+'` - * step with a pure, testable `evaluateIssueLink` verdict function so a PR - * that only REFERENCES an issue (rather than closing it — e.g. a follow-up - * regression-coverage PR) is not forced to fabricate a closing keyword, while - * two pre-existing constraints are preserved exactly: - * - * 1. (#1389, anti-forgery) The backmerge exemption only fires when the - * head ref carries the backmerge prefix AND the PR is same-repo (not a - * fork) — dropping the `sameRepo` conjunct would let a fork forge a - * branch name to bypass the issue-link requirement entirely. - * 2. (#3211, truncation) `gh pr view --json files` truncates the file list - * at 100 entries with no in-band signal that it did so (see - * scripts/pr-changed-files.cjs). The reference-only carve-out below - * only applies when every changed file is a test/doc file, so a PR - * whose file list may be truncated must fail closed rather than let an - * unseen 101st+ file (which could touch `src/`) slip through. - * - * Tests assert on the typed ISSUE_LINK_REASON enum, never on free text. - */ - -const { fileListIsComplete, parseChangedFilesEnv } = require('./pr-changed-files.cjs'); -const { runMain } = require('./lib/cli-exit.cjs'); - -const ISSUE_LINK_REASON = Object.freeze({ - OK_CLOSING_KEYWORD: 'ok_closing_keyword', - OK_BACKMERGE_EXEMPT: 'ok_backmerge_exempt', - OK_DEPENDABOT_EXEMPT: 'ok_dependabot_exempt', - OK_FOLLOWUP_REFERENCE: 'ok_followup_reference', - FAIL_NO_ISSUE_REFERENCE: 'fail_no_issue_reference', - FAIL_REFERENCE_NEEDS_CLOSING: 'fail_reference_needs_closing', - FAIL_FILE_LIST_INCOMPLETE: 'fail_file_list_incomplete', -}); - -// #1389: backmerge PRs are opened by CI against `next`, never by a human or a -// fork, so they are exempt from the issue-link requirement outright — but -// ONLY when combined with `sameRepo === true` below (see header comment). -const BACKMERGE_BRANCH_PREFIX = 'chore/backmerge-main-to-next-'; - -// #4196: Dependabot has no mechanism to link a PR it opens to a repo issue — -// its alerts live in the Security tab, not as issues, so there is nothing -// for it to reference. `pr.user.login` is authenticated by GitHub (not -// forgeable by a crafted title/branch), so this is safe without a sameRepo -// conjunct: no external actor can make GitHub report this login for a PR -// they opened. -const DEPENDABOT_LOGIN = 'dependabot[bot]'; - -// A follow-up-only PR (references an issue without closing it) is only -// allowed to skip the closing keyword when every changed file is a test or -// doc file — i.e. it cannot be the PR that actually implements the fix. -const EXEMPT_PATH_PREFIXES = ['tests/', 'docs/']; - -// Root-level markdown is documentation — this mirrors the repo's own doc-only -// classifier (.claude/hooks/pre-pr-gate.sh:111), whose `[^/]+\.md` anchor is -// deliberately root-only so runtime-loaded text under a subdirectory -// (msd-core/workflows/*.md, agents/*.md, commands/**/*.md) stays gated. -// CHANGELOG.md is excluded: scripts/changeset/lint.cjs classes a direct edit to -// it as user-facing precisely to close a bypass, so it must not ride in on the -// docs carve-out either. -const EXCLUDED_ROOT_DOCS = new Set(['CHANGELOG.md']); - -// Case-insensitive lookup set derived from EXCLUDED_ROOT_DOCS. The exclusion -// check below is case-insensitive because the `.md` extension test above it -// already is (`/\.md$/i`) — `changelog.md` or `CHANGELOG.MD` would otherwise -// slip past the exclusion while still passing the extension test. In -// practice this is defense in depth rather than a live bypass: the GitHub -// API always reports the real path with its actual, fixed casing (the -// filesystem is case-sensitive on the runners this executes on), so a PR -// cannot rename CHANGELOG.md to bypass the check by casing alone. -const EXCLUDED_ROOT_DOCS_UPPER = new Set( - Array.from(EXCLUDED_ROOT_DOCS, (name) => name.toUpperCase()), -); - -function isRootLevelDoc(normalizedPath) { - if (!normalizedPath) return false; - if (normalizedPath.includes('/')) return false; - if (!/\.md$/i.test(normalizedPath)) return false; - if (EXCLUDED_ROOT_DOCS_UPPER.has(normalizedPath.toUpperCase())) return false; - return true; -} - -// Mirrors the shipped `grep -qiE '(closes|fixes|resolves)\s+#[0-9]+'` exactly: -// no additional keywords, no `\b` anchors (the shell grep it replaces has -// none either) — see the corpus-parity test in -// tests/require-issue-link-policy.test.cjs. -const CLOSING_KEYWORD_REGEX = /(?:closes|fixes|resolves)\s+#[0-9]+/i; - -// Accepts a soft "this PR relates to #N" reference without claiming to close -// it. The leading `\b` prevents matching inside a longer word (e.g. `xref#1`, -// `prefs #1`) because there is no word-boundary transition between the -// preceding word character and the start of the alternative — verified -// explicitly for `preferences #1` / `unreferenced #1` in the test suite. -const FOLLOWUP_REFERENCE_REGEX = /\b(?:refs?|references?|relates\s+to|related\s+to|follow[-\s]?up\s+to)\s+#[0-9]+/i; - -function hasClosingKeyword(body) { - return CLOSING_KEYWORD_REGEX.test(String(body || '')); -} - -function hasFollowUpReference(body) { - return FOLLOWUP_REFERENCE_REGEX.test(String(body || '')); -} - -// Path separator normalization — unconditional, per repo convention (see -// CLAUDE.md "Path-sep normalization"), so a Windows-style diff entry like -// `tests\windows\a.test.cjs` is still recognized as tests/-prefixed. -function normalizePath(p) { - return String(p).replace(/\\/g, '/'); -} - -/** - * Returns true iff every changed file qualifies as a test/doc file, i.e. each - * one is one of: - * 1. under one of EXEMPT_PATH_PREFIXES (`tests/`, `docs/`) — the trailing - * slash on each prefix makes this directory-boundary aware, so - * lookalikes like `tests-e2e/`, `testsuite/`, or `docsite/` are - * correctly rejected (they are not `tests/` or `docs/`); or - * 2. a root-level markdown file (no `/`, `.md` extension, case-insensitive) - * that is not in EXCLUDED_ROOT_DOCS — see isRootLevelDoc. - */ -function allPathsAreTestsOrDocs(changedFiles) { - if (!Array.isArray(changedFiles) || changedFiles.length === 0) return false; - return changedFiles.every((file) => { - const normalized = normalizePath(file); - if (EXEMPT_PATH_PREFIXES.some((prefix) => normalized.startsWith(prefix))) return true; - return isRootLevelDoc(normalized); - }); -} - -function evaluateIssueLink({ prBody, headRef, sameRepo, authorLogin, changedFiles, changedFilesTotal }) { - if (authorLogin === DEPENDABOT_LOGIN) { - return { ok: true, reason: ISSUE_LINK_REASON.OK_DEPENDABOT_EXEMPT }; - } - - if (hasClosingKeyword(prBody)) { - return { ok: true, reason: ISSUE_LINK_REASON.OK_CLOSING_KEYWORD }; - } - - // #1389: `sameRepo === true` is required, not just the branch-name prefix — - // a fork PR could otherwise name its branch to forge this exemption. - if (String(headRef || '').startsWith(BACKMERGE_BRANCH_PREFIX) && sameRepo === true) { - return { ok: true, reason: ISSUE_LINK_REASON.OK_BACKMERGE_EXEMPT }; - } - - if (!hasFollowUpReference(prBody)) { - return { ok: false, reason: ISSUE_LINK_REASON.FAIL_NO_ISSUE_REFERENCE }; - } - - // #3211: a truncated file list cannot be trusted to prove "tests/docs - // only" — fail closed rather than risk approving on an unseen file. - if (!fileListIsComplete(changedFiles, changedFilesTotal)) { - return { ok: false, reason: ISSUE_LINK_REASON.FAIL_FILE_LIST_INCOMPLETE }; - } - - if (allPathsAreTestsOrDocs(changedFiles)) { - return { ok: true, reason: ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE }; - } - - return { ok: false, reason: ISSUE_LINK_REASON.FAIL_REFERENCE_NEEDS_CLOSING }; -} - -function main() { - const changedFiles = parseChangedFilesEnv(process.env.CHANGED_FILES); - const parsedTotal = Number.parseInt(process.env.CHANGED_FILES_TOTAL, 10); - const changedFilesTotal = Number.isNaN(parsedTotal) ? undefined : parsedTotal; - - const result = evaluateIssueLink({ - prBody: process.env.PR_BODY || '', - headRef: process.env.HEAD_REF || '', - sameRepo: process.env.SAME_REPO === 'true', - authorLogin: process.env.PR_AUTHOR_LOGIN || '', - changedFiles, - changedFilesTotal, - }); - - process.stdout.write(`${JSON.stringify(result)}\n`); - if (process.env.GITHUB_OUTPUT) { - const fs = require('node:fs'); - fs.appendFileSync(process.env.GITHUB_OUTPUT, `ok=${result.ok ? 'true' : 'false'}\n`); - fs.appendFileSync(process.env.GITHUB_OUTPUT, `reason=${result.reason}\n`); - fs.appendFileSync(process.env.GITHUB_OUTPUT, `result=${JSON.stringify(result)}\n`); - } - - return result.ok ? 0 : 1; -} - -if (require.main === module) runMain(main); - -module.exports = { - ISSUE_LINK_REASON, - BACKMERGE_BRANCH_PREFIX, - DEPENDABOT_LOGIN, - EXEMPT_PATH_PREFIXES, - EXCLUDED_ROOT_DOCS, - CLOSING_KEYWORD_REGEX, - FOLLOWUP_REFERENCE_REGEX, - hasClosingKeyword, - hasFollowUpReference, - normalizePath, - isRootLevelDoc, - allPathsAreTestsOrDocs, - evaluateIssueLink, -}; diff --git a/tests/ci-pr-mergeability.test.cjs b/tests/ci-pr-mergeability.test.cjs index e62604fb7..c87971fca 100644 --- a/tests/ci-pr-mergeability.test.cjs +++ b/tests/ci-pr-mergeability.test.cjs @@ -667,12 +667,7 @@ const GATED = Object.freeze({ /** Lanes that must keep running on a conflicted PR — security and policy. */ const NEVER_GATED = Object.freeze([ - 'auto-close-unsolicited-prs.yml', - 'close-draft-prs.yml', 'pr-target-validator.yml', - 'pr-title-validator.yml', - 'pr-template-format.yml', - 'require-issue-link.yml', 'dismiss-unauthorized-pr-approvals.yml', ]); diff --git a/tests/ci-test-scope.test.cjs b/tests/ci-test-scope.test.cjs index 17f692894..833996432 100644 --- a/tests/ci-test-scope.test.cjs +++ b/tests/ci-test-scope.test.cjs @@ -392,10 +392,8 @@ describe('INERT_WORKFLOWS allowlist integrity guard', () => { // Canonical inert workflow list — reused by both tests below. const knownInert = [ - 'stale.yml', 'branch-cleanup.yml', 'branch-naming.yml', 'auto-label-issues.yml', - 'auto-branch.yml', 'auto-backmerge.yml', 'close-draft-prs.yml', - 'dismiss-unauthorized-pr-approvals.yml', 'pr-target-validator.yml', - 'pr-template-format.yml', 'require-issue-link.yml', 'changeset-required.yml', + 'stale.yml', 'branch-cleanup.yml', 'branch-naming.yml', 'auto-backmerge.yml', + 'dismiss-unauthorized-pr-approvals.yml', 'pr-target-validator.yml', 'changeset-required.yml', 'docs-required.yml', 'discord-changelog.yml', ]; diff --git a/tests/gemini-runtime-removed.test.cjs b/tests/gemini-runtime-removed.test.cjs index f9c80c7b5..120a8935c 100644 --- a/tests/gemini-runtime-removed.test.cjs +++ b/tests/gemini-runtime-removed.test.cjs @@ -462,50 +462,6 @@ describe('#4709 no shipped surface mints a retired runtime id', () => { } }); - test('PR template runtime checklists name only supported runtimes', () => { - // #1928's follow-up dropped Gemini CLI from .github/ISSUE_TEMPLATE/*.yml but missed the PR - // templates, which kept offering it under "Runtimes tested" -- a contributor-facing surface - // still advertising a retired runtime two releases later. Labels here are DISPLAY names - // ("Claude Code", not "claude"), so they are checked against the label table, not the id set. - const labels = new Set( - Object.keys(registry.runtimes).map((id) => getRuntimeLabel(id)), - ); - // Non-runtime checklist entries that legitimately appear in the same list. - const NON_RUNTIME = /^(Other:|N\/A\b)/; - const offenders = []; - - const templateDir = path.join(ROOT, '.github', 'PULL_REQUEST_TEMPLATE'); - for (const name of fs.readdirSync(templateDir).filter((f) => f.endsWith('.md'))) { - const file = path.join(templateDir, name); - const fileLines = fs.readFileSync(file, 'utf8').split(/\r?\n/); - let inRuntimeSection = false; - fileLines.forEach((line, i) => { - if (/^#+\s*Runtimes tested/i.test(line)) { - inRuntimeSection = true; - return; - } - // The section ends at the next heading or horizontal rule. - if (inRuntimeSection && /^(#+\s|---\s*$)/.test(line)) { - inRuntimeSection = false; - return; - } - if (!inRuntimeSection) return; - const m = /^\s*-\s*\[\s*\]\s*(.+?)\s*$/.exec(line); - if (!m) return; - const label = m[1]; - if (NON_RUNTIME.test(label)) return; - if (!labels.has(label)) { - offenders.push(`.github/PULL_REQUEST_TEMPLATE/${name}:${i + 1} offers "${label}"`); - } - }); - } - - assert.deepStrictEqual(offenders, [], - 'a PR template asks contributors which runtime they tested and lists one MSD does not ' - + 'support. Labels must match the runtime label table (src/runtime-name-policy.cts ' - + `RUNTIME_LABELS), so a retired runtime cannot linger here. Offenders:\n ${offenders.join('\n ')}`); - }); - /** * #4709 Phase 3 — the Gemini CLI reviewer lane is retired. * diff --git a/tests/pr-template-policy.test.cjs b/tests/pr-template-policy.test.cjs deleted file mode 100644 index d637fd4af..000000000 --- a/tests/pr-template-policy.test.cjs +++ /dev/null @@ -1,333 +0,0 @@ -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); - -const { evaluatePrTemplate, allPathsAreTooling, hasExemptMarker, TOOLING_PATH_ALLOWLIST, EXEMPT_MARKER_REGEX } = require('../scripts/pr-template-policy.cjs'); -const { FILE_LIST_PAGE_LIMIT } = require('../scripts/pr-changed-files.cjs'); - -const fixBody = [ - '## Fix PR', - '', - '## Linked Issue', - 'Fixes #123', - '', - '## What was broken', - 'The thing was broken.', - '', - '## What this fix does', - 'The thing now works.', - '', - '## Root cause', - 'A missing guard.', - '', - '## Testing', - 'node --test tests/example.test.cjs', - '', - '## Checklist', - '- [x] Issue linked above with `Fixes #NNN`', -].join('\n'); - -const enhancementBody = [ - '## Enhancement PR', - '', - '## Linked Issue', - 'Closes #123', - '', - '## What this enhancement improves', - 'Existing output.', - '', - '## Before / After', - '**Before:** noisy', - '**After:** clear', - '', - '## How it was implemented', - 'Small refactor.', - '', - '## Testing', - 'node --test tests/example.test.cjs', - '', - '## Scope confirmation', - '- [x] Matches approved issue.', - '', - '## Checklist', - '- [x] Tests pass', -].join('\n'); - -const featureBody = [ - '## Feature PR', - '', - '## Linked Issue', - 'Closes #123', - '', - '## Feature summary', - 'Adds a new thing.', - '', - '## What changed', - '### New files', - 'None.', - '### Modified files', - 'One file.', - '', - '## Implementation notes', - 'Implemented as approved.', - '', - '## Spec compliance', - '- [x] Criterion met', - '', - '## Testing', - 'node --test tests/example.test.cjs', - '', - '## Scope confirmation', - '- [x] Exact scope.', - '', - '## Checklist', - '- [x] Tests pass', -].join('\n'); - -describe('pr-template-policy carve-out', () => { - // A. Path-scope auto-skip — CI-only PR is accepted - test('auto-skips enforcement for CI-only changed files', () => { - const result = evaluatePrTemplate('This is a CI change with no template.', 'NONE', ['.github/workflows/test.yml']); - assert.equal(result.valid, true); - assert.equal(result.action, 'pass'); - assert.equal(result.skipped, 'tooling-paths'); - }); - - // B. Path-scope auto-skip — docs-only PR is accepted - test('auto-skips enforcement for docs-only changed files', () => { - const result = evaluatePrTemplate('Updated documentation.', 'NONE', ['docs/CONFIGURATION.md', 'README.md']); - assert.equal(result.valid, true); - assert.equal(result.action, 'pass'); - assert.equal(result.skipped, 'tooling-paths'); - }); - - // C. Path-scope auto-skip — dependency-bump PR is accepted - test('auto-skips enforcement for dependency-bump changed files', () => { - const result = evaluatePrTemplate('Bump deps.', 'NONE', ['package.json', 'package-lock.json']); - assert.equal(result.valid, true); - assert.equal(result.action, 'pass'); - assert.equal(result.skipped, 'tooling-paths'); - }); - - // D. Path-scope auto-skip is path-strict — mixed PR still enforced - test('does NOT skip enforcement when any changed file is outside the tooling allowlist', () => { - const result = evaluatePrTemplate('Mixed change.', 'NONE', ['.github/workflows/test.yml', 'src/feature.ts']); - assert.equal(result.valid, false); - assert.equal(result.action, 'close'); - }); - - // E. Explicit marker accepted (non-empty reason) - test('auto-skips enforcement when PR body contains a valid exempt marker with a reason', () => { - const body = '\n\nThis removes the Node 26 CI lane.'; - const result = evaluatePrTemplate(body, 'NONE', ['src/anything.ts']); - assert.equal(result.valid, true); - assert.equal(result.action, 'pass'); - assert.equal(result.skipped, 'exempt-marker'); - }); - - // F. Explicit marker requires non-empty reason - test('does NOT skip enforcement when exempt marker has an empty reason', () => { - const bodyEmpty = '\n\nSome description.'; - const bodyNoReason = '\n\nSome description.'; - const r1 = evaluatePrTemplate(bodyEmpty, 'NONE', ['src/anything.ts']); - const r2 = evaluatePrTemplate(bodyNoReason, 'NONE', ['src/anything.ts']); - assert.equal(r1.valid, false); - assert.equal(r2.valid, false); - }); - - // G. Regression — DEFAULT_TEMPLATE_MARKERS still rejected - test('regression: DEFAULT_TEMPLATE_MARKERS body is still rejected when no carve-out applies', () => { - const body = 'Wrong template — please use a typed template.\n\nEvery PR must use a typed template.'; - const result = evaluatePrTemplate(body, 'NONE', ['src/feature.ts']); - assert.equal(result.valid, false); - assert.match(result.reason, /default wrong-template guidance/); - }); - - // H. Regression — fix template still accepted - test('regression: fix template PR body is still accepted', () => { - const result = evaluatePrTemplate(fixBody, 'NONE', ['src/feature.ts']); - assert.equal(result.valid, true); - assert.equal(result.action, 'pass'); - assert.equal(result.template, 'fix'); - }); -}); - -describe('pr-template-policy helper: allPathsAreTooling', () => { - test('returns true when all paths match the allowlist', () => { - assert.equal(allPathsAreTooling(['.github/workflows/ci.yml'], TOOLING_PATH_ALLOWLIST), true); - assert.equal(allPathsAreTooling(['package.json', 'package-lock.json'], TOOLING_PATH_ALLOWLIST), true); - assert.equal(allPathsAreTooling(['README.md'], TOOLING_PATH_ALLOWLIST), true); - }); - - test('returns false when any path does not match the allowlist', () => { - assert.equal(allPathsAreTooling(['.github/workflows/ci.yml', 'src/index.ts'], TOOLING_PATH_ALLOWLIST), false); - }); - - test('returns false for an empty file list', () => { - assert.equal(allPathsAreTooling([], TOOLING_PATH_ALLOWLIST), false); - }); -}); - -describe('pr-template-policy helper: hasExemptMarker', () => { - test('matches a marker with a non-empty reason', () => { - assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), true); - assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), true); - assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), true); - }); - - test('does not match a marker with empty or whitespace-only reason', () => { - assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), false); - assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), false); - }); - - test('does not match a marker with no pr-template-exempt keyword', () => { - assert.equal(hasExemptMarker('', EXEMPT_MARKER_REGEX), false); - assert.equal(hasExemptMarker('some random text', EXEMPT_MARKER_REGEX), false); - }); -}); - -describe('pr-template-policy', () => { - test('passes PR bodies that use the fix template', () => { - const result = evaluatePrTemplate(fixBody, 'NONE'); - - assert.equal(result.valid, true); - assert.equal(result.action, 'pass'); - assert.equal(result.template, 'fix'); - }); - - test('passes PR bodies that use the enhancement template', () => { - const result = evaluatePrTemplate(enhancementBody, 'FIRST_TIMER'); - - assert.equal(result.valid, true); - assert.equal(result.action, 'pass'); - assert.equal(result.template, 'enhancement'); - }); - - test('does not flag default-template marker phrase inside a valid enhancement template', () => { - const body = enhancementBody.replace( - '## Linked Issue', - [ - '> **Using the wrong template?**', - '> - Bug fix: use [fix.md](?template=fix.md)', - '> - New feature: use [feature.md](?template=feature.md)', - '', - '## Linked Issue', - ].join('\n'), - ); - const result = evaluatePrTemplate(body, 'COLLABORATOR'); - - assert.equal(result.valid, true); - assert.equal(result.action, 'pass'); - assert.equal(result.template, 'enhancement'); - }); - - test('passes PR bodies that use the feature template', () => { - const result = evaluatePrTemplate(featureBody, 'FIRST_TIME_CONTRIBUTOR'); - - assert.equal(result.valid, true); - assert.equal(result.action, 'pass'); - assert.equal(result.template, 'feature'); - }); - - test('closes first-time PRs that keep the default template', () => { - const result = evaluatePrTemplate([ - '## Wrong template - please use the correct one for your PR type', - '', - 'Every PR must use a typed template.', - ].join('\n'), 'FIRST_TIMER'); - - assert.equal(result.valid, false); - assert.equal(result.action, 'close'); - assert.equal(result.trusted, false); - assert.match(result.reason, /default wrong-template guidance/); - }); - - test('warns contributors instead of closing when the template is missing', () => { - const result = evaluatePrTemplate('This is a free-form PR body.', 'CONTRIBUTOR'); - - assert.equal(result.valid, false); - assert.equal(result.action, 'warn'); - assert.equal(result.trusted, true); - }); - - test('warns collaborators, members, and owners instead of closing', () => { - for (const association of ['COLLABORATOR', 'MEMBER', 'OWNER']) { - const result = evaluatePrTemplate('This is a free-form PR body.', association); - - assert.equal(result.valid, false); - assert.equal(result.action, 'warn'); - assert.equal(result.trusted, true); - } - }); - - test('does not close for an unfilled issue slug when the template is present', () => { - const body = fixBody.replace('Fixes #123', 'Fixes #'); - const result = evaluatePrTemplate(body, 'FIRST_TIMER'); - - assert.equal(result.valid, true); - assert.equal(result.action, 'pass'); - assert.equal(result.template, 'fix'); - }); - - test('closes first-time PRs that remove required template sections', () => { - const result = evaluatePrTemplate(fixBody.replace('## What was broken', '## Background'), 'NONE'); - - assert.equal(result.valid, false); - assert.equal(result.action, 'close'); - assert.deepEqual(result.missingHeadings, ['What was broken']); - }); - - test('closes first-time PRs with empty body', () => { - const result = evaluatePrTemplate('', 'FIRST_TIMER'); - - assert.equal(result.valid, false); - assert.equal(result.action, 'close'); - assert.match(result.reason, /PR body is empty; a typed pull request template is required\./); - }); - - test('warns trusted contributors with empty body', () => { - const result = evaluatePrTemplate('', 'CONTRIBUTOR'); - - assert.equal(result.valid, false); - assert.equal(result.action, 'warn'); - assert.equal(result.trusted, true); - assert.match(result.reason, /PR body is empty; a typed pull request template is required\./); - }); -}); - -// `gh pr view --json files` returns at most 100 paths and does not -// paginate, while the PR's `changedFiles` field reports the true total. -// The tooling-paths carve-out RELAXES template enforcement, so trusting a -// possibly-truncated list would let a >100-file PR skip enforcement on the -// strength of its first 100 (all-tooling) paths. Verified live: PR -// golem15/msd-core#3202 returns 100 paths for 118 changed files. -describe('pr-template-policy carve-out — truncated file lists (#3211)', () => { - const toolingPaths = (n) => Array.from({ length: n }, (_, i) => `docs/generated-${i}.md`); - - // A. Forgery vector — a truncated 100-of-118 list must not be trusted - test('a truncated 100-of-118 tooling file list does not skip enforcement', () => { - const result = evaluatePrTemplate('no template here', 'NONE', toolingPaths(FILE_LIST_PAGE_LIMIT), 118); - assert.equal(result.skipped, undefined); - // author association 'NONE' is untrusted, so enforcement closes the PR. - assert.equal(result.action, 'close'); - }); - - // B. A 100-file tooling list whose total agrees is genuinely complete - test('a 100-file tooling list whose total agrees still skips enforcement', () => { - const result = evaluatePrTemplate('no template here', 'NONE', toolingPaths(FILE_LIST_PAGE_LIMIT), 100); - assert.equal(result.skipped, 'tooling-paths'); - assert.equal(result.valid, true); - }); - - // C. A 100-file tooling list with no corroborating total fails closed - test('a 100-file tooling list with no corroborating total fails closed', () => { - const result = evaluatePrTemplate('no template here', 'NONE', toolingPaths(FILE_LIST_PAGE_LIMIT), undefined); - assert.equal(result.skipped, undefined); - }); - - // D. Back-compat boundary — every existing caller passes no total, and - // lists shorter than the page cap cannot have been truncated. - test('a 99-file tooling list is trusted without a total (below the page cap)', () => { - const result = evaluatePrTemplate('no template here', 'NONE', toolingPaths(FILE_LIST_PAGE_LIMIT - 1), undefined); - assert.equal(result.skipped, 'tooling-paths'); - }); -}); diff --git a/tests/require-issue-link-policy.property.test.cjs b/tests/require-issue-link-policy.property.test.cjs deleted file mode 100644 index 906d22b6d..000000000 --- a/tests/require-issue-link-policy.property.test.cjs +++ /dev/null @@ -1,126 +0,0 @@ -'use strict'; - -/** - * Property-based tests for scripts/require-issue-link-policy.cjs (#3211). - * - * See CLAUDE.md "Property-Based Testing": parsers/budget-limit/bijective - * contracts require at least one fast-check property test. This module has - * three such contracts: the closing-keyword parse, the tests/docs-only - * carve-out, and the truncation-detection fail-closed guarantee. - */ - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); -const fc = require('fast-check'); - -const { ISSUE_LINK_REASON, evaluateIssueLink } = require('../scripts/require-issue-link-policy.cjs'); - -describe('evaluateIssueLink — properties', () => { - test('P1: any body containing a closing keyword yields OK_CLOSING_KEYWORD', () => { - fc.assert( - fc.property( - fc.string(), - fc.string(), - fc.integer({ min: 1, max: 99999 }), - (pre, post, n) => { - // Guard: a leading/trailing newline stops `pre`/`post` from gluing - // onto the "Closes #" token and changing what CLOSING_KEYWORD_REGEX - // sees — the regex has no word-boundary anchors, so word characters - // immediately adjacent to "Closes" are irrelevant to it either way, - // but the newline keeps the constructed body unambiguous to read. - const body = `${pre}\nCloses #${n}\n${post}`; - const result = evaluateIssueLink({ - prBody: body, - headRef: 'fix/1-something', - sameRepo: false, - changedFiles: ['src/init.cts'], - changedFilesTotal: 1, - }); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_CLOSING_KEYWORD); - }, - ), - { numRuns: 200 }, - ); - }); - - test('P2: a non-exempt path in the diff never yields OK_FOLLOWUP_REFERENCE', () => { - fc.assert( - fc.property( - fc.array( - fc.oneof( - fc.constantFrom('tests/a.test.cjs', 'docs/guide.md'), - fc.string({ minLength: 1, maxLength: 20 }).map((s) => `src/${s}.cts`), - ), - { minLength: 1, maxLength: 8 }, - ), - (paths) => { - // At least one path must actually be non-exempt for the property - // to be meaningful; skip runs where fc happened to draw only - // tests/docs paths. The exempt set is tests/, docs/, and root-level - // markdown (see EXEMPT_PATH_PREFIXES / isRootLevelDoc), so this - // filter names the non-exempt generator (`src/*.cts`) directly - // rather than re-deriving the exempt predicate — re-deriving it - // here would silently go stale the next time the exempt set grows. - fc.pre(paths.some((p) => p.startsWith('src/'))); - const result = evaluateIssueLink({ - prBody: 'Refs #1', - headRef: 'fix/1-something', - sameRepo: false, - changedFiles: paths, - changedFilesTotal: paths.length, - }); - assert.notStrictEqual(result.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - }, - ), - { numRuns: 200 }, - ); - }); - - test('P3: changedFiles at/above the page cap with a mismatched total never yields OK_FOLLOWUP_REFERENCE', () => { - fc.assert( - fc.property( - fc.integer({ min: 100, max: 150 }), - fc.integer({ min: 0, max: 300 }), - (length, total) => { - fc.pre(total !== length); - const changedFiles = Array.from({ length }, (_, i) => `tests/generated-${i}.test.cjs`); - const result = evaluateIssueLink({ - prBody: 'Refs #1', - headRef: 'fix/1-something', - sameRepo: false, - changedFiles, - changedFilesTotal: total, - }); - assert.notStrictEqual(result.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - }, - ), - { numRuns: 200 }, - ); - }); - - test('P4: changedFiles below the page cap with a mismatched total never yields OK_FOLLOWUP_REFERENCE', () => { - // The sub-100 counterpart to P3, and the property the reviewed blocker - // violated: fileListIsComplete used to only consult the total once - // length >= FILE_LIST_PAGE_LIMIT, so any mismatch below the cap - // (heredoc truncation, newline inflation) went undetected below 100. - fc.assert( - fc.property( - fc.integer({ min: 1, max: 60 }), - fc.integer({ min: 0, max: 60 }), - (len, total) => { - fc.pre(total !== len); - const changedFiles = Array.from({ length: len }, (_, i) => `tests/generated-${i}.test.cjs`); - const result = evaluateIssueLink({ - prBody: 'Refs #1', - headRef: 'fix/1-something', - sameRepo: false, - changedFiles, - changedFilesTotal: total, - }); - assert.notStrictEqual(result.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - }, - ), - { numRuns: 200 }, - ); - }); -}); diff --git a/tests/require-issue-link-policy.test.cjs b/tests/require-issue-link-policy.test.cjs deleted file mode 100644 index 26ef18a21..000000000 --- a/tests/require-issue-link-policy.test.cjs +++ /dev/null @@ -1,454 +0,0 @@ -'use strict'; - -// docs-guard-exempt: allPathsAreTestsOrDocs(['tests/a.cjs', 'docs/b.md']) below -// spot-checks a pure path-classifier predicate with string literals — no real -// docs/ file is ever read or asserted on for content. - -/** - * Tests for scripts/require-issue-link-policy.cjs (#3211, preserving #1389). - * - * All assertions are on the typed ISSUE_LINK_REASON enum, never on free - * text — see CLAUDE.md "Mutation Score" / test conventions. - */ - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('node:fs'); -const path = require('node:path'); -const yaml = require('js-yaml'); - -const { - ISSUE_LINK_REASON, - hasClosingKeyword, - hasFollowUpReference, - allPathsAreTestsOrDocs, - evaluateIssueLink, - EXEMPT_PATH_PREFIXES, - EXCLUDED_ROOT_DOCS, -} = require('../scripts/require-issue-link-policy.cjs'); - -const { fileListIsComplete } = require('../scripts/pr-changed-files.cjs'); - -function forkPr(overrides = {}) { - return { - prBody: '', - headRef: 'fix/123-something', - sameRepo: false, - changedFiles: ['src/init.cts'], - changedFilesTotal: 1, - ...overrides, - }; -} - -function testPaths(n) { - return Array.from({ length: n }, (_, i) => `tests/generated-${i}.test.cjs`); -} - -describe('evaluateIssueLink', () => { - // 1. Real-world vector: a fork PR that adds regression coverage for an - // issue without closing it. - test('fork PR referencing an issue with a tests-only diff is OK_FOLLOWUP_REFERENCE', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #2269 — regression coverage.', - changedFiles: ['tests/commit-files-pathspec.test.cjs'], - changedFilesTotal: 1, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - assert.strictEqual(result.ok, true); - }); - - // 2. Every accepted reference form, including a lowercase variant. - test('every accepted follow-up reference form passes with a tests-only diff', () => { - const forms = [ - 'Refs #1', 'Ref #1', 'References #1', 'Relates to #1', - 'Related to #1', 'Follow-up to #1', 'Follow up to #1', 'refs #1', - ]; - for (const body of forms) { - const result = evaluateIssueLink(forkPr({ prBody: body, changedFiles: ['tests/a.test.cjs'], changedFilesTotal: 1 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE, `form: ${body}`); - } - }); - - // 3. Docs-only and mixed tests+docs diffs are both allowed. - test('docs-only and mixed tests+docs diffs pass', () => { - const docsOnly = evaluateIssueLink(forkPr({ - prBody: 'Refs #1', changedFiles: ['docs/CONFIGURATION.md'], changedFilesTotal: 1, - })); - assert.strictEqual(docsOnly.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - - const mixed = evaluateIssueLink(forkPr({ - prBody: 'Refs #1', changedFiles: ['tests/a.test.cjs', 'docs/guide.md'], changedFilesTotal: 2, - })); - assert.strictEqual(mixed.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - }); - - // 4. Windows-style backslash path is normalized before the prefix check. - test('backslash path is normalized and recognized as tests/', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #1', changedFiles: ['tests\\windows\\a.test.cjs'], changedFilesTotal: 1, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - }); - - // 5. CRLF vs LF bodies must produce the same reason. - test('CRLF and LF bodies with the same reference produce the same reason', () => { - const lf = evaluateIssueLink(forkPr({ prBody: 'Refs #1\n\nMore prose.', changedFiles: ['tests/a.test.cjs'], changedFilesTotal: 1 })); - const crlf = evaluateIssueLink(forkPr({ prBody: 'Refs #1\r\n\r\nMore prose.', changedFiles: ['tests/a.test.cjs'], changedFilesTotal: 1 })); - assert.strictEqual(lf.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - assert.strictEqual(crlf.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - }); - - // 6. No reference at all, even with a docs-only diff, fails. - test('no reference at all fails even with a docs-only diff', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Just a description, no issue mentioned.', changedFiles: ['docs/guide.md'], changedFilesTotal: 1, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_NO_ISSUE_REFERENCE); - }); - - // 7 & 8. A reference (not a closing keyword) touching source files needs - // an actual closing keyword instead. - test('reference-only PR touching a source file fails needs-closing', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #2269', changedFiles: ['src/init.cts'], changedFilesTotal: 1, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_REFERENCE_NEEDS_CLOSING); - }); - - test('reference-only PR with a mixed tests+source diff fails needs-closing', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #2269', - changedFiles: ['tests/a.test.cjs', 'tests/b.test.cjs', 'src/b.cts'], - changedFilesTotal: 3, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_REFERENCE_NEEDS_CLOSING); - }); - - // 9. Bodies that must NOT be recognized as any kind of issue reference. - test('non-reference bodies fail with FAIL_NO_ISSUE_REFERENCE', () => { - const bodies = ['see #123', '#123', 'unlike #123', 'issue 123', 'a #123 b']; - for (const body of bodies) { - const result = evaluateIssueLink(forkPr({ prBody: body, changedFiles: ['tests/a.test.cjs'], changedFilesTotal: 1 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_NO_ISSUE_REFERENCE, `body: ${JSON.stringify(body)}`); - } - }); - - // 10. Lookalike words that embed "ref"/"reference" inside a longer word - // must not be treated as a reference. - test('lookalike embedded-word bodies fail with FAIL_NO_ISSUE_REFERENCE', () => { - const bodies = ['prefs #1', 'unreferenced #1', 'xref#1', 'preferences #1']; - for (const body of bodies) { - const result = evaluateIssueLink(forkPr({ prBody: body, changedFiles: ['tests/a.test.cjs'], changedFilesTotal: 1 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_NO_ISSUE_REFERENCE, `body: ${JSON.stringify(body)}`); - } - }); - - // 11. Directory-lookalike paths (tests-e2e/, src/tests/, testsuite/, - // docsite/) must NOT be treated as tests/ or docs/. - test('directory-lookalike paths are rejected, forcing needs-closing', () => { - const paths = ['tests-e2e/src/a.ts', 'src/tests/x.ts', 'testsuite/y.cjs', 'docsite/z.md']; - for (const p of paths) { - const result = evaluateIssueLink(forkPr({ prBody: 'Refs #1', changedFiles: [p], changedFilesTotal: 1 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_REFERENCE_NEEDS_CLOSING, `path: ${p}`); - } - }); - - // 12. A closing keyword on a source diff passes outright. - test('Closes #123 with a source diff is OK_CLOSING_KEYWORD', () => { - const result = evaluateIssueLink(forkPr({ prBody: 'Closes #123', changedFiles: ['src/init.cts'], changedFilesTotal: 1 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_CLOSING_KEYWORD); - }); - - // 13. Closing keyword case/whitespace variants. - test('closing keyword variants (case, whitespace) all pass', () => { - const bodies = ['closes #1', 'FIXES #1', 'Resolves #1', 'resolves\t#1']; - for (const body of bodies) { - const result = evaluateIssueLink(forkPr({ prBody: body, changedFiles: ['src/init.cts'], changedFilesTotal: 1 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_CLOSING_KEYWORD, `body: ${JSON.stringify(body)}`); - } - }); - - // 14 & 15. #1389 anti-forgery property: the backmerge exemption requires - // BOTH the branch prefix AND sameRepo === true. - test('backmerge branch + sameRepo true is exempt with no reference at all', () => { - const result = evaluateIssueLink(forkPr({ - prBody: '', headRef: 'chore/backmerge-main-to-next-20260101', sameRepo: true, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_BACKMERGE_EXEMPT); - }); - - test('#1389 anti-forgery: backmerge branch name from a FORK (sameRepo false) is NOT exempt', () => { - const result = evaluateIssueLink(forkPr({ - prBody: '', headRef: 'chore/backmerge-main-to-next-20260101', sameRepo: false, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_NO_ISSUE_REFERENCE); - }); - - // #4196: Dependabot has no mechanism to link a PR it opens to a repo - // issue (its alerts live in the Security tab, not as issues) — exempt by - // authenticated author login, with no reference and no source-file - // restriction, mirroring the backmerge exemption's structure above. - test('#4196: dependabot[bot] author is exempt with no reference at all, even on a source diff', () => { - const result = evaluateIssueLink(forkPr({ - prBody: '', authorLogin: 'dependabot[bot]', changedFiles: ['src/init.cts'], changedFilesTotal: 1, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_DEPENDABOT_EXEMPT); - assert.strictEqual(result.ok, true); - }); - - test('#4196 anti-forgery: an author login that merely CONTAINS "dependabot" is NOT exempt', () => { - const lookalikes = ['dependabot', 'Dependabot[bot]', 'not-dependabot[bot]', 'dependabot[bot] ']; - for (const authorLogin of lookalikes) { - const result = evaluateIssueLink(forkPr({ prBody: '', authorLogin, changedFiles: ['src/init.cts'], changedFilesTotal: 1 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_NO_ISSUE_REFERENCE, `authorLogin: ${JSON.stringify(authorLogin)}`); - } - }); - - // 16. hasClosingKeyword corpus parity — expected values come from the - // shipped shell grep this regex replaces: - // grep -qiE '(closes|fixes|resolves)\s+#[0-9]+' - test('hasClosingKeyword corpus parity with the replaced shell grep', () => { - const cases = [ - ['Closes #2269', true], - ['closes #1', true], - ['Fixes #12', true], - ['Resolves #3', true], - ['fixes #4', true], - ['Closes #123 and more prose', true], - ['Refs #2269', false], - ['Follow-up to #2269', false], - ['no reference at all', false], - ['Closes #', false], - ['Closes123', false], - ['closes issue 5', false], - ]; - for (const [body, expected] of cases) { - assert.strictEqual(hasClosingKeyword(body), expected, `body: ${JSON.stringify(body)}`); - } - }); - - // 17-18. BOUNDARY: exactly at and just below the page cap, with a total - // that matches, must pass. - test('BOUNDARY 99 tests-only paths, total 99 passes', () => { - const paths = testPaths(99); - const result = evaluateIssueLink(forkPr({ prBody: 'Refs #1', changedFiles: paths, changedFilesTotal: 99 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - }); - - test('BOUNDARY 100 tests-only paths, total 100 passes', () => { - const paths = testPaths(100); - const result = evaluateIssueLink(forkPr({ prBody: 'Refs #1', changedFiles: paths, changedFilesTotal: 100 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - }); - - // 19. Real vector: PR #3202 — gh returns 100 of 118 changed files. - test('BOUNDARY 100 tests-only paths, total 118 fails FAIL_FILE_LIST_INCOMPLETE (PR #3202 vector)', () => { - const paths = testPaths(100); - const result = evaluateIssueLink(forkPr({ prBody: 'Refs #1', changedFiles: paths, changedFilesTotal: 118 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_FILE_LIST_INCOMPLETE); - }); - - test('100 tests-only paths, total undefined fails FAIL_FILE_LIST_INCOMPLETE', () => { - const paths = testPaths(100); - const result = evaluateIssueLink(forkPr({ prBody: 'Refs #1', changedFiles: paths, changedFilesTotal: undefined })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_FILE_LIST_INCOMPLETE); - }); - - // Reviewed BLOCKER: the old fileListIsComplete only consulted the total - // when length >= FILE_LIST_PAGE_LIMIT, so ANY mechanism that shortens the - // list below 100 went undetected. A $GITHUB_OUTPUT heredoc terminated - // early by a file named after the delimiter is exactly such a mechanism — - // it truncates the list well below the page cap, with the true total - // still available from the separate, non-paginated `changedFiles` field. - test('a list shorter than its authoritative total fails closed', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #1', changedFiles: ['CONTRIBUTING.md'], changedFilesTotal: 3, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_FILE_LIST_INCOMPLETE); - }); - - // A path containing a literal newline can inflate the parsed list past the - // true total (e.g. a filename that itself looks like another path once - // split on newlines) — the total is the authority in both directions. - test('a list longer than its authoritative total fails closed', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #1', - changedFiles: ['tests/a.test.cjs', 'tests/b.test.cjs', 'tests/c.test.cjs'], - changedFilesTotal: 2, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_FILE_LIST_INCOMPLETE); - }); - - // 21. An empty changedFiles list cannot confirm anything. - test('empty changedFiles fails FAIL_FILE_LIST_INCOMPLETE', () => { - const result = evaluateIssueLink(forkPr({ prBody: 'Refs #1', changedFiles: [], changedFilesTotal: 0 })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_FILE_LIST_INCOMPLETE); - }); -}); - -describe('fileListIsComplete', () => { - // 22. Direct table of (changedFiles, changedFilesTotal) -> expected. - test('boundary table', () => { - const cases = [ - [['a', 'b', 'c'], 3, true], - [['a', 'b', 'c'], undefined, true], - [testPaths(100), 100, true], - [testPaths(100), 101, false], - [testPaths(100), undefined, false], - [[], 0, false], - [undefined, 0, false], - [testPaths(3), 5, false], - [testPaths(5), 3, false], - [testPaths(3), 3, true], - [testPaths(3), undefined, true], - ]; - for (const [changedFiles, changedFilesTotal, expected] of cases) { - assert.strictEqual( - fileListIsComplete(changedFiles, changedFilesTotal), - expected, - `changedFiles.length=${Array.isArray(changedFiles) ? changedFiles.length : changedFiles}, total=${changedFilesTotal}`, - ); - } - }); -}); - -describe('hasFollowUpReference', () => { - // 23. Direct spot checks on the raw predicate. - test('rejects a closing keyword, accepts a reference', () => { - assert.strictEqual(hasFollowUpReference('Closes #1'), false); - assert.strictEqual(hasFollowUpReference('Refs #1'), true); - }); -}); - -describe('allPathsAreTestsOrDocs', () => { - // 24. Direct spot checks on the raw predicate. - test('true for tests/docs mix, false for a non-exempt path, false for empty', () => { - assert.strictEqual(allPathsAreTestsOrDocs(['tests/a.cjs', 'docs/b.md']), true); - assert.strictEqual(allPathsAreTestsOrDocs(['tests/a.cjs', '.github/workflows/x.yml']), false); - assert.strictEqual(allPathsAreTestsOrDocs([]), false); - }); - - // 25. Root-level markdown is a third accepted shape; CHANGELOG.md is - // excluded from it even though it is root-level markdown. - test('root-level markdown is accepted, CHANGELOG.md and subdirectory markdown are not', () => { - assert.strictEqual(allPathsAreTestsOrDocs(['CONTRIBUTING.md']), true); - assert.strictEqual(allPathsAreTestsOrDocs(['CHANGELOG.md']), false); - assert.strictEqual(allPathsAreTestsOrDocs(['agents/x.md']), false); - assert.strictEqual(allPathsAreTestsOrDocs(['package.json']), false); - }); - - // The exclusion of CHANGELOG.md must not be defeatable by casing — the - // extension test (`/\.md$/i`) is already case-insensitive, so the - // exclusion lookup must match it rather than silently letting a - // differently-cased CHANGELOG.md ride in on the docs carve-out. - test('CHANGELOG.md exclusion is case-insensitive', () => { - assert.strictEqual(allPathsAreTestsOrDocs(['changelog.md']), false); - assert.strictEqual(allPathsAreTestsOrDocs(['CHANGELOG.MD']), false); - }); -}); - -describe('require-issue-link policy — root-level documentation (#2290 shape)', () => { - // #2290 is the motivating PR the follow-up-reference exemption failed to - // cover: its diff is CONTRIBUTING.md (root-level markdown) plus a tests/ - // file, and the old EXEMPT_PATH_PREFIXES-only predicate rejected it because - // CONTRIBUTING.md is neither tests/ nor docs/-prefixed. - test('the motivating PR #2290 shape qualifies', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #2269', - changedFiles: ['CONTRIBUTING.md', 'tests/commit-files-pathspec.test.cjs'], - changedFilesTotal: 2, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - }); - - test('a root-level README change qualifies', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #2269', changedFiles: ['README.md'], changedFilesTotal: 1, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.OK_FOLLOWUP_REFERENCE); - }); - - // scripts/changeset/lint.cjs classes a direct CHANGELOG.md edit as - // user-facing specifically to close a bypass — the docs carve-out here - // must not undo that by treating CHANGELOG.md as ordinary documentation. - test('a direct CHANGELOG.md edit does NOT qualify', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #2269', changedFiles: ['CHANGELOG.md'], changedFilesTotal: 1, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_REFERENCE_NEEDS_CLOSING); - }); - - test('CHANGELOG.md alongside real docs still disqualifies', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #2269', changedFiles: ['docs/a.md', 'CHANGELOG.md'], changedFilesTotal: 2, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_REFERENCE_NEEDS_CLOSING); - }); - - // These are runtime-loaded text, deliberately gated by the same root-only - // anchor pre-pr-gate.sh uses — a subdirectory .md file is not root-level. - test('subdirectory markdown is not root-level documentation', () => { - const paths = ['msd-core/workflows/next.md', 'agents/reviewer.md', 'commands/msd/plan.md', 'src/notes.md']; - for (const p of paths) { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #2269', changedFiles: [p], changedFilesTotal: 1, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_REFERENCE_NEEDS_CLOSING, `path: ${p}`); - } - }); - - test('a root-level non-markdown file does not qualify', () => { - const result = evaluateIssueLink(forkPr({ - prBody: 'Refs #2269', changedFiles: ['package.json'], changedFilesTotal: 1, - })); - assert.strictEqual(result.reason, ISSUE_LINK_REASON.FAIL_REFERENCE_NEEDS_CLOSING); - }); -}); - -describe('require-issue-link policy — the workflow guidance matches the rule', () => { - // Parity assertion (CLAUDE.md "Generative Fix Divergence"): the sticky - // comment's guidance text is generated separately from the predicate it - // describes, and the two have already drifted once (the predicate widened - // to accept root-level markdown while the guidance kept saying "nothing - // outside tests/ and docs/"). Every expectation below is derived from the - // module's actual exports, never hardcoded, so a future widening of the - // predicate without a guidance update fails this test. - const workflowPath = path.join(__dirname, '..', '.github', 'workflows', 'require-issue-link.yml'); - const workflowDoc = yaml.load(fs.readFileSync(workflowPath, 'utf8')); - const job = workflowDoc.jobs['check-issue-link']; - const steps = job.steps; - const lastStep = steps[steps.length - 1]; - const script = lastStep.with.script; - - // Non-vacuous guards: if these fail, the assertions below would otherwise - // silently pass against zero-length input. - test('the resolved script text and export lists are non-empty (guard)', () => { - assert.strictEqual(typeof script, 'string'); - assert.ok(script.length > 200, `expected script.length > 200, got ${script.length}`); - assert.ok(EXEMPT_PATH_PREFIXES.length > 0, 'EXEMPT_PATH_PREFIXES must be non-empty'); - assert.ok(EXCLUDED_ROOT_DOCS instanceof Set, 'EXCLUDED_ROOT_DOCS must be a Set'); - assert.ok(EXCLUDED_ROOT_DOCS.size > 0, 'EXCLUDED_ROOT_DOCS must be non-empty'); - }); - - test('guidance names every EXEMPT_PATH_PREFIXES entry verbatim', () => { - for (const prefix of EXEMPT_PATH_PREFIXES) { - assert.ok(script.includes(prefix), `guidance script missing prefix: ${prefix}`); - } - }); - - // isRootLevelDoc accepts root-level *.md — the guidance must say so; this - // is the exact shape that drifted before. - test('guidance mentions root-level markdown', () => { - assert.ok(script.includes('root-level'), 'guidance script missing "root-level"'); - }); - - test('guidance names every EXCLUDED_ROOT_DOCS entry verbatim', () => { - for (const doc of EXCLUDED_ROOT_DOCS) { - assert.ok(script.includes(doc), `guidance script missing excluded doc: ${doc}`); - } - }); - - test('guidance mentions an accepted non-closing reference marker', () => { - assert.ok(script.includes('Refs #'), 'guidance script missing "Refs #"'); - }); -}); diff --git a/tests/workflow-maintainer-skip.test.cjs b/tests/workflow-maintainer-skip.test.cjs index 1af1c5138..bd711782a 100644 --- a/tests/workflow-maintainer-skip.test.cjs +++ b/tests/workflow-maintainer-skip.test.cjs @@ -7,9 +7,6 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); -const yaml = require('js-yaml'); - -const { evaluateIssueLink, ISSUE_LINK_REASON } = require('../scripts/require-issue-link-policy.cjs'); const MAINTAINER_SKIP_EXPR = 'contains(fromJSON(\'["OWNER","MEMBER","COLLABORATOR"]\'), github.event.pull_request.author_association) == false'; @@ -53,42 +50,20 @@ function assertMaintainerSkip(source) { } describe('PR policy workflow maintainer carve-outs', () => { - test('draft PR auto-close does not run for maintainer-authored PRs', () => { - const workflow = readWorkflow('.github/workflows/close-draft-prs.yml'); - - assert.match(workflow, /github\.event\.pull_request\.draft == true/); - assertMaintainerSkip(workflow); - }); - - test('draft PR auto-close triggers on pull_request_target so fork PRs cannot bypass it', () => { - const workflow = readWorkflow('.github/workflows/close-draft-prs.yml'); - - // A bare `pull_request` trigger hands fork PRs (how first-time/external - // contributors contribute) a read-only GITHUB_TOKEN, so the close/comment - // API calls 403 and the draft PR survives — bypassing the auto-close. - // `pull_request_target` runs in the base-repo context with a write-capable - // token. Guard against a regression back to the bypassable trigger. - assert.match(workflow, /^\s*pull_request_target:/m); - assert.doesNotMatch(workflow, /^\s*pull_request:\s*$/m); - }); - test('PR target validator does not run for maintainer-authored PRs', () => { const workflow = readWorkflow('.github/workflows/pr-target-validator.yml'); assertMaintainerSkip(workflow); }); - // #2331: same defect class as the close-draft-prs.yml trigger lock above, - // swept across the three PR-policy workflows it had never covered. Each one + // #2331: a PR-policy workflow that comments and THEN emits its verdict. Each one // comments on the PR and THEN emits its verdict; on a bare `pull_request` // trigger a fork PR's read-only GITHUB_TOKEN 403s the comment call, the // unhandled rejection kills the github-script step, and the verdict // (core.setFailed) never runs — the contributor gets an API stack trace // instead of the instructions the comment exists to deliver. for (const { file, name, scope, otherScope } of [ - { file: '.github/workflows/pr-title-validator.yml', name: 'PR title validator', scope: 'pull-requests', otherScope: 'issues' }, { file: '.github/workflows/pr-target-validator.yml', name: 'PR target validator', scope: 'pull-requests', otherScope: 'issues' }, - { file: '.github/workflows/require-issue-link.yml', name: 'Require issue link', scope: 'issues', otherScope: 'pull-requests' }, ]) { test(`${name} triggers on pull_request_target so fork PRs get the verdict, not a 403`, () => { const workflow = readWorkflow(file); @@ -108,9 +83,9 @@ describe('PR policy workflow maintainer carve-outs', () => { // // Each file needs only ONE: GitHub accepts either `issues: write` or // `pull-requests: write` for issues.createComment when the target is a - // PR. Verified from this repo's history, not the docs — pr-title-validator - // has posted on `pull-requests: write` alone, require-issue-link on - // `issues: write` alone. The 403 was the fork downgrade, not the scope. + // PR. Verified from this repo's history, not the docs — pr-target-validator + // has posted on `pull-requests: write` alone. The 403 was the fork + // downgrade, not the scope. // // The negative half is the point of this test: a pull_request_target // workflow must not carry privilege it never exercises, so adding the @@ -149,13 +124,12 @@ describe('PR policy workflow maintainer carve-outs', () => { }); } - // #2331: these two workflows echo attacker-controlled text (PR title / fork - // branch name) into a bot-authored comment posted with a write token. Raw + // #2331: this workflow echoes attacker-controlled text (the fork branch + // name) into a bot-authored comment posted with a write token. Raw // interpolation into an inline-code span lets a single backtick close the span // so the remainder renders as live Markdown — on a PR title (no charset limit) // that is enough to autolink an arbitrary URL from github-actions[bot]. for (const { file, name, varName } of [ - { file: '.github/workflows/pr-title-validator.yml', name: 'PR title validator', varName: 'titleForMarkdown' }, { file: '.github/workflows/pr-target-validator.yml', name: 'PR target validator', varName: 'headForMarkdown' }, ]) { test(`${name} strips backticks before echoing untrusted text into the comment`, () => { @@ -200,8 +174,8 @@ describe('PR policy workflow maintainer carve-outs', () => { // The exact trap that made the first cut of the ordering test fail against // correct code: these workflows name `core.setFailed` in their own comments, // before the call, so a raw-source indexOf compares the comment. - const raw = readWorkflow('.github/workflows/pr-title-validator.yml'); - const code = readWorkflowCode('.github/workflows/pr-title-validator.yml'); + const raw = readWorkflow('.github/workflows/pr-target-validator.yml'); + const code = readWorkflowCode('.github/workflows/pr-target-validator.yml'); assert.ok( raw.indexOf('core.setFailed') < raw.indexOf('Could not post'), @@ -230,100 +204,6 @@ describe('PR policy workflow maintainer carve-outs', () => { assert.equal(sanitize('``a``'), "''a''"); }); - test('draft PR sweep enforces the same policy as the event-driven close', () => { - const workflow = readWorkflow('.github/workflows/close-draft-prs-sweep.yml'); - - // Timer-driven in base-repo context, plus a manual dispatch for testing. - // It must NOT be a fork-triggered event (no pull_request / pull_request_target trigger). - assert.match(workflow, /schedule:/); - assert.match(workflow, /cron:\s*'0 \*\/6 \* \* \*'/); - assert.match(workflow, /workflow_dispatch:/); - assert.doesNotMatch(workflow, /^\s*pull_request(_target)?:/m); - - // Write-capable token (needed to close PRs from base context). Tolerant of - // intervening blank lines or additional permission keys. - assert.match(workflow, /permissions:\s+pull-requests:\s*write/); - - // Identical maintainer carve-out to close-draft-prs.yml — a Set membership - // test over author_association, negated (no github.event.pull_request in a - // scheduled run). - assert.match(workflow, /new Set\(\['OWNER', 'MEMBER', 'COLLABORATOR'\]\)/); - assert.match(workflow, /!MAINTAINER_ASSOCIATIONS\.has\([^)]*\.author_association\)/); - - // Paginates over open PRs and filters to drafts. - assert.match(workflow, /github\.paginate\(github\.rest\.pulls\.list/); - assert.match(workflow, /state:\s*'open'/); - assert.match(workflow, /pr\.draft === true/); - - // Same user-facing policy message as close-draft-prs.yml (locks the core - // content so the sweep cannot silently drift to a weaker message). - assert.match(workflow, /## Draft PRs are not accepted/); - assert.match(workflow, /npm run test:coverage/); - assert.match(workflow, /CONTRIBUTING\.md#pull-request-guidelines/); - }); -}); - -describe('Require Issue Link back-merge automation carve-out', () => { - // #3211 moved the whole verdict — including the #1389 backmerge carve-out — - // out of the YAML `if:` and into scripts/require-issue-link-policy.cjs - // (evaluateIssueLink), renaming the step from `check`/`found` to - // `policy`/`ok`. The old assertions here measured the carve-out's PREVIOUS - // location (raw `startsWith(github.head_ref, ...)` / `head.repo.full_name` - // text inside the workflow's `if:`) and went stale once that logic - // relocated — the property they guarded still holds, just not at that - // text. This test locks the property at its new home: (a)+(b) assert the - // workflow now delegates to the policy module at the correct step-level - // `if:`, (c) asserts the carve-out itself BEHAVIORALLY against the real - // module, and (d) keeps the bootstrap fallback's legacy grep locked so the - // introducing-PR path (before the module exists on the base branch) is - // never silently open. - test('the backmerge carve-out survives the move into the policy module', () => { - const workflow = readWorkflow('.github/workflows/require-issue-link.yml'); - - // (a) The failing step's `if:` is keyed on the policy output and is - // STEP-level, not job-level. A job-level `if:` would make the required - // "Issue link required" check report `skipped` instead of `success` on - // an exempt PR, which blocks branch protection (#1389). - assert.match(workflow, /if: steps\.policy\.outputs\.ok != 'true'/); - - const doc = yaml.load(workflow); - assert.equal( - doc.jobs['check-issue-link'].if, - undefined, - 'job-level `if:` would report "skipped" on an exempt PR and block branch protection (#1389)' - ); - - // (b) The workflow delegates the verdict to the policy module. - assert.match(workflow, /node scripts\/require-issue-link-policy\.cjs/); - - // (c) The carve-out itself still holds — asserted BEHAVIORALLY against - // the real module rather than by grepping YAML, since the verdict no - // longer lives in the YAML text at all. - const backmergeArgs = { - prBody: 'Automated backmerge.', - headRef: 'chore/backmerge-main-to-next-20260101', - changedFiles: ['src/a.cts'], - changedFilesTotal: 1, - }; - assert.equal( - evaluateIssueLink({ ...backmergeArgs, sameRepo: true }).reason, - ISSUE_LINK_REASON.OK_BACKMERGE_EXEMPT - ); - // #1389 anti-forgery: a fork must not be able to forge the exemption by - // branch name alone — dropping the `sameRepo` conjunct would let a fork - // PR name its branch `chore/backmerge-main-to-next-*` and bypass the - // issue-link requirement entirely. - assert.equal( - evaluateIssueLink({ ...backmergeArgs, sameRepo: false }).reason, - ISSUE_LINK_REASON.FAIL_NO_ISSUE_REFERENCE - ); - - // (d) The bootstrap fallback (used only on the PR that first introduces - // the policy module, before it exists on the base branch) still carries - // the legacy closing-keyword grep, so the gate is never silently open - // during the changeover. - assert.match(workflow, /\(closes\|fixes\|resolves\)/); - }); }); describe('Auto-backmerge needs_review version-manifest carve-out (#1404)', () => {