Skip to content

Commit 41fa3ae

Browse files
Add AGENTS.md with hard rules for AI-generated PRs (#65)
* Fix: pin uuid to ^9.0.1 to keep browserify build working uuid@8 is npm-deprecated and uuid@14 (current latest) drops the main field plus ships ES2021 (??=) syntax that browserify@16 cannot parse. uuid@9.0.1 is the highest version with a main field and CJS output that browserify@16 accepts. Stage: git add package.json package-lock.json * Add AGENTS.md with hard rules for AI-generated PRs Captures recurring problems from AI-authored PRs: drive-by formatter sweeps, synthetic-model tests instead of real round-trip, missing file lists, .prettierrc edits across overlapping PRs, and unverified XLSX serialization changes. README and CONTRIBUTING point to AGENTS.md. PR template adds a Files-changed section and a checklist mirroring the rules. Stage: git add AGENTS.md README.md CONTRIBUTING.md .github/PULL_REQUEST_TEMPLATE.md
1 parent 10a5c5d commit 41fa3ae

4 files changed

Lines changed: 122 additions & 7 deletions

File tree

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,28 @@
1-
<!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. The two fields below are mandatory. -->
1+
<!-- Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. -->
2+
3+
<!-- AI agents (Claude Code, Cursor, Codex, Copilot Workspace, etc.) and humans submitting AI-generated code: read AGENTS.md before opening this PR. PRs that ignore those rules get closed. -->
24

35
## Summary
46

57
<!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? -->
68

9+
## Files changed
10+
11+
<!-- List EVERY file changed, including configs, lockfiles, and incidental edits. If this PR touches .prettierrc, .eslintrc, package.json, package-lock.json, README.md, or index.d.ts, call it out explicitly. -->
12+
713
## Test plan
814

9-
<!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI. -->
15+
<!-- Demonstrate the code is solid. Include the exact commands you ran and their output. For bugfixes, include a test (in spec/) that fails before the fix and passes after. -->
1016

1117
## Related to source code (for typings update)
1218

13-
<!-- List with permalink into source code to prove that changes are true -->
19+
<!-- List with permalink into source code to prove that changes are true. -->
20+
21+
## Checklist
22+
23+
- [ ] Only the lines required for the fix/feature are changed (no formatter sweeps, no drive-by refactors)
24+
- [ ] Tests use real fixture round-trip (`wb.xlsx.load` / `writeBuffer`), not synthetic model objects, where possible
25+
- [ ] Checked open PRs (`gh pr list --state open`) for conflicting changes to the same files
26+
- [ ] (If serialization is touched) Output verified to open in Excel, or `soffice --headless` round-trip clean
27+
- [ ] (If `--no-verify` was used to bypass the pre-commit hook) Reason explained above
28+
- [ ] (If a major dep was bumped) Runtime smoke test described above

AGENTS.md

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# AGENTS.md
2+
3+
Instructions for AI coding agents (Claude Code, Cursor, Codex, Copilot Workspace, etc.) opening pull requests against this repository.
4+
5+
Human contributors: see [CONTRIBUTING.md](CONTRIBUTING.md). The rules below also apply to any human submitting AI-generated code.
6+
7+
If you are an AI agent, read this file in full before making changes. The patterns below come from real PRs we have had to reject or rework.
8+
9+
---
10+
11+
## Hard rules
12+
13+
### 1. Surgical scope
14+
15+
- Touch ONLY the lines required for the fix or feature.
16+
- Do NOT extract constants, rename variables, collapse `Promise.all`, rewrite `reduce` to `for`, or "clean up" surrounding code, even if your linter or style tool suggests it.
17+
- If the pre-commit hook (prettier ↔ eslint conflict) fights you, commit with `--no-verify` and call this out in the PR description. Do NOT reshape unrelated code to satisfy the hook.
18+
19+
### 2. No formatter sweeps
20+
21+
- Do NOT run `npx prettier --write .`, `eslint --fix` on whole files, or any tool that rewrites files you did not otherwise change.
22+
- Do NOT modify `.prettierrc`, `.eslintrc`, or other config files in a feature/bugfix PR. If a config change is needed, open a separate config-only PR.
23+
24+
### 3. PR description must be complete
25+
26+
- List EVERY file changed in the PR description, including configs, lockfiles, and incidental edits.
27+
- If you used `--no-verify`, say so and explain why.
28+
- If your change depends on or conflicts with another open PR, link it.
29+
30+
### 4. Tests must use real fixtures, not synthetic models
31+
32+
- For XLSX read/write fixes, your test MUST round-trip through `wb.xlsx.load(...)` and/or `wb.xlsx.writeBuffer(...)` against a real or minimal fixture file.
33+
- Do NOT build hand-rolled model objects and call internal serializers (e.g. `XLSX.reconcile`, individual xforms) directly. Synthetic-model tests can pass while the real load/write path still breaks.
34+
- Place fixture files under `spec/integration/data/` following existing naming.
35+
36+
### 5. Cross-check open PRs before submitting
37+
38+
- Before opening a PR, list the currently open PRs in this repo and verify your change does not touch the same files or overlap in scope. The maintainer has had three concurrent PRs all editing `.prettierrc` because none of them checked.
39+
- Run: `gh pr list --state open --limit 50 --json number,title,files`
40+
41+
### 6. XLSX serialization changes must be Excel-verified
42+
43+
If your change touches anything that writes XLSX (xforms, sheet/workbook serialization, pivot tables, charts, comments, conditional formatting):
44+
45+
- Verify the output file actually opens in Excel without a "Repaired Records" warning, OR
46+
- Round-trip with LibreOffice headless and inspect the bytes:
47+
```bash
48+
soffice --headless --convert-to xlsx /tmp/your-output.xlsx --outdir /tmp/roundtrip
49+
unzip -p /tmp/roundtrip/your-output.xlsx xl/<relevant-part>.xml | head
50+
```
51+
- Unit tests do NOT catch Excel's repair warnings. This step is mandatory for serialization-touching PRs.
52+
53+
### 7. Dependency bumps require runtime smoke tests
54+
55+
- Major-version dep bumps (e.g. `fast-csv`, `unzipper`, `archiver`) MUST be smoke-tested against the runtime paths that use them, not just `npm test`.
56+
- Streaming reader changes: load a real `.xlsx`. CSV writer changes: write and re-parse real CSV. Document the smoke test in the PR.
57+
58+
### 8. One concern per PR
59+
60+
- Bug fix + sibling bugs of the same shape in the same file: ONE PR. Good.
61+
- Bug fix + unrelated cleanup + dep bump: THREE PRs. Required.
62+
- If you find yourself touching `index.d.ts`, `README.md`, `package-lock.json`, etc. incidentally, stop and revert those edits unless they are the actual subject of the PR.
63+
64+
---
65+
66+
## Per-PR checklist (also in `.github/pull_request_template.md`)
67+
68+
Before opening the PR, verify:
69+
70+
- [ ] Only the lines required for the fix are changed
71+
- [ ] No prettier/eslint sweeps on unrelated files
72+
- [ ] Every file change is listed in the PR description below
73+
- [ ] Tests use real fixture round-trip (`wb.xlsx.load` / `writeBuffer`), not synthetic models
74+
- [ ] `gh pr list --state open` checked for conflicting PRs
75+
- [ ] (If serialization) Output file opens in Excel or `soffice --headless` without warnings
76+
- [ ] (If `--no-verify` used) Reason noted in PR description
77+
- [ ] (If dep bump) Runtime smoke test described in PR
78+
79+
---
80+
81+
## Repository conventions
82+
83+
- Code style: enforced by ESLint + Prettier (the conflict above is real — live with it on a per-PR basis)
84+
- Tests: Mocha, in `spec/unit/`, `spec/integration/`, `spec/end-to-end/`
85+
- Run unit tests fast: `npm run test:unit` (skips the build step)
86+
- Full suite: `npm test` (build + unit + integration + e2e + jasmine)
87+
- Node target: see `engines` in `package.json`
88+
- Browserify is in the build chain. Do NOT introduce dependencies that ship ES2021+ syntax (`??=`, `?.()`) in their CJS output, or that publish `exports`-only packages without a `main` field. Both break the browser bundle.
89+
90+
---
91+
92+
## What this fork is
93+
94+
This is a curated Protobi fork of [exceljs/exceljs](https://github.com/exceljs/exceljs). We selectively adopt upstream features for production use. We are NOT a general-purpose alternative fork. See [CONTRIBUTING.md](CONTRIBUTING.md) for what we accept.
95+
96+
If your change is a generic improvement that has not been merged upstream, consider opening it against [exceljs/exceljs](https://github.com/exceljs/exceljs) first.

CONTRIBUTING.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
Thank you for your interest in contributing!
44

5+
> **AI agents and humans submitting AI-generated code:** read [AGENTS.md](AGENTS.md) first. It has hard rules about PR scope, formatter sweeps, real-fixture testing, and Excel-verification for serialization changes. PRs that ignore those rules get closed regardless of the underlying fix quality.
6+
57
## About This Fork
68

79
This is a **curated fork** of ExcelJS maintained by Protobi. We selectively adopt features from the upstream repository that we need for our production systems.

README.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,13 @@ npm install exceljs
145145

146146
Contributions are very welcome! It helps me know what features are desired or what bugs are causing the most pain.
147147

148-
I have just one request; If you submit a pull request for a bugfix, please add a unit-test or integration-test (in the spec folder) that catches the problem.
149-
Even a PR that just has a failing test is fine - I can analyse what the test is doing and fix the code from that.
148+
**Before opening a PR, read [CONTRIBUTING.md](CONTRIBUTING.md).**
150149

151-
Note: Please try to avoid modifying the package version in a PR.
152-
Versions are updated on release and any change will most likely result in merge collisions.
150+
**If you are an AI agent (Claude Code, Cursor, Codex, Copilot Workspace, etc.) — or a human submitting AI-generated code — read [AGENTS.md](AGENTS.md) in full first.** It contains hard rules about scope, formatter sweeps, real-fixture testing, and Excel-verification for serialization changes. AI-generated PRs that ignore these get closed.
151+
152+
For humans: if you submit a pull request for a bugfix, please add a unit-test or integration-test (in the `spec/` folder) that catches the problem. Even a PR that just has a failing test is fine — I can analyse what the test is doing and fix the code from that.
153+
154+
Note: Please try to avoid modifying the package version in a PR. Versions are updated on release and any change will most likely result in merge collisions.
153155

154156
To be clear, all contributions added to this library will be included in the library's MIT licence.
155157

0 commit comments

Comments
 (0)