mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 21:05:21 +02:00
19c76bfc3feed589639084efa8d3c780753d7fd3
24
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
19c76bfc3f |
fix(ci): avoid empty pnpm caches from lockfile refresh (#13267)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - CI installs dependencies before it verifies and builds cloud artifacts. > - Install jobs share a pnpm package-store cache with lockfile refresh. > - Lockfile refresh resolves versions without downloading packages. > - That job saved an empty cache before full install jobs could save theirs. > - This PR prevents lockfile refresh from publishing that empty entry. > - Full install jobs can then populate the cache and reuse dependencies. ## Linked Issues or Issue Description Refs #13259 for the related cloud verification cache work. No duplicate empty-cache fix was found. **What happened?** Refresh Lockfile run 34517514932 saved a 216-byte default-branch pnpm cache at 18:58:08 UTC on September 10. Full install jobs still restore that empty entry. The cache API reports 216 bytes for master and about 703 MB for populated entries with the same key and cache version in PR scopes. **Expected behavior** A job that installs dependencies should populate the shared package-store cache. **Steps to reproduce** 1. Run lockfile refresh with a new lockfile cache key. 2. Its resolution-only command leaves the package store empty. 3. The Node action saves the empty archive before a full install finishes. 4. Later jobs report a cache hit but download packages again. **Paperclip version or commit** Observed on master |
||
|
|
bc68312327 |
ci: use reserved AWS capacity for post-merge cloud verification (#13257)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Cloud deployments consume a verified image and exact-source
migrator.
> - An image alone is not deployable until source checks and artifact
checks pass.
> - GitHub-hosted queues delayed those checks and the final readiness
signal.
> - This PR gives trusted master work a separate concurrency allowance
on existing AWS runners.
> - Community PRs and arbitrary source inputs keep the GitHub-hosted
fallback.
## Linked Issues or Issue Description
Refs #13243.
**What existing behavior does this improve?**
Time from a master merge to the Cloud deployable v1 signal.
**Current behavior**
For merge
|
||
|
|
a23ae894a5 |
ci: cache Rust dependencies used by post-merge typecheck (#13259)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Cloud images become deployable only after source verification passes. > - The typecheck job builds the native Runner binary through the server package. > - Fresh runners repeatedly compile Rust dependencies for that binary. > - This PR caches those dependencies for exact-source master verification. > - Workspace code and every typecheck still rebuild or run as before. ## Linked Issues or Issue Description Refs #13243 and #13257. **What existing behavior does this improve?** The typecheck portion of post-merge cloud source verification. **Current behavior** The typecheck job has no Rust dependency cache. An observed release build in this job took 4m 13s, including dependency compilation. **Proposed behavior** Restore dependency build outputs for canonical master pushes with the exact source SHA. Use a separate cache key from the Runner verification job, which builds other profiles. **Reason and benefit** A warm cache should remove roughly 2–3 minutes of dependency compilation from this job. Overall deployment gains depend on the remaining critical path. The first cache population still compiles from scratch. **Breaking changes** None. All checks remain enabled. Non-master callers compile without restoring or saving this cache. ## What Changed - Select the pinned Rust toolchain before the typecheck cache lookup. - Reuse the existing pinned Rust cache action with a typecheck-specific key. - Exclude workspace crates and installed cargo executables. - Test restore/save trust boundaries and document cache behavior. ## Verification - All workflow script tests pass locally, including nine new cache trust/contract cases. - actionlint passes for release-verify.yml. - Full local typecheck and build pass on the same source base (167s and 206s); `pnpm test:run` is still running and is recorded with #13257. This PR changes only the workflow, cache guard tests, and documentation. - All 32 current-head checks are successful or intentionally skipped, including the complete Linux test matrix, build, and Greptile 5/5 with no unresolved findings. Verify cache population and subsequent restore on actual master runs. ## Risks - The first run and any toolchain/dependency invalidation compile from scratch. - Cache restore/save overhead reduces the benefit for small dependency graphs. - Disable the cache step to roll back; the existing uncached build remains valid. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository tools, and code execution. Exact serving model ID and context window are not exposed by this environment. ## 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 — focused change tests pass; full-suite local permission failures are disclosed above - [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 - [x] 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> |
||
|
|
d0b7ba4194 |
ci: route approved master cloud builds to AWS (#13243)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Paperclip Cloud deploys images built from master commits. > - Cloud image builds share GitHub-hosted capacity with other workflows. > - The organization already operates AWS runners through RunsOn Fleet. > - This pull request allows approved master builds to use a dedicated cloud Fleet. > - The benefit is separate build capacity with a quick operator rollback. ## Linked Issues or Issue Description Refs #13189, #13192. **What existing behavior does this improve?** Placement of the Docker cloud build after a master merge. **Current behavior** Every Docker cloud build uses a GitHub-hosted runner. Busy periods delay the job. **Proposed behavior** An operator variable enables the approved cloud Fleet for canonical master pushes and manual master builds. Other events, refs, and repositories use GitHub-hosted runners. ## What Changed - Add a guarded AWS runner selector to the Docker cloud job. - Keep the existing image cache, verification, and publication steps. - Test the selector against master, branch, tag, PR, fork, and disabled contexts. - Document provisioning requirements, placement checks, and rollback. ## Verification - 29 focused Node tests pass for routing, readiness, and disk handling. - The full workflow-script Node suite passes. - `pnpm -r typecheck` passes locally. - The pinned PR routing regression suite passes. The first live PR run assigned 21 jobs to the approved AWS PR group. AWS then reclaimed 16 Spot instances. The failed run is being repeated on GitHub-hosted runners while the Fleet moves to On-Demand. - Actionlint passes with existing shellcheck findings excluded (SC2012, SC2016, SC2129). - `git diff --check` passes. - Greptile reports 5/5 on commit `764d505a41dd2023751c3f361906fa9ea35bf0c6`, with no review threads. - All 30 current-head CI checks pass, including typecheck, build, all server/workspace test shards, Runner verification, and browser tests. Two Storybook checks are intentionally skipped for this change. Run: https://github.com/paperclipai/paperclip/actions/runs/34630550799 - The broader local test/build sequence is still running. This Mac has reported failures in unchanged application suites; their complete Linux CI shards pass. Local targeted workflow tests and typecheck pass. - Both On-Demand Fleets are deployed and healthy. Live master cloud-build verification follows the merge. ## Risks - Missing Fleet capacity or runner-group authorization can leave an AWS job queued. Disable `AWS_CLOUD_BUILDS_ENABLED` and rerun the workflow to use GitHub-hosted capacity. - The runner group must restrict access to this repository and the master version of `docker-cloud.yml`. - Docker needs more disk space than the PR Fleet. Provision 120 GiB disks and retain the free-space check. - This changes image build placement only. Source verification and migrator publication remain separate prerequisites. ## Model Used OpenAI GPT-6 through Codex, with reasoning, tool use, and code execution. The exact serving model identifier and context-window size are not exposed by this environment. ## 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 - [x] 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> |
||
|
|
96bba78fba |
feat: add readable Storybook branch bookmarks (#13231)
## Thinking Path > - Paperclip helps people manage AI agents for work. > - Maintainers use Storybook previews to review the board UI. > - Branch previews need stable bookmarks that people can read. > - The current publisher only provides a hashed branch path. > - This pull request adds a readable branch bookmark after each successful upload. > - Existing branch and build links keep working. ## Linked Issues or Issue Description Refs #13226. **What existing behavior does this improve?** Manual Storybook publication for repository branches. **Current behavior** The stable branch path contains a hash. The expected `/storybook/branches/master/` URL does not exist. **Proposed behavior** Each publication updates a readable bookmark. The action summary and Markdown artifact link it. Master uses `/storybook/branches/master/`. Other names use a safe path segment that preserves case and escapes special characters. **Reason and benefit** Maintainers can save and share a readable URL that opens the latest published branch build. **Breaking changes** None. Existing hashed branch entries still update. Existing build URLs remain valid. **Additional context** This follows the publisher in #13226. A duplicate search found no related bookmark change. It does not overlap planned core work in ROADMAP.md. ## What Changed - Generate readable branch bookmarks without collisions with existing build directories. - Upload the bookmark only after the full build and compatibility entry uploads succeed. - Link the bookmark in the existing summary and Markdown artifact. - Document branch-name escaping and test path isolation, stable links, and upload order. ## Verification - `node --test scripts/__tests__/storybook-deploy.test.mjs`: 20 tests pass. - `actionlint .github/workflows/storybook-deploy.yml .github/workflows/storybook-visual.yml`: passes. - `git diff --check`: passes. - [Master bookmark publication](https://github.com/paperclipai/paperclip/actions/runs/34613344758): passed. Opened `/storybook/branches/master/` in the browser and confirmed a story renders. Downloaded the Markdown report and verified its bookmark link. - [Feature branch bookmark publication](https://github.com/paperclipai/paperclip/actions/runs/34613449034): passed. Its separate bookmark uses `codex~2Fstorybook-bookmarks`. - Greptile: 5/5 on `dccaf10413ecf447cb34e622b6b3c505791abb51`, with no unresolved review threads. All current-head Paperclip CI gates pass, including typecheck, tests, build, browser suites, and the canary dry run. - Full local repository checks were not repeated for this focused publisher change. The preceding run passed typecheck but encountered unrelated native-session test failures. ## Risks - Special characters in branch names use `~HH` byte escapes. For example, `feature/foo` becomes `feature~2Ffoo`. - Names that could overlap an existing hashed build directory escape the final hyphen. Very long names retain a hash suffix. - The two branch entries update separately. If the final upload fails, the workflow fails and a rerun can repair the bookmark. ## Model Used OpenAI GPT-6 via Codex, with reasoning, shell tools, and live deployment verification. The exact runtime model ID and context-window size are not exposed in this session. ## 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 - [x] 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> |
||
|
|
a20ecce409 |
feat: publish CODEOWNER-approved Storybook branch previews (#13226)
## Thinking Path > - Paperclip helps people manage AI agents for work. > - Maintainers use Storybook to review the board UI. > - Reviews need public previews of selected repository branches. > - Each branch needs its own URL so previews do not replace each other. > - This pull request adds manual, CODEOWNER-controlled publishing to S3 and CloudFront. > - The action returns stable branch links and permanent build links in its summary and a Markdown artifact. ## Linked Issues or Issue Description **What existing behavior does this improve?** The existing Storybook build and manual visual-review workflow. **Current behavior** The repository has no manual branch-preview publisher. A single GitHub Pages site cannot support independent publishers without combining their output. **Proposed behavior** A CODEOWNER selects a source branch and approves publication. Each branch has a stable CloudFront URL. A completed build becomes the branch target only after its upload succeeds. The action attaches `storybook-deployment.md` with the preview links and source commit. **Reason and benefit** Maintainers can share multiple branch previews at the same time. Branch builds have no repository token permissions or AWS credentials. Dependency caching and install hooks are disabled. The publisher cannot write runner dashboard files or delete objects. **Breaking changes** None. Normal visual checks keep their existing behavior. This does not change application code or GitHub Pages settings. **Additional context** Searched public issues and PRs for Storybook deployment work. No duplicate deployment proposal was found. This is maintainer infrastructure, not a roadmap-level core feature. ## What Changed - Add `Storybook Deploy` with a source-branch input and a manual entry through `Storybook Visual`. - Check the original actor and rerunner against default-branch CODEOWNERS. Require a protected deployment environment with CODEOWNER reviewers. - Separate public-source builds with no repository permissions from an OIDC publisher restricted to the Storybook S3 prefix. - Publish distinct branch URLs and retain build URLs. Preserve Storybook deep links across the branch redirect. - Add the run summary, a downloadable Markdown deployment report, focused tests, and operator setup docs and IAM policies. ## Verification - `node --test scripts/__tests__/storybook-deploy.test.mjs`: 19 tests pass. - `actionlint .github/workflows/storybook-deploy.yml .github/workflows/storybook-visual.yml`: passes. - [Feature branch live publication and deployment-only rerun](https://github.com/paperclipai/paperclip/actions/runs/34533202273): passed. - [Master branch live publication](https://github.com/paperclipai/paperclip/actions/runs/34533204743): passed. - Both public branch URLs render a component story without browser errors. A deployment-only rerun updates only the selected branch entry and preserves the previous build URL. - AWS policy simulation allows Storybook uploads and denies dashboard writes and object deletion. - Full local typechecking passes. Full local tests, build, and current-head PR checks are running. - [Revised build and Markdown artifact validation](https://github.com/paperclipai/paperclip/actions/runs/34605623088): passed. Downloaded the report and verified its branch URL, build URL, and source commit. - The public verifier also checks that the stable branch URL points to this build and rejects stale targets. ## Risks - Storybook previews are public. Maintainers must publish only public UI fixtures. - Retained builds accumulate until an operator prunes them. - Environment reviewers must stay synchronized with CODEOWNERS. The workflow fails closed if its environment loses required protection. - The existing CloudFront distribution is shared with runner reports. Separate S3 prefixes and a dedicated role prevent the publisher from overwriting those reports. ## Model Used OpenAI GPT-6 via Codex, with reasoning, shell tools, and browser verification. The exact runtime model ID and context-window size are not exposed in this session. ## 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 - [ ] 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> |
||
|
|
d56be3f3fc |
fix(ci): verify deployable cloud artifacts independently (#13192)
Verify source, build the cloud image, and wait for exact-source migrator packages concurrently. Emit Cloud deployable v1 only when every prerequisite succeeds for the merged full SHA. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
6a7025ebe3 |
ci: skip cloud runner cleanup when disk headroom is ample (#13191)
Skip cloud runner disk cleanup when both the Docker and workspace filesystems have at least 64 GiB free. Preserve the existing cleanup for low, unavailable, or invalid measurements and verify the actual shell behavior across eight scenarios. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
5c660a32f3 |
ci: build cloud images independently for each merge (#13189)
Build cloud images independently for each master commit through a reusable workflow. Preserve production release dependencies and image runtime checks, and write cloud registry caches per commit with bounded ancestor imports to prevent overlapping builds from replacing each other's cache. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
9e970df4c5 |
ci: cache Rust dependencies in release Runner verification (#13194)
Cache external Rust dependencies in trusted master release verification after selecting the package-owned toolchain. Keep source compilation and all validation unconditional; restrict both restore and save to the matching master push. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
4ef6155aae |
ci: harden paid runner browser and lock repair (#12829)
## Thinking Path Paid cells now reuse the AWS image's system Chrome, but Playwright video recording still resolves its revision-pinned FFmpeg helper from the Playwright cache. Run 33875618534 proved Chrome qualification succeeds and then failed before provider startup because that helper was absent. The same run also exposed that generic lock repair can churn unrelated package platform metadata, so the automated repair paths need resolution-only regeneration rather than lockfile-only metadata refresh. ## What Changed - install Playwright FFmpeg only on the AWS/system-Chrome path - retry the small helper installation up to three times before provider secrets are exposed - keep the GitHub-hosted Chromium fallback unchanged - bind static coverage to the exact FFmpeg step block and its pre-secret ordering - add pnpm `--resolution-only` to all four automated lock-repair paths while retaining full transitive resolution - require resolution-only repair in the shared workflow regression The actual generated lockfile correction remains bot-owned by PR #12828 and is intentionally not committed here. ## Verification - `node --test .github/scripts/tests/lockfile-refresh-workflows.test.mjs` - `actionlint -ignore SC2012` on all modified workflows - focused Prettier checks - `git diff --check` - prior run 33875618534: system Chrome 151 qualified; missing Playwright FFmpeg was the sole cell startup failure ## Risks Low. The new network operation is limited to Playwright's pinned FFmpeg payload, happens before paid credentials are exposed, and leaves the hosted-runner path unchanged. Resolution-only is still a full dependency-resolution pass, unlike lockfile-only, while avoiding unrelated current-platform metadata churn. ## Model Used GPT-5 |
||
|
|
fa16f88d6b |
chore(lockfile): refresh pnpm-lock.yaml (#12771)
Use full dependency resolution in automated lockfile repair paths, add regression coverage, and refresh the stale Rollup snapshot. Co-Authored-By: Dotta <cryppadotta@users.noreply.github.com> Co-Authored-By: Codex <codex@openai.com> Co-Authored-By: lockfile-bot <lockfile-bot@users.noreply.github.com> |
||
|
|
7b73b08250 |
ci: enforce migration order against PR target (#12433)
## 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> |
||
|
|
24913064ff |
feat(commitperclip): surface the Co-Authored-By trailers a squash merge needs (#11498)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work, and it takes contributions from outside the core team > - Those contributions arrive as PRs, and this repository squash-merges every one of them > - A squash collapses the whole branch into a single commit authored by whoever pressed the button > - So when a maintainer rebases and lands a contributor's stale PR, the contributor's name survives only if the squash message carries a `Co-Authored-By` trailer > - Nothing prompts for that trailer, and the PR page keeps showing the original author either way, so losing it is invisible at the moment it happens > - This pull request has commitperclip detect the situation and print the exact trailers to paste > - The benefit is that keeping an outside contributor's name is a default rather than something a maintainer has to remember ## Linked Issues or Issue Description No public issue exists. The problem follows, and it is not hypothetical. **What happened?** #11370, #11371 and #11379 landed @stubbi's work yesterday. Each of those PRs carries a comment from me telling them their authorship would be preserved. All three squash commits went in without a `Co-Authored-By` trailer, so `git log` credits none of them: | commit | landed from | credited | | --- | --- | --- | | `66515582e` | #9900 | Claude only | | `bc0b5a164` | #9501 | Claude only | | `35a9b9873` | #8982 | Claude only | | `6542ad1f4` | #11259 | ✅ Jannes Stubbemann + Claude | The last one has the trailer because that message was written by hand with the contributor in mind. The only difference between the two outcomes was memory. Master history cannot be rewritten, so those three are now credited by comment on the original PRs — which is a worse record than a commit trailer, and the reason to make this automatic. **Expected behavior** When a branch carries commits by someone other than the PR author, the merger is told what trailers the squash needs. **Paperclip version or commit** `master` at `92047cac4`. ## What Changed - `.github/scripts/check-pr-coauthors.mjs` — new gate. - `.github/scripts/run-quality-gates.mjs` — fetches the PR's commits and runs it. - `.github/scripts/tests/check-pr-coauthors.test.mjs` — 12 cases. - `.github/workflows/pr.yml` — runs `.github/scripts/tests/`. ### Informational, not a failure The squash message does not exist while the PR is open. This can neither be verified there nor fixed there, so failing a PR on it would block work on something its author cannot satisfy. The gate notices that the situation applies and prints the lines to paste. Run against #11370's actual commits it produces exactly what was missing: ``` This branch carries commits by stubbi. Squash-merging drops that authorship unless the squash message carries their trailers, and nothing else will notice if it does not. Add to the squash body when merging: Co-Authored-By: Jannes Stubbemann <stubbi@users.noreply.github.com> ``` ### Edge cases it handles Bots skipped; the PR author's own commits skipped; logins compared case-insensitively (`PR_AUTHOR` does not always arrive in the same case as the commit author login); each contributor listed once however many commits they wrote; and a commit GitHub could not match to an account falls back to its raw git author — that identity being the one most likely to be lost, not least likely. Paging stops at the API's own 250-commit ceiling rather than spinning on full pages of nothing new. ### The test directory was not running `.github/scripts/tests/` held ten test files covering the existing gates, and no workflow ran any of them. Adding an eleventh would have meant adding a test that never executes, so `pr.yml` now runs the directory. All **149** pass, including the 137 that were already there and previously unverified in CI. ## Verification - 149 tests pass via `node --test '.github/scripts/tests/*.test.mjs'` — the exact command CI now runs. - The gate was run against the real commit shape from #11370 and produces the missing trailer verbatim. This PR is its own negative control: the branch carries only my commits, so the new gate should stay silent on it. If commitperclip prints a co-author note below, the gate is wrong. ## Risks Low. Informational output only — it cannot fail a PR, and `allPassed` is unchanged. It adds one API call per gate run (`/pulls/{n}/commits`), fetched in the same `Promise.all` as the existing PR and files calls. Enabling the previously-unrun test directory could in principle surface a pre-existing failure; all 149 pass locally, so it does not. Revert the commit to restore. ## Model Used Claude Opus 5 (`claude-opus-5`), through Claude Code. Extended thinking enabled. Tool use enabled: file read and edit, shell for test runs. ## 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 - [x] My branch name describes the change and contains no internal ticket id - [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 - [x] 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: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
fd106c6fa6 |
ci(commitperclip): remove the security gate that filed a draft advisory per PR (#11828)
## Thinking Path > - Paperclip's `commitperclip-review` workflow runs `check-pr-security.mjs` on every PR and files a **draft security advisory** whenever one of its heuristics fires (#6469). > - The heuristics fire on most ordinary PRs: any change under `server/src/routes/agents.ts` / `companies.ts` / `approvals.ts` / `authz.ts` / `MarkdownBody.tsx`, any test file containing `fetch(` / `exec(` / `process.env.X`, any `key: "<20+ chars>"` string (it flagged `pluginKey: "paperclipai.plugin-llm-wiki"`), any touch of `.github/workflows/`. > - The repository now holds **1,566 commitperclip-authored draft advisories** against ~99 human-reported ones, burying the reports that matter under the 🔒 Security tab. > - Nothing consumes them: no code reads the drafts, nothing reads the `security-review` check run the script also posts, and `master` has no required status checks. The "Review and dismiss if not a real concern" footer assumed a human triage loop that never existed. > - A second bug made it worse: the advisories endpoint is cursor-paginated and ignores `page=`, so `findExistingDraftAdvisory` only ever saw the newest 100 drafts and re-flagged PRs got a second draft (1,566 drafts for 1,386 distinct PRs; 136 PRs have 2+). > - Removing the gate stops the flood at the source; the quality gates and Dependency Review carry on unchanged. ## Linked Issues or Issue Description **Problem:** `check-pr-security.mjs` files a draft security advisory for nearly every PR, flooding the repository's advisory list with bot-authored noise that no one reads. Human-reported advisories in `triage` state are buried among ~1,560 `🚨 Security flag — PR #NNNN` drafts. **Expected:** the advisory list contains only real vulnerability reports. Heuristic PR checks, if wanted at all, do not create disclosure records. ## What Changed - Deleted `.github/scripts/check-pr-security.mjs` and `.github/scripts/tests/check-pr-security.test.mjs`. - Removed the `Run security gates` step from `.github/workflows/commitperclip-review.yml`, and the `security-events: write` permission that only it used. - No other script imports from the removed module (`resolveBaseRef` lives in `check-pr-dependencies.mjs` and stays). ## Verification - `node --test .github/scripts/tests/*.test.mjs` → 114 pass, 0 fail. - `grep -rn check-pr-security .github` → no remaining references. - The 1,563 existing bot drafts are being closed out-of-band via the API (there is no delete endpoint for advisories). ## Risks - Low. The only behaviour removed is the draft-advisory filing and the informational `security-review` check run, neither of which is consumed by code or branch protection. - Recommended follow-up for an org admin: drop `security_advisories: write` from the commitperclip App's permissions so no workflow can recreate this. ## Model Used Claude Fable 5 (claude-fable-5) via Claude Code, with tool use: GitHub API reads, file edits, local test runs. ## 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 run tests locally and they pass - [x] I have considered and documented any risks above 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016HitAcRu3NW5YDeBxXxePi --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
b5a3a863c3 |
feat(release): bootstrap new npm packages with a placeholder publish (#11757)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Its release pipeline publishes a set of npm packages from CI with npm trusted publishing (GitHub OIDC), gated by `scripts/release-package-manifest.json` > - A brand-new package name cannot be published by CI directly: the PR bootstrap gate requires the name to resolve on npm, and a trusted-publisher rule can only be configured after the package page exists > - The current bootstrap helper closes that gap by building the package locally and publishing its real output from a maintainer machine — before the PR that adds the package has passed CI or review > - This pull request replaces that flow: the helper now publishes a minimal deprecated placeholder at version `0.0.0` that only reserves the name, so every real version ships from CI > - The benefit is that unreviewed build output never reaches npm, and the bootstrap runs from any checkout (including `master`, before the new package's PR merges) with no local build ## Linked Issues or Issue Description **What existing behavior does this improve?** The one-time npm bootstrap for a brand-new release package (`pnpm run release:bootstrap-package`). **Current behavior** The helper builds the target package locally and publishes the real build output from a maintainer machine. That content has not passed repository CI or review at publish time. The helper also requires the new package to exist in the local workspace, so it must run from the (unmerged) PR branch that adds the package. **Proposed behavior** The helper publishes a three-file placeholder at version `0.0.0` (manifest, README, and an `index.js` that throws a descriptive error), waits for the registry to show the package, then deprecates it. The PR bootstrap gate (`scripts/check-release-package-bootstrap.mjs`) only requires the name to resolve on the registry, so the placeholder satisfies it. The first real calver release from CI supersedes the placeholder, and a stable release moves `latest` off it — the same `latest` window that existed under the old flow, but containing an explicit inert stub instead of unreviewed code. **Reason and benefit** Real package content only ever reaches npm from CI, after review and merge. The bootstrap becomes safer (scope guard refuses names outside `@paperclipai/`, already-published names are rejected) and simpler (no local build, no workspace state, runs from any checkout). **Breaking changes** None at runtime. The helper's CLI surface changes: it now takes a package name only (no directory selector) and drops `--skip-build`. `doc/PUBLISHING.md` is updated to match. ## What Changed - `scripts/bootstrap-npm-package.mjs`: replaced the build-and-publish flow with a placeholder publish — stages `package.json` + `README.md` + throwing `index.js` at version `0.0.0` in a temp directory, previews with `npm publish --dry-run`, and publishes only with `--publish`. One-time passwords are prompted interactively (never passed as arguments, since they are single-use and would land in shell history), with re-prompt on a rejected or expired code. After publishing, the helper polls the registry until the package is visible (a first publish can lag by minutes; verified live at ~5 minutes), requiring two consecutive sightings before prompting for a second code and deprecating the placeholder so accidental installs warn loudly; on timeout or failure it prints the exact manual `npm deprecate` command. Added an `@paperclipai/`-scope guard and a fail-fast error when `--publish` runs without an interactive terminal. Removed the workspace-plan dependency so it runs from any checkout. - `scripts/bootstrap-npm-package.test.mjs`: rewrote for the new interface — argument parsing, scope validation, the generated placeholder files (manifest shape, throwing entry point, README), the OTP re-prompt loop, and the registry poll (consecutive-sighting requirement, timeout, transient-error tolerance) via injected fakes. - `doc/PUBLISHING.md`: rewrote the "One-time bootstrap sequence for a new package" section for the placeholder flow, including the `latest` dist-tag window and the trusted-publishing setup ordering (placeholder publish → trusted publisher rule → `"publishFromCi": true`). - `.github/scripts/check-pr-release-bootstrap.mjs` (+ test, + wiring in `run-quality-gates.mjs`): new informational commitperclip notice on PRs that need this bootstrap. It fires when the PR newly release-enables a package that is missing from npm, or adds an unpublished `publishFromCi: false` package that published packages declare a `workspace:*` dependency on, and names the exact maintainer command — so contributors know the red `policy` check is not theirs to fix. It never fails the gate (the `policy` job remains the enforcer), only looks up scope-validated names on the registry, and stays quiet on registry errors. ## Verification - `node --test scripts/bootstrap-npm-package.test.mjs`: 13/13 pass - `node --test .github/scripts/tests/*.test.mjs`: 147/147 pass (10 new for the PR notice) - `pnpm run test:release-registry`: 82/82 pass - Replayed the new PR notice against a real historical PR's live API data (files, manifest at base and head refs): with the registry in its pre-bootstrap state it produces the exact maintainer instruction; with the package bootstrapped it stays silent - Full live end-to-end run: the flow bootstrapped `@paperclipai/adapter-kimi-local` for real — dry-run preview (634-byte, 3-file tarball), publish, registry visibility after ~5 minutes of propagation lag, deprecation confirmed via `npm view ... deprecated` - Guards verified live: an already-published name is rejected, an out-of-scope name (`left-pad`) is rejected, unknown options (including the removed `--otp`) are rejected, and `--publish` in a non-interactive shell fails fast before any network call ## Risks - The `latest` dist-tag points at the deprecated `0.0.0` placeholder until the first stable release supersedes it. This window also existed under the old flow (which parked `latest` at a locally built version); internal consumers are unaffected because release version rewrites pin exact calver versions. - The registry poll caps at ~10 minutes. If propagation is slower than that, the helper prints the exact `npm deprecate ... --otp <code>` command to run manually once `npm view` resolves. - The helper no longer validates the name against the workspace release plan, so a typo within the `@paperclipai/` scope would reserve a wrong name. The dry-run preview shows the exact name before any publish. ## Model Used - Anthropic, **Claude Fable 5** (`claude-fable-5`) via Claude Code, with repository, shell, and Git tooling. It analyzed the existing bootstrap flow and the release scripts (`release-package-map.mjs`, `check-release-package-bootstrap.mjs`, `release.sh` dist-tag handling), wrote the replacement script and tests, updated the documentation, and ran the verification above. ## 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 - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
131d476a7e |
fix(ci): make PR-template inline-description contract explicit (#10558)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Contributors use the PR template to describe changes before review > - The linked-issue gate reads literal template labels, not freeform prose > - The template did not state that rule, so a good prose summary could fail on first submission > - The gate also skipped two issue template paths that the PR template already points to > - This pull request makes the template and the gate match the same contract > - The benefit is that a good-faith author can pass the check on the first try ## Linked Issues or Issue Description No public issue exists. Related public PR: #7762. The problem is a mismatch between the PR template and the linked-issue gate. The template gave a bare placeholder and did not explain the literal-label rule. The gate also missed the enhancement and docs issue templates. ## What Changed - Replaced the bare PR-template placeholder with labeled inline skeletons for bug, feature, and adapter paths. - Added one sentence that says the gate reads literal labels on separate lines. - Added the enhancement and docs issue template field sets to the linked-issue gate. - Added and updated tests for prose-only bodies, template skeleton bodies, and extra issue template coverage. ## Verification - `node --test .github/scripts/tests/check-pr-linked-issue.test.mjs` - The branch contains one commit: `561f1ed3a1434ed4562306f74d40968163ef1444` ## Risks Low risk. The three-field minimum stays in place. The main change is clearer author guidance in the PR template. ## Model Used OpenAI Codex, GPT-5, tool use. ## 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 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 - [x] 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> |
||
|
|
4518f272b2 |
fix(release): publish all remaining @paperclipai workspace packages from CI + guard unpublishable edges (#8365)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work, distributed primarily via the `paperclipai` npm package and its `@paperclipai/*` workspace packages. > - The release subsystem (`scripts/release-package-*`) decides which workspace packages CI republishes at the unified calver version each release, and `replaceWorkspaceDeps()` rewrites every internal `workspace:` dependency to that calver version at publish time. > - The gap: a package that publishes from CI can declare a `workspace:` dependency on a package that is NOT enrolled for CI publish. The dependency spec gets rewritten to a calver version that is never actually published, so the dependent becomes uninstallable. > - This shipped for real: a recent change made `@paperclipai/server` depend on `@paperclipai/skills-catalog`, but skills-catalog was not on the calver release train — so canary builds after that merge failed to resolve `@paperclipai/skills-catalog@<calver>` and `npx paperclipai@canary run` broke. > - This PR addresses it durably by (a) putting every remaining internal package on the CI release train so no internal dependency can dangle, and (b) adding a fail-fast guard so this class of break can never ship again. > - The benefit is that canary and stable installs resolve all internal dependencies, and any future unpublishable workspace edge fails the release build with a clear, named error instead of producing a broken package. ## Linked Issues or Issue Description Refs #8327 (introduced the `server -> skills-catalog` runtime dependency that surfaced the gap). No public issue tracks this; describing it in-PR: - **Problem:** After #8327, `npx paperclipai@canary run` failed for builds past the merge because `@paperclipai/skills-catalog` was rewritten to a calver version that was never published (the package was not enrolled for CI publish). Versions before the merge still ran. - **Root cause:** A `publishFromCi:true` package can declare a `workspace:` dependency on a package that is not `publishFromCi:true`; the release-time version rewrite then points at a non-existent published version. ## What Changed - Enrolled every remaining internal package on the calver release train by setting `publishFromCi: true` in `scripts/release-package-manifest.json`: `skills-catalog`, `teams-catalog`, `plugin-workspace-diff`, `plugin-kubernetes`, `plugin-novita-sandbox`. There are now zero `publishFromCi:false` entries. - `skills-catalog`, `teams-catalog`, `plugin-workspace-diff` already existed on npm — CI simply republishes them at calver. - `plugin-kubernetes` and `plugin-novita-sandbox` were not on npm; their one-time first publish was bootstrapped so the `check-release-package-bootstrap` gate passes. - Added `findUnpublishableWorkspaceEdges()` to `scripts/release-package-map.mjs`, wired into `buildReleasePackagePlan()`. The release map build now fails fast (surfaced by the `check` CI already runs) whenever a `publishFromCi:true` package declares a runtime `workspace:` dependency (`dependencies`/`optionalDependencies`/`peerDependencies`) on a non-`publishFromCi:true` `@paperclipai/*` package, naming the offending edge. - Added tests covering positive/negative detection, all three dependency sections, unknown-package edges, off-train edges, and the live manifest. ## Verification - `node --test scripts/release-package-map.test.mjs` → 9/9 pass (includes a test asserting the live manifest has no unpublishable edges). - `node scripts/check-release-package-bootstrap.mjs scripts/release-package-manifest.json` → passes, naming all five newly-enabled packages (all confirmed present on npm). - Confirmed via `npm view` that all five packages resolve on the public registry. ## Risks - Low risk. The change only enrolls already-existing (or freshly-bootstrapped) packages onto the existing release train and adds a build-time validation. No runtime code paths change. The new guard can only *fail* a release that was already going to ship a broken package. ## Model Used Claude Opus 4.7 (claude-opus-4-7), extended thinking, with tool use / code execution. ## 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 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] If this change affects the UI, I have included before/after screenshots (N/A — no UI changes) - [x] I have updated relevant documentation to reflect my changes (N/A — no doc-facing behavior change) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
47bd02647c |
fix(commitperclip): stop security gate from hanging the review check (#7847)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The commitperclip review workflow runs a security gate as part of CI on every PR > - The security script's header promises it always exits 0 and stays silent/informational, but PRs that triggered a flag were failing with a 5-minute timeout > - Two compounding bugs: `findExistingDraftAdvisory` paginated without an upper bound, and the workflow step did not have `continue-on-error: true`, so any hang inside the script turned into a hard `review` check failure that blocked merge > - This pull request caps the advisory pagination at 20 pages and adds `continue-on-error: true` to the workflow step, aligning runtime behavior with the script's documented "always exit 0" contract > - The benefit is that future PRs flagged by the security gate no longer block merge on a 5-minute timeout, and the gate stays silent/informational as intended ## Linked Issues or Issue Description Fixes: #7849 ## What Changed - `.github/workflows/commitperclip-review.yml`: added `continue-on-error: true` to the `Run security gates` step so a hang or non-zero exit cannot fail the `review` check (matches the script's documented "always exit 0" contract). - `.github/scripts/check-pr-security.mjs`: capped `findExistingDraftAdvisory` pagination at 20 pages (= 2000 advisories) and short-circuited with a `console.warn` when the cap is hit; if no match is found within the cap, callers will simply create a new draft instead of hanging forever. - `.github/scripts/tests/check-pr-security.test.mjs`: added a test asserting the pagination cap is enforced. ## Verification - `node .github/scripts/tests/check-pr-security.test.mjs` — 31/31 pass, including the new cap test. - Step-level guarantee: `continue-on-error: true` makes the `Run security gates` step non-blocking for the job, so even an unexpected hang/timeout in this step can no longer fail the `review` check. ## Risks - Low risk. Pagination cap is a defensive bound; the worst case is a duplicate draft advisory (acceptable — the workflow continues). `continue-on-error: true` is exactly what the script header already promised; the workflow now matches its stated contract. ## Model Used - Claude (claude-opus-4-7), extended thinking, tool use ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [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 - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
d5889919a9 |
feat(commitperclip): widen linked-issue gate, add dedup-search check (#7632)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Contributor onboarding leans on a small set of CI gates
("commitperclip") that read PR metadata and enforce the rules in
CONTRIBUTING.md
> - We recently landed two new contributor rules: every PR must either
link an existing issue or inline an issue-template-shaped description,
and the author must affirm they searched for duplicate PRs first
> - The existing `check-pr-linked-issue` gate only accepted
`Fixes/Closes/Resolves #N`, and there was no gate at all for the
dedup-search affirmation, so the new rules were unenforced
> - This pull request widens the linked-issue gate (accept `Refs #N`,
accept inline template-shaped descriptions) and adds a new dedup-search
gate wired into `run-quality-gates`
> - The benefit is that the rules we ask contributors to follow are now
mechanically enforced, lowering review noise without raising contributor
friction
## Linked Issues or Issue Description
Refs #4260 — the PR cited in the original rule discussion, whose body is
the canonical example of the inline-issue-description shape this gate
now accepts.
**Problem or motivation**
The repo recently adopted two contributor rules: (1) link the issue your
PR fixes or inline a description in the issue-template shape, and (2)
confirm you searched for duplicate PRs before opening one. Neither rule
was enforced — the existing linked-issue check only matched
`Fixes/Closes/Resolves #N`, and nothing looked for the dedup-search
affirmation. Reviewers had to remember and re-state the rules by hand on
every PR.
**Proposed solution**
Widen `check-pr-linked-issue.mjs` and add a new
`check-pr-dedup-search.mjs` so commitperclip enforces the rules already
documented in CONTRIBUTING.md and the PR template:
- Accept `Refs #N` as a valid link verb alongside `Fixes`, `Closes`,
`Resolves`.
- Accept a PR body with an inline issue-template-shaped description (≥3
fields matched against the bug, feature, or adapter templates) as a
valid alternative to a linked issue.
- New `check-pr-dedup-search.mjs` looks for a checked checkbox affirming
the author searched for similar/duplicate/prior PRs, wired into
`run-quality-gates.mjs` with the same skip/override semantics as the
other gates.
**Alternatives considered**
- A single regex over the whole PR body looking for issue-template field
names — rejected, too brittle and gave no useful error message when it
failed. The per-field counter lets us tell the author exactly how many
template fields we matched and which template they're closest to.
- Making the dedup-search check live inside `check-pr-linked-issue` —
rejected, the two rules are orthogonal and a separate gate gives a
cleaner error message and respects `[skip-quality-gates]` independently.
**Roadmap alignment**
This is contributor-workflow plumbing for rules that already landed on
`master`. It is not a roadmap feature and does not overlap with planned
core work.
## What Changed
- `check-pr-linked-issue.mjs`: accept `Refs #N`; add
`hasInlineIssueDescription` (per-template field counter, ≥3 fields) so a
fully inlined description satisfies the gate.
- `check-pr-dedup-search.mjs`: new gate, looks for a checked
dedup-search checkbox in the PR body, with clear failure guidance.
- `run-quality-gates.mjs`: wire the new gate in alongside the existing
checks.
- `CONTRIBUTING.md`: "Before You Start: Search First" callout pointing
at the PR-template checkbox the gate enforces.
- Tests: unit tests for `Refs #N`, the inline-description threshold
(bolded labels, plain labels, headings), and the dedup-search gate
across checked/unchecked/missing/skip-prefix paths.
## Verification
- `node --test .github/scripts/tests/check-pr-linked-issue.test.mjs` →
all pass
- `node --test .github/scripts/tests/check-pr-dedup-search.test.mjs` →
all pass
- Full repo test matrix: 116/116 pass locally
For the gate itself, this PR exercises both new code paths: the body
inlines a feature-template-shaped description with ≥3 matched fields,
and the dedup-search checkbox below is checked.
## Risks
Low risk. The change is contributor-CI plumbing — no runtime or
migration impact. The widened linked-issue gate is strictly more
permissive (it can only flip prior FAILs to PASS), and the new
dedup-search gate honors the existing `[skip-quality-gates]` prefix, so
an author can always bypass it the same way as the other gates if
needed.
## Model Used
Claude Opus 4.7 (`claude-opus-4-7`), extended-thinking enabled, tool use
for filesystem + shell.
## 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 run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] If this change affects the UI, I have included before/after
screenshots
- [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
- [ ] 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>
|
||
|
|
c369d3d357 |
fix: exempt Dependabot PRs from manual-lockfile block and quality gates (#7457)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies, including how we ship the public `paperclip` repo itself > - The `PR` and `commitperclip PR Review` workflows are the CI gating layer that decides whether any pull request — human or bot — can be merged to `master` > - Dependabot opens dependency PRs that always carry a `pnpm-lock.yaml` diff and an auto-generated PR body, but our `policy` job hard-fails any non-`chore/refresh-lockfile` lockfile change, and our `commitperclip` quality gate requires a Thinking-Path / What-Changed / Verification / Risks / Model template Dependabot can't produce > - Because `policy` fails first, every downstream lane (`Build`, `Typecheck + Release Registry`, `General tests`, `Verify serialized server`, `Canary Dry Run`, `e2e`, and the required `verify` check) skips and `verify` fails — so we never see whether the upgrade is actually safe > - Socket.dev (PR Alerts + Project Report) and Snyk already run on every dependency PR and are the supply-chain compensating control against malicious upgrades; the missing piece is just letting our own build/test signal run so a human can merge with confidence > - This pull request adds a narrow Dependabot bypass to the two gates that block on lockfile diffs and PR-template prose, while leaving every other policy and security check active > - The benefit is that Dependabot PRs like #7331 will now run the full PR matrix, giving reviewers real evidence to approve or reject — without weakening any check that targets supply-chain or build-correctness risk ## What Changed - `.github/workflows/pr.yml` — extended the existing `chore/refresh-lockfile` bypass on the `policy` job's "Block manual lockfile edits" step to also skip when `github.actor == 'dependabot[bot]'`. Every other policy step (Dockerfile deps stage validation, `no-git-push` enforcement, release-package map check, release bootstrap, manifest-driven `pnpm install --lockfile-only` resolution) keeps running on Dependabot PRs. - `.github/workflows/commitperclip-review.yml` — gated the `Run quality gates` step and the dependent `Fail if quality gates failed` step on `github.event.pull_request.user.login != 'dependabot[bot]'`. `Run security gates` (`check-pr-security.mjs`) stays unconditional so supply-chain visibility into Dependabot lockfile churn is preserved. No changes to `.github/scripts/*.mjs` — keeping the bypass at the workflow level avoids churning unit-tested code. ## Verification - CI on this PR: `policy` should pass and the downstream lanes (`Build`, `Typecheck + Release Registry`, `General tests`, `Verify serialized server`, `Canary Dry Run`, `e2e`, `verify`) should all run normally (this PR isn't from Dependabot, so the bypass condition is false — proves we didn't accidentally widen the exemption). - After merge, ask Dependabot to rebase #7331 (`@dependabot rebase`) and confirm: - `PR / policy` → `success` (lockfile step now `skipped`, other policy steps `success`) - `PR / Build`, `PR / Typecheck + Release Registry`, `PR / General tests (server|workspaces-a|workspaces-b)`, `PR / Verify serialized server (1/4..4/4)`, `PR / Canary Dry Run`, `PR / e2e` → all execute (none `skipped`) - `PR / verify` → `success` once the matrix passes - `commitperclip PR Review / review` → `success` (quality-gates steps `skipped` for Dependabot; security gates ran) - Socket and Snyk checks unchanged - Local sanity-check: `git diff origin/master..HEAD` shows only the two workflow files, 7 added / 2 removed lines. ## Risks - **Auto-merging a poisoned dep.** Mitigated by Socket.dev + Snyk + human merge approval. This change only affects CI gating, not who clicks "Merge". - **Spoofing `github.actor` as `dependabot[bot]`.** GitHub sets `github.actor` from the push actor; spoofing requires a compromised Dependabot install token, which is the same threat model that already lets an attacker push anything to a Dependabot-controlled branch — not a new risk surface. - **Policy "Validate dependency resolution when manifests change" step running `pnpm install --lockfile-only --no-frozen-lockfile` on a Dependabot lockfile.** That step intentionally uses `--lockfile-only`, so it only verifies the manifest resolves and does not push or commit the result. Existing behavior is unchanged. - Low overall: the diff is two workflow-level `if:` conditions in steps that already had bypasses. ## Model Used - Provider: Anthropic Claude (via Claude Code in the Paperclip executor) - Model ID: claude-opus-4-7 - Context window: 200K - Reasoning mode: standard tool-use; no extended thinking required for this change - Capabilities used: file edit, bash, GraphQL/REST API calls - Plan was drafted, approved by board, and split into child issues before implementation; see [PAPA-490](https://paperclip.ing/PAPA/issues/PAPA-490) for the planning thread. ## 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 run tests locally and they pass (this change is workflow-only — no code under test; lint via `yamllint` clean) - [x] I have added or updated tests where applicable (workflow gating; no script changes, no unit-testable surface) - [x] If this change affects the UI, I have included before/after screenshots (no UI changes) - [x] I have updated relevant documentation to reflect my changes (no docs reference these gates) - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge |
||
|
|
03e1e3abd2 |
Revert "Remove linked-issue gate from commitperclip" (#7426)
Reverts paperclipai/paperclip#7423 Decided to keep this in place so we can automate issue reproduction in the future. We all make mistakes. Even me, if you can believe it. |
||
|
|
68401f82f3 |
Remove linked-issue gate from commitperclip (#7423)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - The `commitperclip` PR quality gates enforce hygiene on every PR before merge > - One of those gates required PRs to link to a tracking issue, which adds friction for small/internal changes that don't need a tracker entry > - The repository owner decided the linked-issue requirement is no longer the right default > - This pull request removes the linked-issue gate (the script, its tests, and the orchestrator wiring) > - The benefit is fewer false-failing PR checks and one less mandatory authoring step ## What Changed - Deleted `.github/scripts/check-pr-linked-issue.mjs` - Deleted `.github/scripts/tests/check-pr-linked-issue.test.mjs` - Removed the `checkLinkedIssue` import, the `Promise.resolve(checkLinkedIssue(...))` entry in the `Promise.all` block, the `issueResult` destructured binding, and the `...issueResult.failures` spread from `.github/scripts/run-quality-gates.mjs` ## Verification - `node --test .github/scripts/tests/*.test.mjs` — 72/72 tests pass across the remaining 4 gate suites - `git grep -n 'check-pr-linked-issue\|checkLinkedIssue\|check-pr-linked'` — no matches - Inspected `run-quality-gates.mjs` — no orphaned `issueResult` references ## Risks - Low risk. Pure removal of one optional gate; the `.github/workflows/commitperclip-review.yml` workflow only invokes the orchestrator and needs no changes. PR template and `CONTRIBUTING.md` do not mention linked issues, so no docs change is required. ## Model Used - Claude (Anthropic), `claude-opus-4-7`, extended-thinking mode, tool use 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 run tests locally and they pass - [x] I have added or updated tests where applicable (existing gate-suite tests still pass; removed gate's tests deleted with it) - [x] If this change affects the UI, I have included before/after screenshots (n/a — CI script change) - [x] I have updated relevant documentation to reflect my changes (no docs reference the removed gate) - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
96feaa331a |
feat(commitperclip): add automated PR quality and security gates (#6469)
Fixes #6470 ## Thinking Path > - Paperclip is an open-source AI agent platform receiving a high volume of community PRs — currently 2,398 open > - The contributor experience is broken: PRs sit for months with no feedback, contributors don't know why they're stuck, and maintainers spend review time on PRs that are missing basics > - Common problems: no linked issue, no test coverage, incomplete PR template, manually-edited lockfile — all catchable before human review > - At the same time, accepting untrusted PRs from unknown contributors is a real attack surface: malicious packages, secret injection, tampering with CI scripts, and code touching the sensitive paths from the April security advisories > - This PR adds automated gates that run on every PR: quality failures get a clear comment telling contributors exactly what to fix, security concerns are silently flagged as draft advisories and block merge via a pending check run > - The benefit is a dramatically faster feedback loop for good-faith contributors and a meaningful security layer for the maintainers reviewing them ## What Changed - **`.github/workflows/commitperclip-review.yml`** — new workflow using `pull_request_target` (runs in base branch context, has secrets, never executes PR code). Runs quality gates + security gates on every PR open/update. - **`.github/dependabot.yml`** — weekly automated dependency vulnerability PRs for npm and GitHub Actions. - **`.github/scripts/get-bot-token.mjs`** — generates a short-lived commitperclip installation token from `COMMITPERCLIP_KEY` secret. - **`.github/scripts/run-quality-gates.mjs`** — orchestrates 5 quality gates, posts/updates a single consolidated comment on the PR. - **`.github/scripts/check-pr-template.mjs`** — validates all 5 required template sections, Thinking Path depth (≥3 sentences), Model Used not placeholder. - **`.github/scripts/check-pr-linked-issue.mjs`** — requires `Fixes #NNN` or issue URL in PR body. - **`.github/scripts/check-pr-test-coverage.mjs`** — requires at least one test file in the diff. - **`.github/scripts/check-pr-lockfile.mjs`** — blocks manual `pnpm-lock.yaml` edits (only the refresh bot may change it). - **`.github/scripts/check-pr-dependencies.mjs`** — informational comment when new npm packages are added. - **`.github/scripts/check-pr-security.mjs`** — 6 silent security checks: secret patterns, CI workflow tampering, build script changes, supply chain (new packages in lockfile), suspicious test patterns (outbound network/shell exec/env var reads), and changes to the 9 sensitive path prefixes from the April advisories. When any fire: creates a draft security advisory + sets `security-review` check to `in_progress` (blocks merge). When clean: sets `security-review` to `success`. - **`actions/dependency-review-action@v4`** — per-PR dependency vulnerability check (fails if new dep has known CVE). - **44 unit tests** across all gate modules (`node:test`, no external deps). ## Verification Run all unit tests locally: ```bash node --test .github/scripts/tests/*.test.mjs ``` Expected: 44 pass, 0 fail. End-to-end: open a PR missing the template, linked issue, and test files → commitperclip posts a consolidated comment listing all failures. Open a PR with all gates satisfied → `✅ All checks passing` comment posted, all check runs green. ## Risks **`pull_request_target` security model:** This workflow runs in base branch context and has access to secrets. It explicitly checks out `ref: master` (never PR code) and reads the PR diff via GitHub API only — no PR code is ever executed. This is the correct pattern for running secret-bearing checks on fork PRs; deviating from it (e.g. checking out the PR branch) would be a security vulnerability. **False positives on security gates:** The sensitive-path gate flags any PR touching the 9 path prefixes from the April advisories. Legitimate fixes to those paths will trigger draft advisories. This is intentional — those paths warrant a human look regardless. The `security-review` check can be manually resolved by a maintainer once reviewed. **commitperclip not yet installed:** Until the app is installed on this repo and the `COMMITPERCLIP_KEY` secret is added, the workflow will fail on the token generation step. The quality gate comment won't post, but Dependency Review will still run independently. ## Model Used Claude Sonnet 4.5, 200k context window, extended thinking enabled, tool use: read/edit files, bash execution, GitHub API calls ## 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 run tests locally and they pass (44/44) - [x] I have added or updated tests where applicable (44 unit tests across all gate modules) - [ ] If this change affects the UI, I have included before/after screenshots (N/A — CI only) - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- ## One-time setup needed from you, Dotta 1. **Install commitperclip app** on this repo: https://github.com/apps/commitperclip/installations/new 2. **Add `COMMITPERCLIP_KEY`** as a repository secret (Actions → Secrets) — ask @brandonburr for the key 3. **Add `security_advisories: write` and `checks: write`** to the commitperclip app permissions (commit-capital org → Settings → Apps → commitperclip → Permissions) 4. **Install Socket.dev** from GitHub Marketplace for supply chain scanning 5. **Branch protection** (optional but recommended): require `commitperclip-review` and `security-review` checks to pass before merge ## Dashboard integration note The `commitperclip-review` check run result maps cleanly to your PR triage dashboard. A single filter on your Worker: ```javascript const gatesCheck = checkRuns.find(r => r.name === 'commitperclip-review'); if (gatesCheck?.conclusion === 'failure') return null; // filter from queue ``` For security flags: `GET /repos/paperclipai/paperclip/security-advisories?state=draft` — advisory titles include `PR #NNN` for cross-referencing. PRs with a matching draft advisory have `security-review` in `in_progress` state (grey spinner, can't merge via branch protection). --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Devin Foley <devin@devinfoley.com> Co-authored-by: Paperclip <noreply@paperclip.ing> |