mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
DottaandPaperclip
7b73b08250
## Thinking Path > - Paperclip is the open source app that people use to manage AI agents for work. > - Paperclip applies database migrations in numeric order. > - Two branches can create the same migration number before either branch merges. > - The existing repository check can find duplicates only after both histories are present in one checkout. > - A pull request must compare its new migrations with the target branch before merge. > - This pull request adds that comparison to the existing PR policy job. > - The benefit is an early failure with exact renumbering instructions. ## Linked Issues or Issue Description **What existing behavior does this improve?** The PR policy check for files in `packages/db/src/migrations`. **Current behavior** A stale branch can add the same migration number as the target branch. The existing check does not compare PR additions with the target branch migration tip. **Proposed behavior** The policy job fails when a new PR migration number is not greater than every migration on the target branch. The error names the conflict, the next safe number, and the related files to update. **Reason and benefit** This prevents duplicate or out-of-order migration numbers from reaching `master`. It also gives contributors and agents a direct repair procedure. **Breaking changes** None. The change rejects migration numbering that is already unsafe. ## What Changed - Added a dependency-free check that compares new PR migration files with the target branch tip. - Added the check to the existing PR policy job. - Added tests for no-op, valid, duplicate, and lower-number cases. ## Verification - `node --test '.github/scripts/tests/*.test.mjs'` passed 133 tests. - `pnpm -r typecheck` passed. - `pnpm build` passed. - `pnpm test:run` passed 4,889 tests and failed 24 unrelated macOS path and wildcard-listener tests that also affect the current `master` checkout. ## Risks - Low risk. The check reads Git history and does not modify migrations. - The check permits gaps. It only requires each new migration number to follow the target branch tip. - The existing migration check continues to validate duplicate numbers, snapshots, and journal entries inside the PR. ## Model Used OpenAI Codex, GPT-5. The exact serving model ID and context-window size are not exposed in this session. Reasoning, tool use, web access, and code execution were enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
44 lines
1.6 KiB
JavaScript
44 lines
1.6 KiB
JavaScript
import { test } from 'node:test';
|
|
import assert from 'node:assert/strict';
|
|
import { checkMigrationOrder } from '../check-pr-migration-order.mjs';
|
|
|
|
const migration = (name) => `packages/db/src/migrations/${name}.sql`;
|
|
|
|
test('passes when a PR has no new migrations', () => {
|
|
const result = checkMigrationOrder([migration('0230_on_master')], []);
|
|
|
|
assert.equal(result.passed, true);
|
|
});
|
|
|
|
test('passes when every PR migration follows the target branch', () => {
|
|
const result = checkMigrationOrder(
|
|
[migration('0230_on_master')],
|
|
[migration('0231_first_in_pr'), migration('0232_second_in_pr')],
|
|
);
|
|
|
|
assert.equal(result.passed, true);
|
|
});
|
|
|
|
test('fails with renumbering guidance when a PR reuses the target branch number', () => {
|
|
const result = checkMigrationOrder(
|
|
[migration('0230_on_master')],
|
|
[migration('0230_from_stale_branch')],
|
|
);
|
|
|
|
assert.equal(result.passed, false);
|
|
assert.match(result.message, /already contains migrations through .*0230_on_master\.sql/);
|
|
assert.match(result.message, /renumber this PR's migrations starting at 0231/);
|
|
assert.match(result.message, /meta\/_journal\.json/);
|
|
});
|
|
|
|
test('fails when a PR inserts a migration before the target branch tip', () => {
|
|
const result = checkMigrationOrder(
|
|
[migration('0230_on_master')],
|
|
[migration('0229_from_stale_branch'), migration('0231_valid_but_after_stale')],
|
|
);
|
|
|
|
assert.equal(result.passed, false);
|
|
assert.match(result.message, /0229_from_stale_branch\.sql/);
|
|
assert.doesNotMatch(result.message, /- packages\/db\/src\/migrations\/0231_valid_but_after_stale\.sql/);
|
|
});
|