Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions .github/scripts/spec-id-collision.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// Copyright (c) Meta Platforms, Inc. and affiliates.

'use strict';
/* global module, require */

const {
listLandedSpecIds,
loadOpenPullRequests,
reservedSpecIds,
} = require('../../scripts/lib/spec-id.cjs');

function checkSpecIdCollisions({
openPulls,
currentPullNumber,
expectedHeadOid,
landedIds,
}) {
const current = openPulls.find(pull => pull.number === currentPullNumber);
if (!current) {
throw new Error(
`PR #${currentPullNumber} is not in the open-PR inventory.`,
);
}
if (current.headRefOid !== expectedHeadOid) {
throw new Error(
`PR #${currentPullNumber} moved from ${expectedHeadOid} to ${current.headRefOid}; rerun on the latest head.`,
);
}

const candidateIds = reservedSpecIds(current.files, landedIds);
const conflicts = [];
for (const id of candidateIds) {
const pulls = openPulls.filter(
pull =>
pull.number !== currentPullNumber &&
reservedSpecIds(pull.files, landedIds).includes(id),
);
if (pulls.length > 0) conflicts.push({id, pulls});
}
return {candidateIds, conflicts};
}

function formatConflictMessage(conflicts) {
const lines = ['System-spec ID reservation collision:'];
for (const {id, pulls} of conflicts) {
for (const pull of pulls) {
lines.push(
`- ${id} is also reserved by #${pull.number}: ${pull.title} (${pull.url})`,
);
}
}
lines.push(
'',
'Run `pnpm spec:id` again, move the new spec to the suggested path, update its frontmatter ID, and push.',
);
return lines.join('\n');
}

async function runSpecIdCollisionGate({github, context, core, root}) {
const eventPull = context.payload.pull_request;
if (!eventPull) throw new Error('This check requires a pull_request event.');
const {owner, repo} = context.repo;
if (eventPull.base?.repo?.full_name !== `${owner}/${repo}`) {
throw new Error(
'The event repository does not match the checked repository.',
);
}

const openPulls = await loadOpenPullRequests({
owner,
repo,
graphql: (query, variables) => github.graphql(query, variables),
});
const result = checkSpecIdCollisions({
openPulls,
currentPullNumber: eventPull.number,
expectedHeadOid: eventPull.head.sha,
landedIds: listLandedSpecIds(root),
});

if (result.conflicts.length > 0) {
core.setFailed(formatConflictMessage(result.conflicts));
} else if (result.candidateIds.length > 0) {
core.info(`Reserved by this PR: ${result.candidateIds.join(', ')}`);
} else {
core.info('This PR does not reserve a new system-spec ID.');
}
return result;
}

module.exports = {
checkSpecIdCollisions,
formatConflictMessage,
runSpecIdCollisionGate,
};
117 changes: 117 additions & 0 deletions .github/scripts/spec-id-collision.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Copyright (c) Meta Platforms, Inc. and affiliates.

import fs from 'node:fs';
import path from 'node:path';
import {createRequire} from 'node:module';
import {describe, expect, it} from 'vitest';

const require = createRequire(import.meta.url);
const {
checkSpecIdCollisions,
formatConflictMessage,
} = require('./spec-id-collision.cjs');

const root = path.resolve(import.meta.dirname, '../..');

function pull(number, files, headRefOid = `head-${number}`) {
return {
number,
title: `PR ${number}`,
url: `https://github.com/facebook/astryx/pull/${number}`,
headRefOid,
changedFiles: files.length,
files,
};
}

function added(id) {
return {path: `docs/specs/${id}/spec.md`, changeType: 'ADDED'};
}

describe('spec ID collision gate', () => {
it('reports two open additions of the same new ID', () => {
const result = checkSpecIdCollisions({
openPulls: [pull(10, [added('AST-013')]), pull(20, [added('AST-013')])],
currentPullNumber: 10,
expectedHeadOid: 'head-10',
landedIds: ['AST-001'],
});

expect(result.conflicts).toEqual([
{id: 'AST-013', pulls: [expect.objectContaining({number: 20})]},
]);
expect(formatConflictMessage(result.conflicts)).toContain(
'AST-013 is also reserved by #20',
);
expect(formatConflictMessage(result.conflicts)).toContain('pnpm spec:id');
});

it('allows a modification to an already-landed ID', () => {
const result = checkSpecIdCollisions({
openPulls: [
pull(10, [
{path: 'docs/specs/AST-002/spec.md', changeType: 'MODIFIED'},
]),
pull(20, [added('AST-002')]),
],
currentPullNumber: 10,
expectedHeadOid: 'head-10',
landedIds: ['AST-002'],
});
expect(result).toEqual({candidateIds: [], conflicts: []});
});

it('detects a rename into an ID reserved by another PR', () => {
const result = checkSpecIdCollisions({
openPulls: [
pull(10, [
{
path: 'docs/specs/AST-013/spec.md',
changeType: 'RENAMED',
},
]),
pull(20, [added('AST-013')]),
],
currentPullNumber: 10,
expectedHeadOid: 'head-10',
landedIds: ['AST-001'],
});
expect(result.conflicts).toHaveLength(1);
});

it('excludes the current PR from its collision set', () => {
const result = checkSpecIdCollisions({
openPulls: [pull(10, [added('AST-013')])],
currentPullNumber: 10,
expectedHeadOid: 'head-10',
landedIds: ['AST-001'],
});
expect(result).toEqual({candidateIds: ['AST-013'], conflicts: []});
});

it('fails when the queried head no longer matches the event head', () => {
expect(() =>
checkSpecIdCollisions({
openPulls: [pull(10, [added('AST-013')], 'new-head')],
currentPullNumber: 10,
expectedHeadOid: 'event-head',
landedIds: [],
}),
).toThrow('moved from event-head to new-head');
});

it('keeps the workflow trusted and read-only', () => {
const workflow = fs.readFileSync(
path.join(root, '.github/workflows/spec-id-reservation.yml'),
'utf8',
);
expect(workflow).toContain('pull_request_target:');
expect(workflow).toContain(
'ref: ${{ github.event.repository.default_branch }}',
);
expect(workflow).toContain('persist-credentials: false');
expect(workflow).toContain('pull-requests: read');
expect(workflow).not.toContain('pull-requests: write');
expect(workflow).not.toContain('github.event.pull_request.head.sha }}');
});
});
54 changes: 54 additions & 0 deletions .github/workflows/spec-id-reservation.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.

# A spec ID is reserved by an open PR that adds or renames a new
# docs/specs/AST-NNN/spec.md path. This trusted, read-only workflow compares the
# exact current PR head with complete file inventories for every open PR.
name: Spec ID reservation

on:
pull_request_target:
branches: [main]
types: [opened, synchronize, reopened]

permissions: {}

concurrency:
group: spec-id-reservation
cancel-in-progress: false

jobs:
collision:
name: Spec ID collision
runs-on: ubuntu-slim
permissions:
contents: read
pull-requests: read
steps:
# pull_request_target is privileged context, so execute only helpers from
# the trusted default branch. Never check out or run the PR head here.
- uses: actions/checkout@v7
with:
ref: ${{ github.event.repository.default_branch }}
persist-credentials: false

- name: Check open spec ID reservations
uses: actions/github-script@v9
with:
script: |
const path = require('node:path');
const {runSpecIdCollisionGate} = require(path.join(
process.env.GITHUB_WORKSPACE,
'.github/scripts/spec-id-collision.cjs',
));
try {
await runSpecIdCollisionGate({
github,
context,
core,
root: process.env.GITHUB_WORKSPACE,
});
} catch (error) {
core.setFailed(
`Spec ID collision check could not complete safely: ${error.message}`,
);
}
17 changes: 16 additions & 1 deletion docs/specs/README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# System specs

This directory contains records for consequential shared-system changes. Each
spec lives under `docs/specs/<id>-<slug>/spec.md`; an optional sibling `plan.md`
spec lives under `docs/specs/AST-NNN/spec.md`; an optional sibling `plan.md`
is used only for multi-step implementation.

Templates live separately under `docs/templates/knowledge/`. Existing records
Expand All @@ -17,3 +17,18 @@ Only records with `authority: current` are authoritative. Draft records may
carry unresolved evidence or owner decisions; they do not govern review.
Archived records state why they no longer govern and link a replacement when
one exists. Initial promotion to `current` requires explicit owner approval.

## Reserve an ID

Run `pnpm spec:id` before starting a system spec. The read-only default checks
landed specs and complete file inventories for every open pull request, then
prints the lowest available ID at or above the next landed number. Run
`pnpm spec:id -- --write` to scaffold that proposal from the system-spec
template.

An ID is reserved only after a pull request adds or renames its exact
`docs/specs/AST-NNN/spec.md` path. Re-run the helper immediately before opening
the pull request. The **Spec ID collision** check rejects duplicate open
reservations; when that happens, re-run the helper and move the spec to its new
suggested path. Changes to an ID that already exists on `main` are not new
reservations.
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@
"dev:sandbox": "pnpm -F @astryxdesign/core build && pnpm -F @astryxdesign/sandbox dev",
"dev:sandbox:source": "ASTRYX_SOURCE=1 pnpm -F @astryxdesign/sandbox dev",
"npm:prune-canaries": "node scripts/npm/prune-canaries.mjs",
"check:knowledge": "node scripts/check-knowledge.mjs"
"check:knowledge": "node scripts/check-knowledge.mjs",
"spec:id": "node scripts/spec-id.mjs"
},
"devDependencies": {
"@axe-core/playwright": "^4.12.1",
Expand Down
22 changes: 22 additions & 0 deletions scripts/check-knowledge.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,27 @@ export function discoverKnowledgeRecords(root = DEFAULT_ROOT) {
return records.sort();
}

export function validateSystemSpecIdentity(root, absolutePath, document) {
const filePath = path.relative(root, absolutePath).split(path.sep).join('/');
const match = filePath.match(/^docs\/specs\/([^/]+)\/spec\.md$/);
if (!match) return [];

const directoryId = match[1];
if (!/^AST-[0-9]{3}$/.test(directoryId)) {
return [
`${filePath}: system specs must be placed at docs/specs/AST-NNN/spec.md.`,
];
}

const expectedId = `spec:${directoryId}`;
const actualId = document.frontmatter.get('id');
return actualId === expectedId
? []
: [
`${filePath}: frontmatter id must be ${expectedId} to match its directory; received ${JSON.stringify(actualId)}.`,
];
}

export function parseAnatomyThemingBlock(
content,
filePath = '<component spec>',
Expand Down Expand Up @@ -896,6 +917,7 @@ export async function validateKnowledgeRoot(root = DEFAULT_ROOT) {
const filePath = path.relative(root, absolutePath);
const content = fs.readFileSync(absolutePath, 'utf8');
const document = parseKnowledgeDocument(content, filePath);
problems.push(...validateSystemSpecIdentity(root, absolutePath, document));
const recordVersion = document.frontmatter.get('schema_version');
const versionedSchema = schemas.get(recordVersion);
if (!versionedSchema) {
Expand Down
29 changes: 29 additions & 0 deletions scripts/check-knowledge.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,35 @@ afterEach(() => {
fs.rmSync(root, {recursive: true, force: true});
});

describe('system-spec identity', () => {
it('accepts matching AST-NNN directory and frontmatter IDs', async () => {
const root = fixtureRoot();
writeSystemSpec(root, 'AST-900', systemSpecRecord());

expect(await validateKnowledgeRoot(root)).not.toContainEqual(
expect.stringContaining('to match its directory'),
);
});

it('rejects a frontmatter ID that differs from the directory ID', async () => {
const root = fixtureRoot();
writeSystemSpec(root, 'AST-901', systemSpecRecord());

expect((await validateKnowledgeRoot(root)).join('\n')).toContain(
'frontmatter id must be spec:AST-901 to match its directory; received "spec:AST-900".',
);
});

it('rejects a system-spec directory outside the AST-NNN format', async () => {
const root = fixtureRoot();
writeSystemSpec(root, 'AST-90', systemSpecRecord({id: 'spec:AST-90'}));

expect((await validateKnowledgeRoot(root)).join('\n')).toContain(
'system specs must be placed at docs/specs/AST-NNN/spec.md.',
);
});
});

describe('schema evolution', () => {
it('tracks latest schema versions per kind', () => {
const raw = new Map([
Expand Down
Loading
Loading