Closes#65. risk/medium, control code: adversarial review required before merge.
1. Ambiguous keys — parseArgs → rejectAmbiguousKeys runs before alias normalisation and returns ErrValidation when
any object, at any depth, has two keys equal under strings.EqualFold (exact duplicates included; the map round-trip in normalizeAliases would otherwise collapse them silently);
a top-level key differs from a dst struct json tag only in case (BRANCH).
2. Branch names — ValidateBranchName: ^[A-Za-z0-9][A-Za-z0-9._/-]*$ plus no refs/ prefix, HEAD, .., //, /., trailing / or ., .lock. Applied to file_write_branch.branch and to tbd_ship.branch + tbd_ship.base (base is the branch-protection probe key; refs/heads/main would read as unprotected).
Tests (internal/tools/strict_args_test.go): the issue's exact repro, exact duplicate, case-only mismatch, nested duplicate; 16 bad branch names on both tools; refs-prefixed tbd_ship base; each asserts ErrValidation AND zero requests reached a counting fake Gitea. Watched red before the fix. task check exit 0.
Not covered (out of scope, for the reviewer to judge): other tools taking a ref (file_delete.branch, branch_delete, pr_create head/base, release_create.target, workflow_run_trigger.ref, repo_update.default_branch) do not create refs from the name and are unvalidated. Alias pairs (repo/name, number/index) still resolve canonical-wins.
Closes #65. risk/medium, control code: adversarial review required before merge.
**1. Ambiguous keys** — `parseArgs` → `rejectAmbiguousKeys` runs before alias normalisation and returns `ErrValidation` when
- any object, at any depth, has two keys equal under `strings.EqualFold` (exact duplicates included; the map round-trip in `normalizeAliases` would otherwise collapse them silently);
- a top-level key differs from a dst struct json tag only in case (`BRANCH`).
**2. Branch names** — `ValidateBranchName`: `^[A-Za-z0-9][A-Za-z0-9._/-]*$` plus no `refs/` prefix, `HEAD`, `..`, `//`, `/.`, trailing `/` or `.`, `.lock`. Applied to `file_write_branch.branch` and to `tbd_ship.branch` + `tbd_ship.base` (base is the branch-protection probe key; `refs/heads/main` would read as unprotected).
**Tests** (`internal/tools/strict_args_test.go`): the issue's exact repro, exact duplicate, case-only mismatch, nested duplicate; 16 bad branch names on both tools; refs-prefixed tbd_ship base; each asserts `ErrValidation` AND zero requests reached a counting fake Gitea. Watched red before the fix. `task check` exit 0.
**Not covered (out of scope, for the reviewer to judge):** other tools taking a ref (`file_delete.branch`, `branch_delete`, `pr_create head/base`, `release_create.target`, `workflow_run_trigger.ref`, `repo_update.default_branch`) do not create refs from the name and are unvalidated. Alias pairs (`repo`/`name`, `number`/`index`) still resolve canonical-wins.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---
_Created via git-mcp on behalf of @mathias_
parseArgs now refuses, before any request: two keys equal under
strings.EqualFold in any object (exact duplicates included), and a
top-level key that differs from a schema field only in case. Without
this, encoding/json's case-insensitive last-wins match let
{"branch":"feat/x","Branch":"main"} write to main while a client-side
hook read feat/x.
file_write_branch (branch) and tbd_ship (branch and base) validate the
branch name: ^[A-Za-z0-9][A-Za-z0-9._/-]*$ plus no refs/ prefix, HEAD,
"..", "//", "/.", trailing "/" or ".", or ".lock". refs/heads/main
otherwise created refs/heads/refs/heads/main and wedged API writes to
main. tbd_ship's base is the protection-probe key, so a refs/-prefixed
base would also have read as unprotected.
Closes#65
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pr-gatekeeper verdict — BLOCKED (PR mode, head ae50501)
Finding 1 (blocking): the ambiguity check fails open, and the issue's own repro gets through. internal/tools/tool.gorejectAmbiguousKeys: when scanJSONValue returns any non-ErrValidation error it does return nil // malformed JSON: let json.Unmarshal report it as before — skipping both the duplicate-key scan and the schema-case check. json.Decoder.Token() (no UseNumber) errors on a number that overflows float64, but json.Unmarshal into the args struct silently skips an unknown key holding that number. So one junk key placed before the duplicates disables the control:
Executed against FileWriteBranch.Call with a fake Gitea: err=<nil>, result branch=main, requests GET /repos/probe/t/branches/main, POST /repos/probe/t/contents/case8.txt — the write lands on main. Exact duplicates ("branch":"feat/y8","branch":"main") and -1e309 likewise pass. This is the #65 repro plus 8 bytes; the "done when" clause is not met.
Verified fix direction (in a scratch copy): dec.UseNumber() makes both cases return ErrValidation. But also make the scan fail closed: any scan error should be a rejection, not a skip — a parser that can't read the input must not be read as "no ambiguity found". Add a test with a 1e999 decoy key; the current suite stays green with and without the fail-open line.
Finding 2 (spec deviation, not exploitable as far as I found): the dup detector folds with strings.ToLower, while the issue and the function's doc comment say strings.EqualFold (encoding/json folds via SimpleFold — ſ/s, K(U+212A)/k). At top level the schema-case check (which does use EqualFold) covers it — probed meſſage, ſha: both rejected. In nested objects the gap is harmless today (only nested object is workflow_run_trigger's inputs map, where keys are matched exactly). Either use an EqualFold-equivalent fold or correct the comment so it claims what the code does.
Notes, non-blocking / follow-up candidates:
file_write_branch's base is not validated (it is only a source for old_branch_name, so no ref is created from it — low risk).
tag_create/release_create tag names are unvalidated; a tag named refs/heads/main would create refs/tags/refs/heads/main, plausibly the same "matches more than one" wedge. Not in #65's scope; not tested against a real Gitea.
heads/main / tags/x as branch names pass the validator; not proven harmful.
The scan also rejects case-distinct keys inside workflow_run_trigger.inputs (e.g. Env + env), which are not ambiguous for a map — over-rejection.
Flip tests (scratch copy, not the checkout): removing the rejectAmbiguousKeys call, the schema-case check, ValidateBranchName's body, the refs/ clause, base validation in tbd_ship, and the ToLower fold each turned the new tests red. Changing the fail-open return nil turns nothing red — no test covers it.
CI: commit status for ae50501 is pending with zero statuses/runs — absent. Local go test ./... passes; green covers none of the above.
Not checked: a real Gitea (no scratch instance used); the MCP transport layer's own pre-parsing beyond what parseArgs sees; localtools/mint_service_account_token.go, which decodes without parseArgs.
## pr-gatekeeper verdict — BLOCKED (PR mode, head ae50501)
**Finding 1 (blocking): the ambiguity check fails open, and the issue's own repro gets through.**
`internal/tools/tool.go` `rejectAmbiguousKeys`: when `scanJSONValue` returns any non-ErrValidation error it does `return nil // malformed JSON: let json.Unmarshal report it as before` — skipping *both* the duplicate-key scan and the schema-case check. `json.Decoder.Token()` (no `UseNumber`) errors on a number that overflows float64, but `json.Unmarshal` into the args struct silently skips an unknown key holding that number. So one junk key placed before the duplicates disables the control:
```
{"owner":"probe","repo":"t","name":"t","path":"case8.txt","content":"x","message":"m","z":1e999,"branch":"feat/y8","Branch":"main"}
```
Executed against `FileWriteBranch.Call` with a fake Gitea: `err=<nil>`, result `branch=main`, requests `GET /repos/probe/t/branches/main`, `POST /repos/probe/t/contents/case8.txt` — the write lands on main. Exact duplicates (`"branch":"feat/y8","branch":"main"`) and `-1e309` likewise pass. This is the #65 repro plus 8 bytes; the "done when" clause is not met.
Verified fix direction (in a scratch copy): `dec.UseNumber()` makes both cases return ErrValidation. But also make the scan **fail closed**: any scan error should be a rejection, not a skip — a parser that can't read the input must not be read as "no ambiguity found". Add a test with a `1e999` decoy key; the current suite stays green with and without the fail-open line.
**Finding 2 (spec deviation, not exploitable as far as I found):** the dup detector folds with `strings.ToLower`, while the issue and the function's doc comment say `strings.EqualFold` (encoding/json folds via SimpleFold — `ſ`/`s`, `K`(U+212A)/`k`). At top level the schema-case check (which does use EqualFold) covers it — probed `meſſage`, `ſha`: both rejected. In nested objects the gap is harmless today (only nested object is workflow_run_trigger's `inputs` map, where keys are matched exactly). Either use an EqualFold-equivalent fold or correct the comment so it claims what the code does.
**Notes, non-blocking / follow-up candidates:**
- `file_write_branch`'s `base` is not validated (it is only a source for `old_branch_name`, so no ref is created from it — low risk).
- `tag_create`/`release_create` tag names are unvalidated; a tag named `refs/heads/main` would create `refs/tags/refs/heads/main`, plausibly the same "matches more than one" wedge. Not in #65's scope; not tested against a real Gitea.
- `heads/main` / `tags/x` as branch names pass the validator; not proven harmful.
- The scan also rejects case-distinct keys inside `workflow_run_trigger.inputs` (e.g. `Env` + `env`), which are not ambiguous for a map — over-rejection.
**Flip tests** (scratch copy, not the checkout): removing the `rejectAmbiguousKeys` call, the schema-case check, `ValidateBranchName`'s body, the `refs/` clause, base validation in tbd_ship, and the ToLower fold each turned the new tests red. Changing the fail-open `return nil` turns nothing red — no test covers it.
**CI:** commit status for ae50501 is `pending` with zero statuses/runs — absent. Local `go test ./...` passes; green covers none of the above.
**Not checked:** a real Gitea (no scratch instance used); the MCP transport layer's own pre-parsing beyond what parseArgs sees; `localtools/mint_service_account_token.go`, which decodes without parseArgs.
PR #66 review: json.Decoder.Token() fails on a number outside float64
range (1e999), which json.Unmarshal ignores in an unknown key, so a
decoy key ended the scan and the #65 repro wrote to main. The decoder
now uses UseNumber, and any scan error is a validation error. The
duplicate check folds with strings.EqualFold (as encoding/json does)
instead of ToLower, so long s and the Kelvin sign are caught in nested
objects too.
Refs #65
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pr-gatekeeper verdict, round 2 — code APPROVED, NOT MERGED (PR mode, head 31a3dd3)
Round-1 blocker closed (checked by running it). Against parseArgs at 31a3dd3:
the #65 repro with a "z":1e999 decoy, an exact duplicate after 1e999999999999, and a nested Kelvin-sign duplicate ("Ka" and "ka") all return ErrValidation
whitespace-only input returns ErrValidation (the scan now fails closed); trailing data after the object is still rejected by Unmarshal
an ordinary file_write_branch call still succeeds end to end against a fake Gitea
Flip tests on the round-2 changes (in a scratch copy):
EqualFold switched back to ToLower → nested long-s duplicate goes red.
UseNumber and fail-closed removed together → both decoy cases go red.
Removing either one alone keeps the suite green. Each layer is enough on its own for the decoy, so this is defence in depth that the tests check only as a pair, not a gap. Neither layer has a test of its own.
Non-blocking finding — the duplicate check is now quadratic. The pairwise EqualFold loop is O(n²) in keys per object. Under the server's 1 MiB body cap, one request with ~90k distinct keys takes 7.2 s of CPU in parseArgs (10k keys: 0.09 s, 30k: 0.8 s). Only authenticated callers can send one, so it is a CPU-amplification nuisance, not a bypass. Fix: key a map on a canonical fold (the smallest rune in each unicode.SimpleFold orbit, as encoding/json does), which brings it back to O(n). Should be filed as a follow-up.
Gate: in a scratch copy at 31a3dd3, CI=1 task check exits 0: lint 0 issues, all packages pass with -race, 0 --- FAIL. Plain task check there exits 201 only because scripts/context-sync.sh cannot find the root AGENT.md outside ~/dev; this PR does not touch the derived adapters.
CI: absent..gitea/workflows/cd.yml runs only on push to main and v* tags, so the PR head has no runs. Merging deploys straight away. I was authorised to merge only if CI on the head commit was green, and it is not, so I have not merged. Whether a local gate is enough for this repo's PRs is the caller's / Mathias's call, not mine.
Not checked: a real Gitea; tag-name validation for tag_create and release_create (outside #65, author says it will be filed).
## pr-gatekeeper verdict, round 2 — code APPROVED, NOT MERGED (PR mode, head 31a3dd3)
**Round-1 blocker closed (checked by running it).** Against `parseArgs` at 31a3dd3:
- the #65 repro with a `"z":1e999` decoy, an exact duplicate after `1e999999999999`, and a nested Kelvin-sign duplicate (`"Ka"` and `"ka"`) all return ErrValidation
- whitespace-only input returns ErrValidation (the scan now fails closed); trailing data after the object is still rejected by Unmarshal
- an ordinary `file_write_branch` call still succeeds end to end against a fake Gitea
Both "done when" clauses of #65 hold.
**Flip tests on the round-2 changes** (in a scratch copy):
- EqualFold switched back to ToLower → `nested long-s duplicate` goes red.
- `UseNumber` and fail-closed removed together → both decoy cases go red.
- Removing **either one alone** keeps the suite green. Each layer is enough on its own for the decoy, so this is defence in depth that the tests check only as a pair, not a gap. Neither layer has a test of its own.
**Non-blocking finding — the duplicate check is now quadratic.** The pairwise EqualFold loop is O(n²) in keys per object. Under the server's 1 MiB body cap, one request with ~90k distinct keys takes **7.2 s** of CPU in `parseArgs` (10k keys: 0.09 s, 30k: 0.8 s). Only authenticated callers can send one, so it is a CPU-amplification nuisance, not a bypass. Fix: key a map on a canonical fold (the smallest rune in each `unicode.SimpleFold` orbit, as encoding/json does), which brings it back to O(n). Should be filed as a follow-up.
**Gate:** in a scratch copy at 31a3dd3, `CI=1 task check` exits 0: lint 0 issues, all packages pass with -race, 0 `--- FAIL`. Plain `task check` there exits 201 only because `scripts/context-sync.sh` cannot find the root AGENT.md outside ~/dev; this PR does not touch the derived adapters.
**CI: absent.** `.gitea/workflows/cd.yml` runs only on push to `main` and `v*` tags, so the PR head has no runs. Merging deploys straight away. I was authorised to merge only if CI on the head commit was green, and it is not, so **I have not merged**. Whether a local gate is enough for this repo's PRs is the caller's / Mathias's call, not mine.
**Not checked:** a real Gitea; tag-name validation for tag_create and release_create (outside #65, author says it will be filed).
PR #66 review round 2: the pairwise EqualFold loop was quadratic, 7s of
CPU for ~90k keys under the 1 MiB body cap. foldKey maps each rune to
the smallest member of its unicode.SimpleFold orbit, equal exactly when
strings.EqualFold is, so the scan is a map lookup again. The scaling
test fails on the previous code (7.1s).
Refs #65
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mathias
merged commit dc127a05b5 into main2026-10-05 05:10:51 +00:00
pr-gatekeeper verdict, round 3 — APPROVED and MERGED (PR mode, head cd61239)
foldKey matches strings.EqualFold (checked by running it). I compared foldKey(a)==foldKey(b) with strings.EqualFold(a,b) over 75.6M pairs, with 0 mismatches:
every rune 0..U+10FFFF against its whole SimpleFold orbit, its ±1/±32 neighbours, and all of A..z
all pairs of the 2,380 BMP runes that have a case fold
2M random 1-4 element strings built from s/S/ſ, k/K/U+212A, ß/ss/ẞ, Σ/σ/ς, İ/ı/i/I, µ/μ/Μ, Ω/U+2126, NUL, and invalid UTF-8 bytes (\xff, \xfe, \xc3) alongside U+FFFD
Speed: 90k keys scan in 0.45 s under -race (the quadratic version took 7.2 s for the same input).
Flip tests (scratch copy):
foldKey→ToLower and foldKey→identity both turn TestFoldKeyAgreesWithEqualFold and TestParseArgsRejectsCaseVariantKeys red.
Restoring a pairwise scan turns TestRejectAmbiguousKeysScalesLinearly red.
The checks from rounds 1 and 2 are still covered by the suite.
Gate: in a scratch copy at cd61239, CI=1 task check exits 0: lint 0 issues, 0 --- FAIL. I skipped the context-sync step because it can't find the root AGENT.md outside ~/dev, and this PR doesn't touch the derived adapters.
CI: absent before merge, by design..gitea/workflows/cd.yml runs only on push to main and v* tags. I merged on the local gate because the caller instructed it; CI on the merge commit is the first real CI signal, and merging rolls out the deploy. The caller confirms CI after merge.
Non-blocking: the 2 s timing threshold has about 4× headroom under -race, which is fine but could flake on a heavily loaded runner. Tag-name validation for tag_create and release_create is still a follow-up outside #65.
Merge commit:dc127a05 (merge style, pinned to head cd61239). No tag created.
Not checked: a real Gitea instance; the post-merge CI run.
## pr-gatekeeper verdict, round 3 — APPROVED and MERGED (PR mode, head cd61239)
**foldKey matches strings.EqualFold (checked by running it).** I compared `foldKey(a)==foldKey(b)` with `strings.EqualFold(a,b)` over 75.6M pairs, with 0 mismatches:
- every rune 0..U+10FFFF against its whole SimpleFold orbit, its ±1/±32 neighbours, and all of A..z
- all pairs of the 2,380 BMP runes that have a case fold
- 2M random 1-4 element strings built from s/S/ſ, k/K/U+212A, ß/ss/ẞ, Σ/σ/ς, İ/ı/i/I, µ/μ/Μ, Ω/U+2126, NUL, and invalid UTF-8 bytes (\xff, \xfe, \xc3) alongside U+FFFD
Spot checks: ß~ss false/false, ß~ẞ true/true, σ~ς true/true, \xff~\xfe true/true, İ~i false/false.
**Speed:** 90k keys scan in 0.45 s under -race (the quadratic version took 7.2 s for the same input).
**Flip tests** (scratch copy):
- foldKey→ToLower and foldKey→identity both turn `TestFoldKeyAgreesWithEqualFold` and `TestParseArgsRejectsCaseVariantKeys` red.
- Restoring a pairwise scan turns `TestRejectAmbiguousKeysScalesLinearly` red.
- The checks from rounds 1 and 2 are still covered by the suite.
**Gate:** in a scratch copy at cd61239, `CI=1 task check` exits 0: lint 0 issues, 0 `--- FAIL`. I skipped the context-sync step because it can't find the root AGENT.md outside ~/dev, and this PR doesn't touch the derived adapters.
**CI: absent before merge, by design.** `.gitea/workflows/cd.yml` runs only on push to main and v* tags. I merged on the local gate because the caller instructed it; CI on the merge commit is the first real CI signal, and merging rolls out the deploy. The caller confirms CI after merge.
**Non-blocking:** the 2 s timing threshold has about 4× headroom under -race, which is fine but could flake on a heavily loaded runner. Tag-name validation for tag_create and release_create is still a follow-up outside #65.
**Merge commit:** dc127a05 (merge style, pinned to head cd61239). No tag created.
**Not checked:** a real Gitea instance; the post-merge CI run.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #65. risk/medium, control code: adversarial review required before merge.
1. Ambiguous keys —
parseArgs→rejectAmbiguousKeysruns before alias normalisation and returnsErrValidationwhenstrings.EqualFold(exact duplicates included; the map round-trip innormalizeAliaseswould otherwise collapse them silently);BRANCH).2. Branch names —
ValidateBranchName:^[A-Za-z0-9][A-Za-z0-9._/-]*$plus norefs/prefix,HEAD,..,//,/., trailing/or.,.lock. Applied tofile_write_branch.branchand totbd_ship.branch+tbd_ship.base(base is the branch-protection probe key;refs/heads/mainwould read as unprotected).Tests (
internal/tools/strict_args_test.go): the issue's exact repro, exact duplicate, case-only mismatch, nested duplicate; 16 bad branch names on both tools; refs-prefixed tbd_ship base; each assertsErrValidationAND zero requests reached a counting fake Gitea. Watched red before the fix.task checkexit 0.Not covered (out of scope, for the reviewer to judge): other tools taking a ref (
file_delete.branch,branch_delete,pr_create head/base,release_create.target,workflow_run_trigger.ref,repo_update.default_branch) do not create refs from the name and are unvalidated. Alias pairs (repo/name,number/index) still resolve canonical-wins.🤖 Generated with Claude Code
Created via git-mcp on behalf of @mathias
parseArgs now refuses, before any request: two keys equal under strings.EqualFold in any object (exact duplicates included), and a top-level key that differs from a schema field only in case. Without this, encoding/json's case-insensitive last-wins match let {"branch":"feat/x","Branch":"main"} write to main while a client-side hook read feat/x. file_write_branch (branch) and tbd_ship (branch and base) validate the branch name: ^[A-Za-z0-9][A-Za-z0-9._/-]*$ plus no refs/ prefix, HEAD, "..", "//", "/.", trailing "/" or ".", or ".lock". refs/heads/main otherwise created refs/heads/refs/heads/main and wedged API writes to main. tbd_ship's base is the protection-probe key, so a refs/-prefixed base would also have read as unprotected. Closes #65 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>pr-gatekeeper verdict — BLOCKED (PR mode, head
ae50501)Finding 1 (blocking): the ambiguity check fails open, and the issue's own repro gets through.
internal/tools/tool.gorejectAmbiguousKeys: whenscanJSONValuereturns any non-ErrValidation error it doesreturn nil // malformed JSON: let json.Unmarshal report it as before— skipping both the duplicate-key scan and the schema-case check.json.Decoder.Token()(noUseNumber) errors on a number that overflows float64, butjson.Unmarshalinto the args struct silently skips an unknown key holding that number. So one junk key placed before the duplicates disables the control:Executed against
FileWriteBranch.Callwith a fake Gitea:err=<nil>, resultbranch=main, requestsGET /repos/probe/t/branches/main,POST /repos/probe/t/contents/case8.txt— the write lands on main. Exact duplicates ("branch":"feat/y8","branch":"main") and-1e309likewise pass. This is the #65 repro plus 8 bytes; the "done when" clause is not met.Verified fix direction (in a scratch copy):
dec.UseNumber()makes both cases return ErrValidation. But also make the scan fail closed: any scan error should be a rejection, not a skip — a parser that can't read the input must not be read as "no ambiguity found". Add a test with a1e999decoy key; the current suite stays green with and without the fail-open line.Finding 2 (spec deviation, not exploitable as far as I found): the dup detector folds with
strings.ToLower, while the issue and the function's doc comment saystrings.EqualFold(encoding/json folds via SimpleFold —ſ/s,K(U+212A)/k). At top level the schema-case check (which does use EqualFold) covers it — probedmeſſage,ſha: both rejected. In nested objects the gap is harmless today (only nested object is workflow_run_trigger'sinputsmap, where keys are matched exactly). Either use an EqualFold-equivalent fold or correct the comment so it claims what the code does.Notes, non-blocking / follow-up candidates:
file_write_branch'sbaseis not validated (it is only a source forold_branch_name, so no ref is created from it — low risk).tag_create/release_createtag names are unvalidated; a tag namedrefs/heads/mainwould createrefs/tags/refs/heads/main, plausibly the same "matches more than one" wedge. Not in #65's scope; not tested against a real Gitea.heads/main/tags/xas branch names pass the validator; not proven harmful.workflow_run_trigger.inputs(e.g.Env+env), which are not ambiguous for a map — over-rejection.Flip tests (scratch copy, not the checkout): removing the
rejectAmbiguousKeyscall, the schema-case check,ValidateBranchName's body, therefs/clause, base validation in tbd_ship, and the ToLower fold each turned the new tests red. Changing the fail-openreturn nilturns nothing red — no test covers it.CI: commit status for
ae50501ispendingwith zero statuses/runs — absent. Localgo test ./...passes; green covers none of the above.Not checked: a real Gitea (no scratch instance used); the MCP transport layer's own pre-parsing beyond what parseArgs sees;
localtools/mint_service_account_token.go, which decodes without parseArgs.pr-gatekeeper verdict, round 2 — code APPROVED, NOT MERGED (PR mode, head
31a3dd3)Round-1 blocker closed (checked by running it). Against
parseArgsat31a3dd3:"z":1e999decoy, an exact duplicate after1e999999999999, and a nested Kelvin-sign duplicate ("Ka"and"ka") all return ErrValidationfile_write_branchcall still succeeds end to end against a fake GiteaBoth "done when" clauses of #65 hold.
Flip tests on the round-2 changes (in a scratch copy):
nested long-s duplicategoes red.UseNumberand fail-closed removed together → both decoy cases go red.Non-blocking finding — the duplicate check is now quadratic. The pairwise EqualFold loop is O(n²) in keys per object. Under the server's 1 MiB body cap, one request with ~90k distinct keys takes 7.2 s of CPU in
parseArgs(10k keys: 0.09 s, 30k: 0.8 s). Only authenticated callers can send one, so it is a CPU-amplification nuisance, not a bypass. Fix: key a map on a canonical fold (the smallest rune in eachunicode.SimpleFoldorbit, as encoding/json does), which brings it back to O(n). Should be filed as a follow-up.Gate: in a scratch copy at
31a3dd3,CI=1 task checkexits 0: lint 0 issues, all packages pass with -race, 0--- FAIL. Plaintask checkthere exits 201 only becausescripts/context-sync.shcannot find the root AGENT.md outside ~/dev; this PR does not touch the derived adapters.CI: absent.
.gitea/workflows/cd.ymlruns only on push tomainandv*tags, so the PR head has no runs. Merging deploys straight away. I was authorised to merge only if CI on the head commit was green, and it is not, so I have not merged. Whether a local gate is enough for this repo's PRs is the caller's / Mathias's call, not mine.Not checked: a real Gitea; tag-name validation for tag_create and release_create (outside #65, author says it will be filed).
pr-gatekeeper verdict, round 3 — APPROVED and MERGED (PR mode, head
cd61239)foldKey matches strings.EqualFold (checked by running it). I compared
foldKey(a)==foldKey(b)withstrings.EqualFold(a,b)over 75.6M pairs, with 0 mismatches:Spot checks: ß
ss false/false, ßẞ true/true, σς true/true, \xff\xfe true/true, İ~i false/false.Speed: 90k keys scan in 0.45 s under -race (the quadratic version took 7.2 s for the same input).
Flip tests (scratch copy):
TestFoldKeyAgreesWithEqualFoldandTestParseArgsRejectsCaseVariantKeysred.TestRejectAmbiguousKeysScalesLinearlyred.Gate: in a scratch copy at
cd61239,CI=1 task checkexits 0: lint 0 issues, 0--- FAIL. I skipped the context-sync step because it can't find the root AGENT.md outside ~/dev, and this PR doesn't touch the derived adapters.CI: absent before merge, by design.
.gitea/workflows/cd.ymlruns only on push to main and v* tags. I merged on the local gate because the caller instructed it; CI on the merge commit is the first real CI signal, and merging rolls out the deploy. The caller confirms CI after merge.Non-blocking: the 2 s timing threshold has about 4× headroom under -race, which is fine but could flake on a heavily loaded runner. Tag-name validation for tag_create and release_create is still a follow-up outside #65.
Merge commit:
dc127a05(merge style, pinned to headcd61239). No tag created.Not checked: a real Gitea instance; the post-merge CI run.