github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-09-14 07:59:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/staging' into develop

Browse Source
This commit is contained in:
Andrey Antukh 2026-09-11 12:57:51 +02:00
commit bda8459d89
10 changed files with 501 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
.agents/skills/create-commit/SKILL.md
View File

@ -20,6 +20,17 @@ Before drafting any commit, read `mem:workflow/creating-commits` end-to-end. It
is the authoritative source for the commit message format, the emoji menu, is the authoritative source for the commit message format, the emoji menu,
subject/body limits, and the `AI-assisted-by` trailer. Follow it exactly. subject/body limits, and the `AI-assisted-by` trailer. Follow it exactly.
## Iron Rules (non-negotiable)
1. **Wrap every body line at 76 characters or fewer.** Count characters, do
not eyeball. Exceptions: `Signed-off-by:` / `AI-assisted-by:` trailers and
lines carrying a URL. This is the rule agents skip most often.
2. **Subject ≤70 chars**, imperative, capitalized, no trailing period.
3. **Blank line between subject and body.**
4. **Run `./scripts/check-commit` and require exit code 0.** It mechanically
checks rules 1–3. A non-zero exit is a hard blocker: fix the message and
re-commit. Never report the commit as done with a failing checker.
## Workflow ## Workflow
1. **Stage the files** specified by the calling context. Do not ask for 1. **Stage the files** specified by the calling context. Do not ask for
@ -29,12 +40,18 @@ subject/body limits, and the `AI-assisted-by` trailer. Follow it exactly.
that does not match the stated intent, **STOP** and tell the user before that does not match the stated intent, **STOP** and tell the user before
committing. committing.
3. Draft the message following the format in the memory doc, wrapping the body 3. Draft the message following the format in the memory doc, wrapping the body
at 72 characters per line, and run: at 76 characters per line, and run:
```bash ```bash
git commit -m "<subject>" -m "<body>" git commit -m "<subject>" -m "<body>"
``` ```
(or `git commit -F -` if the body has unusual characters). (or `git commit -F -` if the body has unusual characters).
4. The `AI-assisted-by` trailer value is provided by the calling context — use 4. **Verify the message with the checker**:
```bash
./scripts/check-commit
```
If it fails, amend the message (`git commit --amend`) until it passes. Do
not finish with a failing checker.
5. The `AI-assisted-by` trailer value is provided by the calling context — use
it verbatim. it verbatim.
## Constraints ## Constraints
@ -45,3 +62,4 @@ subject/body limits, and the `AI-assisted-by` trailer. Follow it exactly.
- Do not amend a commit you did not create in this session, unless explicitly asked. - Do not amend a commit you did not create in this session, unless explicitly asked.
- Do not bypass pre-commit hooks (`--no-verify`) unless explicitly asked. - Do not bypass pre-commit hooks (`--no-verify`) unless explicitly asked.
- Do not add untracked files that were not created in this session. - Do not add untracked files that were not created in this session.
- Do not skip the `scripts/check-commit` verification step (Iron Rule 4).

2
.serena/memories/critical-info.md
View File

@ -11,7 +11,7 @@ You are working on the GitHub project `penpot/penpot`, a monorepo.
# Development workflow # Development workflow
- Commit/PR/issue creation is **on explicit request only**. Before any of these actions, read the relevant memory — don't infer format from prior examples: - Commit/PR/issue creation is **on explicit request only**. Before any of these actions, read the relevant memory — don't infer format from prior examples:
- Before `git commit` → `mem:workflow/creating-commits` (subject format, body, `AI-assisted-by: model-name` trailer) - Before `git commit` → `mem:workflow/creating-commits` (subject/body format, 76-char body wrapping enforced by `scripts/check-commit`, `AI-assisted-by: model-name` trailer)
- Before `gh issue create` → `mem:workflow/creating-issues` (title derivation, body template, labels, Issue Type) - Before `gh issue create` → `mem:workflow/creating-issues` (title derivation, body template, labels, Issue Type)
- Before `gh pr create` / `gh pr edit` → `mem:workflow/creating-prs` (title format, body structure, "Note:" line) - Before `gh pr create` / `gh pr edit` → `mem:workflow/creating-prs` (title format, body structure, "Note:" line)
- Before a repo-wide pnpm version update → `mem:workflow/updating-pnpm` (workspace - Before a repo-wide pnpm version update → `mem:workflow/updating-pnpm` (workspace

24
.serena/memories/workflow/creating-commits.md
View File

@ -14,12 +14,32 @@ automatically pull the identity from the local git config `user.name` and `user.
:emoji: Subject line (imperative, capitalized, no period, <=70 chars) :emoji: Subject line (imperative, capitalized, no period, <=70 chars)
Body explaining what changed and why. Body explaining what changed and why.
Wrap lines at 72 characters — git log and tooling Wrap lines at 76 characters — git log adds a
render long lines poorly. Keep each line concise. four-space indent, so 76 + 4 fits an 80-column
terminal. Keep each line concise.
AI-assisted-by: model-name AI-assisted-by: model-name
``` ```
## HARD RULES (inexcusable)
These rules are not advisory. Do not commit until every one holds. A commit
that breaks them is wrong, even if the code is right.
- **Body lines MUST wrap at 76 characters or fewer.** Measure every line; do
not eyeball it. This is the rule most often skipped. Rationale: `git log`
indents the body four spaces, so 76 + 4 fits an 80-column terminal.
- **Subject MUST be ≤70 chars**, imperative, capitalized, no trailing period.
- **MUST be a blank line** between subject and body.
- **MUST run `scripts/check-commit` and get exit code 0 before finishing.**
It mechanically validates the rules above; a failing run is a blocker.
- It checks `HEAD` by default: `./scripts/check-commit`
- For another commit: `./scripts/check-commit -c <ref>`
- **NEVER** hand-wave the body as "one long line". If a line exceeds 76,
break it at a space.
- Exceptions inside the body (do not wrap these): `Signed-off-by:`,
`Co-authored-by:`, `AI-assisted-by:` trailers, and lines carrying a URL.
**AI-assisted-by trailer rules:** **AI-assisted-by trailer rules:**
- Use only the model name, e.g. `mimo-v2.5`, `deepseek-v4-flash` - Use only the model name, e.g. `mimo-v2.5`, `deepseek-v4-flash`
- Do NOT add prefixes like `opencode-go/` — use the bare model name - Do NOT add prefixes like `opencode-go/` — use the bare model name

3
AGENTS.md
View File

@ -17,6 +17,9 @@
- **`.claude/skills` is a symlink to `.agents/skills`.** - **`.claude/skills` is a symlink to `.agents/skills`.**
Edit skills only in their canonical location (`.agents/skills`); never edit Edit skills only in their canonical location (`.agents/skills`); never edit
through `.claude/skills`. through `.claude/skills`.
- **Commit message body lines MUST wrap at ≤76 chars** (subject ≤70 chars) and
the commit MUST pass `./scripts/check-commit` with exit code 0 before you
consider it done. This is mechanically checked — do not eyeball it.
- **Read the workflow memory BEFORE the corresponding action**: - **Read the workflow memory BEFORE the corresponding action**:
- Before `git commit` → `mem:workflow/creating-commits` (commit format, AI-assisted-by trailer) - Before `git commit` → `mem:workflow/creating-commits` (commit format, AI-assisted-by trailer)
- Before `gh issue create` → `mem:workflow/creating-issues` (title derivation, body template, Issue Type) - Before `gh issue create` → `mem:workflow/creating-issues` (title derivation, body template, Issue Type)

3
CONTRIBUTING.md
View File

@ -188,8 +188,11 @@ Commit messages must follow this format:
- Add clear and concise description on the body - Add clear and concise description on the body
- Do not end the subject with a period - Do not end the subject with a period
- Keep the subject to **70 characters** or fewer - Keep the subject to **70 characters** or fewer
- **Wrap body lines at 76 characters or fewer** (trailers and URLs excepted)
- Separate the subject from the body with a **blank line** - Separate the subject from the body with a **blank line**
You can check a commit against these rules with `./scripts/check-commit`.
### Examples ### Examples
``` ```

2
backend/src/app/config.clj
View File

@ -92,6 +92,7 @@
:quotes-upload-sessions-per-profile 5 :quotes-upload-sessions-per-profile 5
:quotes-upload-chunks-per-session 20 :quotes-upload-chunks-per-session 20
:upload-max-chunk-size (* 1024 1024 30) ; 30MiB
;; SSRF protection ;; SSRF protection
:ssrf-allowed-hosts #{} :ssrf-allowed-hosts #{}
@ -203,6 +204,7 @@
[:quotes-team-access-requests-per-requester {:optional true} ::sm/int] [:quotes-team-access-requests-per-requester {:optional true} ::sm/int]
[:quotes-upload-sessions-per-profile {:optional true} ::sm/int] [:quotes-upload-sessions-per-profile {:optional true} ::sm/int]
[:quotes-upload-chunks-per-session {:optional true} ::sm/int] [:quotes-upload-chunks-per-session {:optional true} ::sm/int]
[:upload-max-chunk-size {:optional true} ::sm/int]
[:quotes-media-storage-bytes-per-team {:optional true} ::sm/int] [:quotes-media-storage-bytes-per-team {:optional true} ::sm/int]
[:auth-token-cookie-name {:optional true} :string] [:auth-token-cookie-name {:optional true} :string]

89
backend/src/app/rpc/commands/media.clj
View File

@ -339,6 +339,9 @@
;; --- Chunked Upload: Upload a single chunk ;; --- Chunked Upload: Upload a single chunk
(declare ^:private get-upload-chunk)
(declare ^:private check-upload-chunk-slot)
(def ^:private schema:upload-chunk (def ^:private schema:upload-chunk
[:map {:title "upload-chunk"} [:map {:title "upload-chunk"}
[:session-id ::sm/uuid] [:session-id ::sm/uuid]
@ -354,9 +357,31 @@
{::doc/added "2.17" {::doc/added "2.17"
::sm/params schema:upload-chunk ::sm/params schema:upload-chunk
::sm/result schema:upload-chunk-result} ::sm/result schema:upload-chunk-result}
[{:keys [::db/pool] :as cfg} [cfg {:keys [::rpc/profile-id session-id index content]}]
{:keys [::rpc/profile-id session-id index content] :as _params}] (let [session (db/tx-run! cfg check-upload-chunk-slot session-id profile-id index content)]
(let [session (db/get pool :upload-session {:id session-id :profile-id profile-id})] (l/trc :hint "upload-chunk"
:session-id session-id
:chunk (str index "/" (:total-chunks session))
:size (:size content)
:path (:path content))
(let [storage (sto/resolve cfg)
data (sto/content (:path content))]
(sto/put-object! storage
{::sto/content data
::sto/deduplicate? false
::sto/touch true
:content-type (:mtype content)
:bucket sto/tempfile-bucket
:upload-id (str session-id)
:chunk-index index}))
{:session-id session-id
:index index}))
(defn- check-upload-chunk-slot
[{:keys [::db/conn]} session-id profile-id index content]
(let [session (db/get conn :upload-session {:id session-id :profile-id profile-id} {::db/for-update true})]
(when (or (neg? index) (>= index (:total-chunks session))) (when (or (neg? index) (>= index (:total-chunks session)))
(ex/raise :type :validation (ex/raise :type :validation
:code :invalid-chunk-index :code :invalid-chunk-index
@ -365,26 +390,23 @@
:total-chunks (:total-chunks session) :total-chunks (:total-chunks session)
:index index)) :index index))
(when (> (:size content) (cf/get :upload-max-chunk-size))
(ex/raise :type :validation
:code :chunk-too-large
:hint "chunk size exceeds the maximum allowed"
:session-id session-id
:index index
:size (:size content)
:max-size (cf/get :upload-max-chunk-size)))
(l/trc :hint "upload-chunk" (when (get-upload-chunk conn session-id index)
:session-id session-id (ex/raise :type :validation
:chunk (str index "/" (:total-chunks session)) :code :duplicate-chunk-index
:size (:size content) :hint "chunk index already uploaded for this session"
:path (:path content))) :session-id session-id
:index index))
(let [storage (sto/resolve cfg) session))
data (sto/content (:path content))]
(sto/put-object! storage
{::sto/content data
::sto/deduplicate? false
::sto/touched-at (ct/in-future {:hours 1})
:content-type (:mtype content)
:bucket sto/tempfile-bucket
:upload-id (str session-id)
:chunk-index index}))
{:session-id session-id
:index index})
;; --- Chunked Upload: shared helpers ;; --- Chunked Upload: shared helpers
@ -400,6 +422,18 @@
[conn session-id] [conn session-id]
(db/exec! conn [sql:get-upload-chunks (str session-id)])) (db/exec! conn [sql:get-upload-chunks (str session-id)]))
(def ^:private sql:get-upload-chunk
"SELECT id
FROM storage_object
WHERE (metadata->>'~:upload-id') = ?::text
AND (metadata->>'~:chunk-index')::integer = ?
AND deleted_at IS NULL
LIMIT 1")
(defn- get-upload-chunk
[conn session-id index]
(db/exec-one! conn [sql:get-upload-chunk (str session-id) index]))
(defn- concat-chunks (defn- concat-chunks
"Reads all chunk storage objects in order and writes them to a single "Reads all chunk storage objects in order and writes them to a single
temporary file on the local filesystem. Returns a path to that file." temporary file on the local filesystem. Returns a path to that file."
@ -418,18 +452,21 @@
conforming to `media.v/schema:upload` with `:filename`, `:path` and conforming to `media.v/schema:upload` with `:filename`, `:path` and
`:size`. `:size`.
Raises a :validation/:missing-chunks error when the number of stored Raises a :validation/:missing-chunks error when the stored chunk
chunks does not match `:total-chunks` recorded in the session row. indices do not form exactly the `0..total-chunks` range recorded in
the session row (wrong count, gaps or duplicates).
Raises :not-found when the session does not belong to `profile-id`. Raises :not-found when the session does not belong to `profile-id`.
Deletes the session row from `upload_session` on success." Deletes the session row from `upload_session` on success."
[{:keys [::db/conn] :as cfg} profile-id session-id] [{:keys [::db/conn] :as cfg} profile-id session-id]
(let [session (db/get conn :upload-session {:id session-id :profile-id profile-id}) (let [session (db/get conn :upload-session {:id session-id :profile-id profile-id})
chunks (get-upload-chunks conn session-id)] chunks (get-upload-chunks conn session-id)
indices (sort (map :chunk-index chunks))]
(when (not= (count chunks) (:total-chunks session)) (when (or (not= (count chunks) (:total-chunks session))
(not= indices (range (:total-chunks session))))
(ex/raise :type :validation (ex/raise :type :validation
:code :missing-chunks :code :missing-chunks
:hint "number of stored chunks does not match expected total" :hint "stored chunks do not match expected total"
:session-id session-id :session-id session-id
:expected (:total-chunks session) :expected (:total-chunks session)
:found (count chunks))) :found (count chunks)))

196
backend/test/backend_tests/rpc_media_test.clj
View File

@ -681,6 +681,131 @@
(t/is (= :validation (-> out :error ex-data :type))) (t/is (= :validation (-> out :error ex-data :type)))
(t/is (= :missing-chunks (-> out :error ex-data :code)))))) (t/is (= :missing-chunks (-> out :error ex-data :code))))))
(t/deftest chunked-upload-assemble-rejects-duplicate-indices
;; assemble-chunks must validate the index SET, not just the count: a
;; session declaring 2 chunks but storing [0,0] must fail instead of
;; assembling a corrupt file. Chunks are written at the storage level
;; because upload-chunk itself now rejects the second index.
(let [prof (th/create-profile* 1)
_ (th/create-project* 1 {:profile-id (:id prof)
:team-id (:default-team-id prof)})
file (th/create-file* 1 {:profile-id (:id prof)
:project-id (:default-project-id prof)
:is-shared false})
session-id (create-session! prof 2)
storage (:app.storage/storage th/*system*)
source-path (th/tempfile "backend_tests/test_files/sample.jpg")
chunks (split-file-into-chunks source-path 312043)
put-chunk! (fn [idx]
(let [mfile (make-chunk-mfile (first chunks) "image/jpeg")]
(sto/put-object! storage
{::sto/content (sto/content (:path mfile))
::sto/deduplicate? false
::sto/touch true
:content-type "image/jpeg"
:bucket sto/tempfile-bucket
:upload-id (str session-id)
:chunk-index idx})))]
(put-chunk! 0)
(put-chunk! 0)
(let [out (th/command! {::th/type :assemble-file-media-object
::rpc/profile-id (:id prof)
:session-id session-id
:file-id (:id file)
:is-local true
:name "dupe-indices"
:mtype "image/jpeg"})]
(t/is (some? (:error out)))
(t/is (= :validation (-> out :error ex-data :type)))
(t/is (= :missing-chunks (-> out :error ex-data :code))))))
(t/deftest chunked-upload-duplicate-then-assemble
;; A rejected duplicate must leave the first chunk intact: upload 0,
;; re-upload 0 (rejected), then assemble succeeds with the original size.
(let [prof (th/create-profile* 1)
_ (th/create-project* 1 {:profile-id (:id prof)
:team-id (:default-team-id prof)})
file (th/create-file* 1 {:profile-id (:id prof)
:project-id (:default-project-id prof)
:is-shared false})
session-id (create-session! prof 1)
source-path (th/tempfile "backend_tests/test_files/sample.jpg")
chunks (split-file-into-chunks source-path 312043)
mtype "image/jpeg"
size (alength (first chunks))]
(let [out (th/command! {::th/type :upload-chunk
::rpc/profile-id (:id prof)
:session-id session-id
:index 0
:content (make-chunk-mfile (first chunks) mtype)})]
(t/is (nil? (:error out))))
(let [out (th/command! {::th/type :upload-chunk
::rpc/profile-id (:id prof)
:session-id session-id
:index 0
:content (make-chunk-mfile (first chunks) mtype)})]
(t/is (some? (:error out)))
(t/is (= :duplicate-chunk-index (-> out :error ex-data :code))))
(let [out (th/command! {::th/type :assemble-file-media-object
::rpc/profile-id (:id prof)
:session-id session-id
:file-id (:id file)
:is-local true
:name "after-dupe"
:mtype mtype})]
(t/is (nil? (:error out)))
(let [storage (:app.storage/storage th/*system*)
mobj (sto/get-object storage (:media-id (:result out)))]
(t/is (= size (:size mobj)))))))
(t/deftest chunked-upload-rejected-duplicate-keeps-session-usable
;; Rejecting a duplicate must not poison the session: the remaining
;; distinct indices still accumulate and assemble normally.
(let [prof (th/create-profile* 1)
_ (th/create-project* 1 {:profile-id (:id prof)
:team-id (:default-team-id prof)})
file (th/create-file* 1 {:profile-id (:id prof)
:project-id (:default-project-id prof)
:is-shared false})
session-id (create-session! prof 2)
source-path (th/tempfile "backend_tests/test_files/sample.jpg")
chunks (split-file-into-chunks source-path 110000)
mtype "image/jpeg"]
(t/is (= 3 (count chunks)))
(let [out (th/command! {::th/type :upload-chunk
::rpc/profile-id (:id prof)
:session-id session-id
:index 0
:content (make-chunk-mfile (nth chunks 0) mtype)})]
(t/is (nil? (:error out))))
(let [out (th/command! {::th/type :upload-chunk
::rpc/profile-id (:id prof)
:session-id session-id
:index 0
:content (make-chunk-mfile (nth chunks 0) mtype)})]
(t/is (some? (:error out)))
(t/is (= :duplicate-chunk-index (-> out :error ex-data :code))))
(let [out (th/command! {::th/type :upload-chunk
::rpc/profile-id (:id prof)
:session-id session-id
:index 1
:content (make-chunk-mfile (nth chunks 1) mtype)})]
(t/is (nil? (:error out))))
;; The live store holds exactly the two distinct indices: the
;; rejected duplicate stored nothing.
(let [rows (th/db-exec! ["SELECT (metadata->>'~:chunk-index')::integer AS idx FROM storage_object WHERE (metadata->>'~:upload-id') = ?::text AND deleted_at IS NULL ORDER BY idx"
(str session-id)])]
(t/is (= [0 1] (mapv :idx rows))))))
(t/deftest chunked-upload-session-not-found (t/deftest chunked-upload-session-not-found
(let [prof (th/create-profile* 1) (let [prof (th/create-profile* 1)
_ (th/create-project* 1 {:profile-id (:id prof) _ (th/create-project* 1 {:profile-id (:id prof)
@ -767,6 +892,77 @@
(t/is (= :validation (-> out :error ex-data :type))) (t/is (= :validation (-> out :error ex-data :type)))
(t/is (= :invalid-chunk-index (-> out :error ex-data :code)))))) (t/is (= :invalid-chunk-index (-> out :error ex-data :code))))))
(t/deftest chunked-upload-duplicate-index-rejected
;; Uploading the same chunk index twice into one session must fail:
;; the second call raises :validation / :duplicate-chunk-index and
;; stores nothing, so one session+index keeps at most one object.
(let [prof (th/create-profile* 1)
session-id (create-session! prof 1)
source-path (th/tempfile "backend_tests/test_files/sample.jpg")
chunks (split-file-into-chunks source-path 312043)
mtype "image/jpeg"
mfile1 (make-chunk-mfile (first chunks) mtype)
mfile2 (make-chunk-mfile (first chunks) mtype)]
;; First upload succeeds
(let [out (th/command! {::th/type :upload-chunk
::rpc/profile-id (:id prof)
:session-id session-id
:index 0
:content mfile1})]
(t/is (nil? (:error out))))
;; Second upload of the same index must be rejected
(let [out (th/command! {::th/type :upload-chunk
::rpc/profile-id (:id prof)
:session-id session-id
:index 0
:content mfile2})]
(t/is (some? (:error out)))
(t/is (= :validation (-> out :error ex-data :type)))
(t/is (= :duplicate-chunk-index (-> out :error ex-data :code))))
;; Exactly one live object stored for that session/index
(let [rows (th/db-exec! ["SELECT id FROM storage_object WHERE (metadata->>'~:upload-id') = ?::text AND (metadata->>'~:chunk-index') = '0' AND deleted_at IS NULL"
(str session-id)])]
(t/is (= 1 (count rows))))))
(t/deftest chunked-upload-chunk-too-large
;; Chunks larger than the configured cap must be rejected with
;; :validation / :chunk-too-large before anything is stored, while a
;; chunk exactly at the cap still uploads fine.
(with-mocks [mock {:target 'app.config/get
:return (th/config-get-mock
{:upload-max-chunk-size 1024})}]
(let [prof (th/create-profile* 1)
session-id (create-session! prof 1)
source-path (th/tempfile "backend_tests/test_files/sample.jpg")
chunks (split-file-into-chunks source-path 312043)
mtype "image/jpeg"]
;; 312043 bytes exceeds the mocked 1024-byte cap: rejected
(let [out (th/command! {::th/type :upload-chunk
::rpc/profile-id (:id prof)
:session-id session-id
:index 0
:content (make-chunk-mfile (first chunks) mtype)})]
(t/is (some? (:error out)))
(t/is (= :validation (-> out :error ex-data :type)))
(t/is (= :chunk-too-large (-> out :error ex-data :code))))
;; Nothing stored for the rejected chunk
(let [rows (th/db-exec! ["SELECT id FROM storage_object WHERE (metadata->>'~:upload-id') = ?::text AND deleted_at IS NULL"
(str session-id)])]
(t/is (= 0 (count rows))))
;; A chunk exactly at the cap still uploads fine
(let [out (th/command! {::th/type :upload-chunk
::rpc/profile-id (:id prof)
:session-id session-id
:index 0
:content (make-chunk-mfile (byte-array 1024 (byte 1)) mtype)})]
(t/is (nil? (:error out)))))))
(t/deftest chunked-upload-sessions-per-profile-quota (t/deftest chunked-upload-sessions-per-profile-quota
;; With the session limit set to 2, creating a third session for the ;; With the session limit set to 2, creating a third session for the
;; same profile must fail with :restriction / :max-quote-reached. ;; same profile must fail with :restriction / :max-quote-reached.

59
scripts/check-commit
View File

@ -5,6 +5,7 @@ Check commit messages against Penpot's commit guidelines.
Validates commit messages using the rules defined in: Validates commit messages using the rules defined in:
- .github/workflows/commit-checker.yml (regex pattern) - .github/workflows/commit-checker.yml (regex pattern)
- CONTRIBUTING.md (formatting rules, subject length, DCO) - CONTRIBUTING.md (formatting rules, subject length, DCO)
- .serena/memories/workflow/creating-commits.md (body wrapped at 76 chars)
By default, checks HEAD. Use --commit to specify a different commit. By default, checks HEAD. Use --commit to specify a different commit.
@ -38,6 +39,20 @@ COMMIT_PATTERN = re.compile(
MERGE_PATTERN = re.compile(r"^(Merge|Revert|Reapply).+[^.]$") MERGE_PATTERN = re.compile(r"^(Merge|Revert|Reapply).+[^.]$")
# ── Body line wrapping ───────────────────────────────────────────────────────
# Commit bodies must wrap at 76 characters (see
# .serena/memories/workflow/creating-commits.md). That leaves room for the
# four-space indent git log adds, fitting an 80-column terminal. Trailers and
# URLs are exempt: they cannot be wrapped without losing meaning.
MAX_BODY_LINE = 76
TRAILER_PATTERN = re.compile(
r"^(Signed-off-by|Co-authored-by|Co-developed-by|Reviewed-by|"
r"Acked-by|Tested-by|Reported-by|Suggested-by|AI-assisted-by):"
)
URL_PATTERN = re.compile(r"https?://\S+")
# ═══════════════════════════════════════════════════════════════════════════════ # ═══════════════════════════════════════════════════════════════════════════════
# Helpers # Helpers
# ═══════════════════════════════════════════════════════════════════════════════ # ═══════════════════════════════════════════════════════════════════════════════
@ -93,11 +108,11 @@ def check_regex(message):
def check_subject_length(message): def check_subject_length(message):
"""Subject line must be ≤ 90 characters.""" """Subject line must be ≤ 70 characters."""
first_line = message.split("\n")[0] first_line = message.split("\n")[0]
if len(first_line) > 90: if len(first_line) > 70:
return False, ( return False, (
f"Subject line exceeds 90 characters ({len(first_line)} chars):\n" f"Subject line exceeds 70 characters ({len(first_line)} chars):\n"
f" {first_line}" f" {first_line}"
) )
return True, None return True, None
@ -148,6 +163,41 @@ def check_body_blank_line(message):
return True, None return True, None
def check_body_line_length(message):
"""Body lines must wrap at 76 characters or fewer.
The subject (first line) has its own length rule. Trailers (e.g.
Signed-off-by) and lines carrying a URL are exempt, since wrapping them
would break tooling or lose information.
"""
lines = message.split("\n")
offenders = []
for line_number, line in enumerate(lines[1:], start=2):
if len(line) <= MAX_BODY_LINE:
continue
if TRAILER_PATTERN.match(line):
continue
if URL_PATTERN.search(line):
continue
# A long token with no whitespace before the limit cannot be wrapped.
if " " not in line[:MAX_BODY_LINE]:
continue
offenders.append((line_number, line))
if not offenders:
return True, None
details = "\n".join(
f" line {line_number} ({len(line)} chars): {line!r}"
for line_number, line in offenders
)
return False, (
f"Body lines must wrap at {MAX_BODY_LINE} characters or fewer. "
"Unwrapped line(s):\n" + details
)
def check_signed_off_by(message): def check_signed_off_by(message):
"""Check for the DCO Signed-off-by line (required for code changes).""" """Check for the DCO Signed-off-by line (required for code changes)."""
if "Signed-off-by:" not in message: if "Signed-off-by:" not in message:
@ -179,10 +229,11 @@ def main():
validators = [ validators = [
("Regex pattern", check_regex), ("Regex pattern", check_regex),
("Subject ≤ 90 chars", check_subject_length), ("Subject ≤ 70 chars", check_subject_length),
("No trailing period in subject", check_subject_no_trailing_dot), ("No trailing period in subject", check_subject_no_trailing_dot),
("Subject capitalized", check_subject_capitalized), ("Subject capitalized", check_subject_capitalized),
("Blank line after subject", check_body_blank_line), ("Blank line after subject", check_body_blank_line),
("Body wrapped at 76 chars", check_body_line_length),
] ]
all_ok = True all_ok = True

136
scripts/test_check_commit.py Normal file
View File

@ -0,0 +1,136 @@
#!/usr/bin/env python3
"""Tests for scripts/check-commit.
Run with:
python3 scripts/test_check_commit.py
Covers the body line-wrapping validator added to enforce the commit body
wrap rule documented in .serena/memories/workflow/creating-commits.md.
"""
import importlib.machinery
import importlib.util
import pathlib
import sys
import unittest
# Loading scripts/check-commit would otherwise emit scripts/__pycache__/.
sys.dont_write_bytecode = True
SCRIPT_PATH = pathlib.Path(__file__).resolve().parent / "check-commit"
def load_check_commit():
"""Load the extensionless scripts/check-commit as a module."""
loader = importlib.machinery.SourceFileLoader("check_commit", str(SCRIPT_PATH))
spec = importlib.util.spec_from_loader("check_commit", loader)
module = importlib.util.module_from_spec(spec)
loader.exec_module(module)
return module
check_commit = load_check_commit()
class BodyLineLengthTests(unittest.TestCase):
def assert_ok(self, message):
ok, error = check_commit.check_body_line_length(message)
self.assertTrue(ok, error)
self.assertIsNone(error)
def assert_fail(self, message):
ok, error = check_commit.check_body_line_length(message)
self.assertFalse(ok)
self.assertIsNotNone(error)
return error
def test_wrapped_body_passes(self):
message = (
":bug: Fix crash when opening the file menu\n"
"\n"
"The menu reused a stale reference after the file was\n"
"closed, which raised an exception on reopen.\n"
)
self.assert_ok(message)
def test_line_at_limit_passes(self):
line = "x " * 38 # 76 chars, breakable
self.assertEqual(len(line), 76)
self.assert_ok(":bug: Fix crash\n\n" + line + "\n")
def test_line_one_over_limit_fails(self):
line = "x " * 38 + "x" # 77 chars, breakable
self.assertEqual(len(line), 77)
error = self.assert_fail(":bug: Fix crash\n\n" + line + "\n")
self.assertIn("76", error)
def test_long_body_line_fails(self):
long_line = "word " * 20 # 100 chars, breakable
error = self.assert_fail(":bug: Fix crash\n\n" + long_line + "\n")
self.assertIn("76", error)
self.assertIn("line 3", error)
def test_subject_is_not_checked(self):
# The subject has its own length rule; the body validator ignores it.
subject = ":bug: " + "S" * 100
self.assert_ok(subject + "\n")
def test_url_line_passes(self):
line = (
"See https://github.com/penpot/penpot/issues/1234"
"/comments/very/long/fragment"
)
self.assert_ok(":books: Update docs\n\n" + line + "\n")
def test_trailer_passes(self):
line = "Signed-off-by: Someone With A Long Name <someone@example.com>"
self.assert_ok(":bug: Fix crash\n\nBody.\n\n" + line + "\n")
def test_unbreakable_token_passes(self):
line = "a" * 100 # no whitespace to wrap at
self.assert_ok(":bug: Fix crash\n\n" + line + "\n")
def test_blank_lines_are_ignored(self):
self.assert_ok(":bug: Fix crash\n\n\n\n")
def test_multiple_offenders_reported(self):
error = self.assert_fail(
":bug: Fix crash\n\n"
+ ("word " * 20)
+ "\n"
+ ("other " * 20)
+ "\n"
)
self.assertIn("line 3", error)
self.assertIn("line 4", error)
class SubjectRulesRegressionTests(unittest.TestCase):
"""Guard the pre-existing validators against accidental breakage."""
def test_valid_subject_passes_regex(self):
ok, error = check_commit.check_regex(":bug: Fix crash on startup")
self.assertTrue(ok, error)
def test_missing_emoji_fails_regex(self):
ok, _ = check_commit.check_regex("Fix crash on startup")
self.assertFalse(ok)
def test_trailing_dot_fails(self):
ok, _ = check_commit.check_subject_no_trailing_dot(":bug: Fix crash.")
self.assertFalse(ok)
def test_subject_at_70_chars_passes(self):
# ":bug: " is 6 chars, so 64 chars of text reach exactly 70.
ok, error = check_commit.check_subject_length(":bug: " + "S" * 64)
self.assertTrue(ok, error)
def test_subject_over_70_chars_fails(self):
ok, error = check_commit.check_subject_length(":bug: " + "S" * 65)
self.assertFalse(ok)
self.assertIn("70", error)
if __name__ == "__main__":
unittest.main(verbosity=2)