mirror of https://github.com/THU-MAIC/OpenMAIC.git synced 2026-10-18 05:40:25 +08:00

Files

T

wyucandClaude Opus 5 cc95851dc5 chore(packages): publish storage and enforce version bumps (#998 )

* chore(packages): publish storage and enforce version bumps

* fix(packages): cover all publish inputs and tarballs

* fix(packages): scope version guard to release inputs

* fix(packages): fail closed on new package inputs

* docs(packages): clarify version guard boundaries

* fix(packages): harden release guard edge cases

* fix(packages): align smoke peers and git inputs

* docs(packages): make release smoke policy explicit

* fix(packages): anchor version guard at repository root

* fix(packages): report version guard setup errors

* fix(packages): harden release validation

* fix(ci): validate complete main pushes

* test(storage): pin the exported PostgreSQL schemas

DOCUMENT_PG_SCHEMA and RUNTIME_PG_SCHEMA are public API. A deployment that
provisions these tables with its own migration tooling has to reproduce the
DDL exactly for ensureDocumentSchema() / ensureSchema() to stay the intended
no-op against an already-provisioned database.

Nothing guarded that today: every statement is CREATE ... IF NOT EXISTS, so
PostgreSQL silently accepts whatever table already exists under the name. A
column type, a nullability, an index or a FK action can drift apart from a
downstream migration without an error, and the first symptom is a store query
failing in production or succeeding against the wrong types.

Pin both constants verbatim, and assert every statement stays IF NOT EXISTS
guarded so the ensure functions remain idempotent. The pin does not judge the
DDL; it makes changing it impossible to do by accident.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(packages): gate publishing on the version check

The version check ran only in ci.yml. publish-packages.yml is a separate
workflow, so it could reach `pnpm publish` on a main push, a matching tag, or
a manual run with dry_run=false even when CI was red, skipped, or never ran.

Run the validation inside the publish job, before publishing, in a new release
mode that works for every trigger. A push range only exists for branch pushes,
so release mode judges each package against the registry instead: a version
that is not published yet is a release, and a version that is already
published is accepted only when the package source has not moved since that
release. `pnpm publish` skips an already-published version, so without that
second half a drifted package releases as a silent no-op.

The anchor for "since that release" is the @openmaic/<name>@<version> tag the
job now writes after every successful publish, falling back to the push range
for packages released before those tags existed. With neither anchor the
release stops instead of guessing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(packages): make the release gate prove only what it can

Three independent reviews of the previous revision converged on one root
cause: it used git tags and a push range as the record of which tree produced
a published version. Neither is trustworthy. Tags are mutable and can be
created or moved by hand, so "the tag resolves" was being read as "the tag is
authentic"; pnpm records no gitHead; and a push range describes one push, not
the origin of a release. Packages released before the scheme existed had no
anchor at all, which made the tag and manual triggers fail closed from the
first run with no way to self-heal, dry runs included.

Drop the anchor machinery. Drift is prevented where it is provable, in diff
mode at merge time, which sees every commit before it can be released. The
pre-publish gate is now limited to claims a release can establish: every
version it publishes is new and moves forward, an already-published version is
reported and left alone, and any registry answer that is not a definitive
404 stops the release.

Around it, close what the reviews found in the workflow:

- real publishes only from a commit contained in main, so a tag or manual run
  cannot ship an unreviewed ref while holding NPM_TOKEN
- one repository-wide concurrency group, so a main push and a tag push cannot
  each decide the same version is unpublished
- publish and mark each package individually, so a partial failure stays
  retryable instead of leaving published packages unrecorded
- fail if the build rewrites tracked package files, which would otherwise
  publish content that is not in the released commit
- compare against every published version when ordering, and refuse to guess
  when the registry holds versions this check cannot order

The schema pin also asserted the constants without asserting what the ensure
functions execute, so those could diverge. Both are now run against a
recording queryable and their exact statement sequence is asserted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(packages): refuse to release from a tree behind the registry

Simplifying the gate introduced a hole that a mutation check caught: a package
whose local version is already published took the "already published, skip"
path before any ordering check, so a tree rolled back to an older published
version passed silently. That is not a harmless no-op, because pnpm publish
rewrites each workspace dependency to the version in the tree, and a sibling
released alongside it would be published declaring a dependency on the older
package.

Order every package against the registry first, then decide: at the highest
published version it is a legitimate skip, below it the release stops.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(packages): couple publishing to a green CI run for the same commit

Second review round, on the reworked design. The headline defect was a hard
blocker: `git fetch --depth=0` is not valid git (that is an actions/checkout
convention), so under `set -e` every real publish aborted before the gate ever
ran. Remove the depth argument; the checkout already has full history.

The rest closes the gap both reviewers reached independently, that being on
main is not the same as having passed the gate:

- wait for this exact commit's CI conclusion and require success. ci.yml runs
  concurrently with this workflow on a push to main and blocks nothing, so an
  unpublished version whose source drifted could be released while its own
  version check was still running or already red.
- require the commit on main's FIRST-PARENT history. Plain reachability also
  accepts every intermediate commit of every branch merged with a merge commit.
  A pull request that adds a bad tree and reverts it in the next commit leaves
  that tree an ancestor of main forever, and a tag pointing at it published it.
- do not cancel CI runs on main. Each run validates only its own push range, so
  cancelling one drops the range that contained the change, and its replacement
  compares against the cancelled tip and sees nothing.
- run the version check on every main push. A branch creation or force push
  reports an unusable `before`, which used to skip the gate entirely; fall back
  to the first parent instead.
- publish with --ignore-scripts. Every package's prepublishOnly reruns the
  build, deleting and regenerating the dist that was just verified and
  smoke-tested, so npm could receive bytes nothing had checked.
- run storage's PostgreSQL contract suites before publishing it. They skip
  themselves without a database, so the one backend that needs a real
  PostgreSQL was shipping unexercised.
- order registry versions with real semver, prereleases included. Rejecting
  every non-x.y.z version meant one historical prerelease anywhere in one
  package's history would refuse every future release of every package.
- warn when a release marker exists but points at another commit.

Note what is NOT enforceable here: a workflow_dispatch or tag run executes the
workflow definition from the selected ref, so a branch that edits this file can
delete these checks. The boundary has to be a GitHub Environment holding
NPM_TOKEN behind a main-only deployment branch rule. The job now declares that
environment and the header states the required setup; the in-file checks are
defence in depth.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(packages): stop the first-parent check failing on SIGPIPE

Third review round. The blocker this time: `git rev-list --first-parent | grep
-qx` under `set -o pipefail`. grep exits at the first match, rev-list then dies
of SIGPIPE, and the pipeline returns 141, so a commit that IS on main is read
as "not on main" and every real publish is refused. It does not reproduce in a
shallow clone, where main's history fits in the pipe buffer; with the full
history this job checks out it always will. Count matches instead, which reads
the whole stream.

Also from this round:

- identify the CI run by workflow file and triggering event rather than by a
  check-run display name. Names are not unique, so any other workflow or app
  publishing a check called "Lint, Typecheck & Unit Tests" could stand in for a
  red version gate.
- fail the main-push version check when the push range is unusable instead of
  substituting HEAD^. A force push can replace many commits at once, so the
  previous commit is not the range that needs checking, and a green run here is
  what publishing depends on.
- correct the stated limitation. It claimed the package-directory input model
  was exact for dsl and storage; it is not. Their dist is whatever the
  lockfile's TypeScript emits, and dsl's shipped JSON schema comes from the
  lockfile's ts-json-schema-generator, so a toolchain bump can change any of the
  four tarballs with no diff under the package directory.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* refactor(packages): make a merged version bump the only release input

The tag trigger was incoherent. Its own documentation admitted that any
@openmaic/* tag republishes the whole family and that which packages actually
go out is decided by the manifests, so the tag name never participated in the
decision. It was a second entry point to one decision function, it bypassed
review entirely because a tag is not a diff and can be created by anyone with
write access on any commit, and it cannot be guarded: environment deployment
rules match GITHUB_REF, so refs/tags/* and a main-only rule are mutually
exclusive.

Drop it. A version bump that landed on main is now the only release input, and
`@openmaic/<name>@<version>` tags are an output written after a package
reaches the registry. Nothing is lost: a manual dispatch from main still
republishes without a new commit, and that path is inside the protected
environment.

Split the workflow so the token has a boundary. `validate` holds everything
that does not need NPM_TOKEN and runs from any ref, which keeps dry runs
useful to contributors. `publish` is the only job declaring the `release`
environment, so the token is never attached to a run that is merely
validating. It rebuilds rather than sharing state, which is the right trade
for a release.

Also add the `actions: read` permission the CI-conclusion query needs; it was
switched from the checks API to the Actions API without updating the scope,
which would have failed with 403 on every real publish.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(contributing): explain when a package version must be bumped

CONTRIBUTING said nothing about versioning or releases, while this branch adds
a CI check that fails a contributor's PR with "publishable package inputs
changed but version did not increase". A gate that rejects work without
telling anyone the rule is the kind that ends up disabled.

State the rule, the exact failure message, which files are exempt, and how to
choose the number, with a specific warning for @openmaic/dsl: it is the
contract the other packages validate against, so narrowing what an existing
document may contain is breaking even when the diff is small.

Also state what contributors do NOT do: publishing is automatic once a bump
lands on main, and the release tag is a marker written afterwards rather than
something to push.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(packages): run renderer's tests before publishing renderer

The pre-publish gate ran renderer's typecheck but never its test suite, so the
one package whose behaviour is hardest to typecheck was the one shipping
unexercised. It has 25 test files and 242 tests; they just were not in the
filter list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(packages): keep the git credential away from package code

Review finding: the publish job held `contents: write` while
actions/checkout had persisted that credential, and it then ran
`pnpm install` and the package builds. Any dependency or build script
executed with a credentialed git remote in reach and a write scope to use
it, which is a wider blast radius than publishing needs.

Separate the two capabilities so no job holds both:

- `validate` and `publish` run package code. Both now check out with
  `persist-credentials: false`, `validate` is pinned to `contents: read`,
  and `publish` drops write scope entirely, keeping only the `actions: read`
  it needs for the CI conclusion and `id-token: write` for provenance.
- a new `mark` job is the only holder of `contents: write` and a git
  credential, and it installs nothing and builds nothing.

`mark` also reconciles instead of only recording this run. A marker whose
push failed previously could never be repaired before, because the release
plan excludes versions already on the registry and the marker logic sat
behind that plan. It now marks any registry version whose tag is missing,
and leaves an existing tag alone rather than moving it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

2026-07-29 23:54:23 -04:00

7.9 KiB

Raw Permalink Blame History

Contributing to OpenMAIC

Thank you for your interest in contributing to OpenMAIC! This guide will help you get started and ensure a smooth collaboration.

How to Contribute

Contribution type	What to do
Bug fix	Open a PR directly (link the issue if one exists)
Extending existing features (e.g. adding a new model provider, new TTS engine)	Open a PR directly
New feature or architecture change	Start a GitHub Discussion or ask in Discord before opening a PR
Design / UI change	Discuss in a GitHub Discussion or Discord first — include mockups or screenshots
Refactor-only PR	Not accepted unless a maintainer explicitly requests it
Documentation	Open a PR directly
Question	Ask in Discord

Claiming Issues

To avoid duplicate effort, please comment on an issue to claim it before you start working. A maintainer will assign you.

If no PR or meaningful update (WIP commit, progress comment) appears within 1 day, the issue may be reassigned to someone else.
If you see an issue already assigned, reach out to the assignee first to coordinate — you may be able to collaborate or split the work.
If you can no longer work on a claimed issue, please leave a comment so others can pick it up.

Prerequisites

Node.js >= 20.9.0
pnpm (latest)
A copy of .env.local — see .env.example for reference

Getting Started

# Clone the repository
git clone https://github.com/THU-MAIC/OpenMAIC.git
cd OpenMAIC

# Install dependencies
pnpm install

# Set up environment variables
cp .env.example .env.local
# Edit .env.local with your API keys

# Start the development server
pnpm dev

Development Workflow

Fork the repository and create a branch from main:
```
git checkout -b feat/your-feature main
```
Branch naming convention:
- feat/ — new features or enhancements
- fix/ — bug fixes
- docs/ — documentation changes
Make your changes and test locally.
Run all CI checks before committing (see below).
Open a Pull Request against main.

Before You Submit a PR

Run the following checks locally — CI will run them too, but catching issues early saves everyone time:

# 1. Format code
pnpm format

# 2. Lint (with auto-fix)
pnpm lint --fix

# 3. TypeScript type checking
npx tsc --noEmit

If formatting or lint auto-fixes produce changes, include them in your commit.

Local Testing

Before marking a PR as Ready for Review, you must:

Verify your goal — confirm that the PR achieves what it set out to do (bug is fixed, feature works as expected, etc.)
Regression test — manually check that existing functionality is not broken by your changes (e.g. navigate key flows, verify related features still work)
Run CI checks locally (see above)

If you have not completed local verification, keep your PR in Draft status. Only move it to Ready for Review once you are confident it works and does not regress other features.

PR Guidelines

Every PR must link to an issue — use Closes #123 or Fixes #456 in the PR description. If no issue exists yet, create one first. PRs without a linked issue will not be reviewed.
Keep PRs focused — one concern per PR; do not mix unrelated changes
Describe what and why — fill out the PR template
Include screenshots — for UI changes, show before/after
Ensure CI passes before requesting review
All UI text must be internationalized (i18n) — do not hardcode user-facing strings

Commit Message Convention

We follow Conventional Commits:

<type>(<scope>): <short description>

[optional body]

[optional footer]

Types: feat, fix, docs, refactor, test, chore, ci, perf, style

Examples:

feat(tts): add Azure TTS provider
fix(whiteboard): prevent canvas from resetting on window resize
docs: add CONTRIBUTING.md

Changing a Published Package

Four packages under packages/@openmaic/ are published to npm: dsl, storage, renderer, and importer. Anything that ships inside one of those tarballs is under version control in the literal sense — the version number on npm has to keep meaning "this exact source".

If your PR changes a publishable file in one of those packages, bump that package's version in its package.json in the same PR. CI enforces this, and without the bump you will see:

<package>: publishable package inputs changed but version did not increase

What counts as publishable: everything under the package directory except files that never reach the tarball, such as docs/, test/, and vitest.config.ts. Editing only those needs no bump. The exact set lives in scripts/check-package-version-bumps.mjs.

Choosing the number is a semver judgement, and it is yours to make rather than something CI can infer:

patch — a fix that changes no documented behaviour
minor — new behaviour that existing consumers can ignore
major — anything an existing consumer must react to

Be deliberate with @openmaic/dsl. It is the contract the other packages and downstream deployments validate against, so a change that narrows what an existing document may contain is a breaking change even when the diff looks small.

You never publish anything yourself. Once your PR is merged, a version that is not yet on the registry is released automatically, and a @openmaic/<name>@<version> tag is written afterwards to record it. That tag is a marker, not a trigger: pushing one does not release anything.

AI-Assisted PRs 🤖

PRs built with AI tools (Codex, Claude, Cursor, etc.) are welcome! We just ask for transparency and self-review:

Mark it — note in the PR title or description that the PR is AI-assisted
AI-review your own code first — before requesting maintainer review, run an AI code review (e.g. Claude, Codex, Copilot) on your changes and address the findings. This is required for AI-assisted PRs to avoid dumping large amounts of unreviewed generated code on maintainers.
You are responsible for what you submit — understand the code, not just the prompt.

AI-assisted PRs are held to the same quality standard as any other PR. Community members are also encouraged to leave constructive feedback on any PR — peer review helps everyone improve.

Project Structure

OpenMAIC/
├── app/              # Next.js app router pages and API routes
├── components/       # React components
├── lib/              # Shared utilities and core logic (i18n in lib/i18n/locales/)
├── packages/         # Internal packages (mathml2omml, pptxgenjs)
├── public/           # Static assets
└── .github/          # Issue templates, PR template, CI workflows

Reporting Bugs

Use the Bug Report issue template. Include:

Steps to reproduce
Expected vs. actual behavior
Browser / OS / Node version
Screenshots or error logs if applicable

Requesting Features

Use the Feature Request issue template. For larger features, please open a Discussion first.

Security Vulnerabilities

Please report security vulnerabilities through GitHub Security Advisories. Do not open a public issue for security vulnerabilities.

License

By contributing to OpenMAIC, you agree that your contributions will be licensed under the MIT License.

7.9 KiB Raw Permalink Blame History