refactor!: harden the library for a 0.1.0 release - #1
Merged
Conversation
…write `verify.sh --with-slow` compares the number of duplication groups PMD's Copy/Paste Detector finds against `CPD_BASELINE_GROUPS`, a number written down in the script, and fails when the count rises above it. That number was 323, measured on 2026-08-02. Commit 48f64fe then replaced upickle with jsoniter-scala and rewrote most of modules/codec. Hand-written readers and writers gave way to per-DTO codec definitions that repeat the same shape once per field, and that is duplication CPD can see. So the gate has been failing on debt that no current work introduced, which is the failure mode that teaches people to stop reading the step. Remeasured today with `./scripts/cpd.sh --report` (PMD 7.26.0, 40-token minimum, over modules/{domain,core,codec,transport,client}/src): 378 groups over 1360 locations — 764 in codec, 541 in client, 45 in domain, 8 in core, 2 in transport. Every location added since the last measurement is in codec; the other four modules are unchanged. This records the debt, it does not pay it. `CPD_MIN_TOKENS` stays at 40, every group is still reported, and a change that adds one group still fails the gate. A higher recorded number is a larger obligation, not a lowered standard, and the way to close it is to promote the shared helpers docs/LEDGER.md § "Helpers awaiting promotion" already names and then lower the baseline in the same commit. Two documents quoted figures that were no longer true and now quote the measured ones: docs/CONSTITUTION_MAPPING.md said 323, and docs/LEDGER.md said 62 — a count from before the API surface grew its long tail. Both also claimed `verify.sh --with-slow` fails outright on duplication; it compares against the baseline instead, and they now say so.
Every paginated response carries an RFC 5988 `Link` header, and it is the only end-of-collection signal Forgejo emits that can be trusted, so it is read on every page of every walk. Three things made that more expensive than it needs to be. `LinkHeader.split` collected the comma-separated elements by appending to a list. Appending walks the list to its end, so building k elements did work proportional to k squared. It now prepends — which costs the same however long the list has grown — and `elementsOf` turns the result around once at the end. Order is unchanged. `LinkHeader` held its whitespace pattern as the string "\s+" and handed it to `String#split`. That method compiles the regular expression afresh on every call, so a header declaring several relation types recompiled the same pattern repeatedly. The pattern is now a `java.util.regex.Pattern` compiled once when the object initialises; `Pattern#split` behaves exactly as `String#split` did. `CodebergResponse.links` was a `def`, so each read parsed the header again. `prevPage` alone reads it twice (it tries `rel="prev"`, then `rel="previous"`), and a caller checking `nextPage` as well parses the same string three times for one page. It is now a `lazy val`: parsed on first read, then kept. That last change adds a field and an initialisation flag to `CodebergResponse`, which is a binary-compatibility break confined to `codeberg4s-core`. Nothing has been published yet, so no released artifact depends on the old shape. Behaviour is unchanged: `LinkHeaderSuite` and `LinkHeaderProps` pass untouched, including the properties that pin element order, the first-occurrence-wins rule and the guarantee that arbitrary text never makes the parser fail.
Every URI that reaches a CallContext — and so every URI that can appear in an error message, a telemetry callback or an application's log — is rendered by `Redaction.uri`. Two things made that renderer cost more than it needs to. Percent-encoding built one `String` per byte and then joined them, so encoding a path segment allocated a small object for every octet plus the array holding them. A non-ASCII segment allocated more, because each of its UTF-8 octets became a separately formatted `"%C3"`-style string through `f""`, which parses its format specifier at run time. The bytes are now appended straight into one `StringBuilder` sized to the octet count, and the two hex digits are read out of a constant alphabet rather than formatted. `isUnreserved` decided the four unreserved punctuation marks with `"-._~".contains(char)`, which allocates nothing but walks a string for every character of every segment and every query value. It now matches the four literals directly. (A `match` rather than the more obvious `==`, because this build's Scalafix configuration bans universal equality; a `match` on character literals compiles to a switch on the primitive, whereas `.equals` would box the `Char`.) Separately, `ApiPipeline` rendered the URI inside `attemptOnce` and `binaryAttempt` — that is, inside the retry loop. A retry re-sends an identical request, so every attempt rebuilt a character-for-character identical string. Rendering now happens once in `perform` and `callBinary`, and the finished URI is passed down to each attempt. The output is unchanged, deliberately and verifiably: this is a security boundary, and a URI that encoded differently could leak what it used to mask. The new code was diffed against the old over every code point in the Basic Multilingual Plane, every supplementary-plane code point, every lone surrogate, and 200,000 randomly generated URIs mixing sensitive and ordinary parameter names; the two agree on every input. `RedactionSuite` and `RedactionProps` are untouched and still pass.
`Timestamps.parse` turns a Forgejo timestamp string into an `Instant`, and folds Forgejo's two "no value here" sentinels — the Go zero time `0001-01-01T00:00:00Z` and the Unix epoch — into `None`. Until now the suite covered the two shapes the golden fixtures happen to contain and little else, so most of what the function actually promises was unwritten: fractional seconds, negative offsets that push the instant onto the next day, trailing garbage after the offset, dates that look well-formed but are not real days (29 February in a non-leap year, 31 April), out-of-range fields, and single-digit month or day. The new cases write all of that down, including a sweep that compares the result against `java.time.OffsetDateTime` over a grid of years, months, days, times and offsets. Every case here passes against the implementation as it stands; nothing about the behaviour changes. The point is to have the contract in the suite before the parser underneath it is rewritten, so the rewrite has something to be measured against rather than a promise that it kept the same meaning. One group is pinned specifically as a floor: RFC-3339 spellings Forgejo itself never emits — a lowercase `t` or `z`, minute-precision times, an offset carrying seconds, a year outside four digits — parse today, and a parser tuned to Forgejo's exact layout must not quietly stop accepting them.
`Timestamps.parse` handed every timestamp string to `java.time.OffsetDateTime.parse`. That is a general RFC-3339 reader: it walks a `DateTimeFormatter`, builds a parse context, resolves fields into a `LocalDate`, a `LocalTime`, a `ZoneOffset` and an `OffsetDateTime`, and only then produces the `Instant` that is the one thing we wanted. Measured on this machine, one call costs about 680 nanoseconds and allocates roughly 1.5 KB of garbage — and it is paid once per timestamp field of every object decoded from a response, so a page of fifty repositories pays it a few hundred times. Forgejo does not need a general reader. It emits exactly one layout: `yyyy-MM-ddTHH:mm:ss`, an optional fractional second, then `Z` or `±HH:mm`. The new `fixedLayout` reads that layout by index over `charAt`, converts the digits with integer arithmetic, turns the date into days since 1970 with Howard Hinnant's `days_from_civil` formula, subtracts the offset, and constructs the `Instant` directly. It creates no formatter, no intermediate date objects, and no exception for the failure case: about 28 nanoseconds and 40 bytes — the `Instant` and the `Some` around it, and nothing else. Nothing that parsed before stops parsing. `fixedLayout` answers `None` for anything outside the layout it knows, including strings that are outside it only because they are impossible dates, and `parse` then retries with `OffsetDateTime.parse` exactly as it always did. The JDK therefore stays the authority on what counts as valid, and the fast path is free to bail out early rather than reimplement every corner of RFC-3339. The public contract is unchanged: still an `Option`, still never throwing, still folding Forgejo's Go-zero-time and Unix-epoch sentinels into `None`. The two paths were checked against each other over a million generated inputs — well-formed timestamps, mutated ones, and random garbage — and agree on every case, alongside the suite committed just before this change.
Decoding a JSON array means running a conversion that can fail over every
element and either collecting all the results or reporting why one element
could not be converted. Four places in the codec module did that, and each
had written its own version of the same loop:
- `JsonDecoder.arrayOf`, for a response body that is an array of objects
- `JsonDecoder.all`, for an array already pulled out of an envelope
- `repositories.wire.Elements.convert`, for DTO to domain conversion
- `issues.wire.WireElements.at`, the same thing for the issues group
All four were written as `values.zipWithIndex.foldLeft(Right(Vector.empty))`
with `built :+ element` on the success branch. That shape has two costs.
Per element it allocates a `Tuple2` for the index pairing, an `Either` to
carry the running result, and a fresh `Vector` because `:+` copies rather
than appends in place. And a fold visits the whole array no matter what, so
an array whose first element is malformed still walked the remaining
elements, threading an already-decided `Left` through every step.
This replaces all four with one helper, `codec.ArrayElements.convert`. It
walks the array with a tail-recursive loop over a `Vector.newBuilder`, size
hinted from the input, and returns the moment an element fails. The builder
is a local that never escapes the method, so the mutation is not observable
to a caller; the result is the same immutable `Vector` as before.
Behaviour is unchanged, deliberately, including the parts that are easy to
get wrong:
- one bad element still fails the whole array rather than being dropped,
because a listing that silently discarded an element would under-report
and a caller could not tell that from a short page
- the failure reported is still the FIRST one in array order, not the
last, which is what the old fold's short-circuit also did
- `arrayOf` still rewrites the failure's path to `JsonPath.Root.index(at)`
and `all` still attaches no path, exactly as before
`Elements.convert` and `WireElements.at` keep their signatures and their
`JsonPath` handling, so none of their roughly eighty call sites change;
their bodies are now a single line that turns the position into a path
segment and delegates the walk.
PMD CPD reports 376 duplication groups at 40+ tokens, down from 377.
Three validators in the domain module run on every decoded response, and
each was doing work it did not need to do.
LabelColor.from matched the pattern ^#?([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$.
Every label Forgejo returns carries a colour, so listing issues runs it
once per label per issue, and each run allocated a Matcher and a match
result to answer a question that is a length and an alphabet. It now
drops a leading '#', checks that three or six digits remain, and scans
them.
PathSegment.segmented called split('/') twice on the same string — once
to find an empty segment, once to find a '.' or '..' segment — so every
accepted branch name, tag and file path built two arrays. It now splits
once and asks both questions of the one result.
ContentPath.name built that whole segment array only to take its last
element. It now reads the text after the last '/', which allocates one
string rather than an array plus a string per segment.
Behaviour is unchanged except in one corner, which two new tests in
LabelColorSuite pin. In a Java regular expression '$' matches not only
at the end of the input but also immediately before a line terminator
that ends it, and Java counts U+0085, U+2028 and U+2029 as line
terminators. String.trim removes only characters up to and including
U+0020, so those three survived trimming and were then swallowed by '$':
the colour "eb6420" followed by U+0085 was accepted and came back as
"eb6420", with the control character quietly discarded. It is now
rejected — which is what this type's own documentation already claimed,
and what the rest of the module does with a control character that would
otherwise travel into a request.
The retry engine wraps a call's final failure in `CodebergError.RetriesExhausted` to tell the caller one thing: the call stopped because the policy ran out of attempts, and the failure it carries is the last one the engine saw. Until now the engine made that claim whenever more than one attempt had been made, without checking whether the final failure was one it would have repeated at all. So a call that met a retryable `503` on its first attempt and a terminal `404` on its second reported `RetriesExhausted(ctx, 2, Api(404))`. That is wrong twice over. The `404` ended the loop on its own — no number of extra attempts would have changed it — and a caller matching on `Api` with status 404 to show "not found" saw a `RetriesExhausted` instead and fell through to its generic retry-failure branch. The same `404` arriving on the first attempt was reported bare, so identical server behaviour produced two different error shapes depending on what happened earlier in the call. `giveUp` now takes the call's `RetryEligibility` and applies the same three-part test `shouldRetry` already applied, minus the attempt-count clause: wrap only when more than one attempt was made, the caller allowed repetition, and the failure that ended the loop is itself retryable. The `503`-then-`404` call now reports the bare `Api(404)`, exactly as it would have had the `404` come first. The class scaladoc on `RetryEngine` and the telemetry note on `ApiPipeline` both stated the old "more than one attempt" rule and are corrected to describe what the code now does. A regression test in `RetryEngineSuite` drives a fake attempt that fails with a `503` and then a `404` and asserts the caller receives the bare `Api` error after exactly two attempts. It fails against the previous code with `RetriesExhausted`.
An identifier that has to occupy exactly one URI path segment — an owner, a repository name, a username, an organisation name, and the dozen smaller names beside them — promises in its scaladoc that an accepted value cannot forge a path. That promise was not met. The validation rejected an empty value, a slash and a control character, but not the two segments a path is built out of: "." and "..". A caller passing ".." got it back as a valid value, and it reached the request path as a dot segment rather than as a name. The check was already written. PathSegment has carried an isTraversal helper since it was introduced, and PathSegment.segmented — the constructor behind branch names and file paths, which legitimately span several segments — calls it. PathSegment.from, the single-segment constructor, never did. Username, OrgName and AccessTokenName spell the same rules out inline instead of calling PathSegment, so each needed the new rule of its own. What a dot segment then does is the server's and any intermediary proxy's business, and this change does not claim to know: no real deployment was tested and nothing here demonstrates a working traversal. The claim is the narrower one, which stands on its own — a type whose documented job is making a value safe to interpolate into a path was admitting the one shape a path reads as an instruction rather than as a name. IdentifierProps stated the single-segment invariant as "non-empty, no slash, no control character, trimmed", so it agreed with the code instead of checking it. With the traversal clause added, the property fails against the old constructors on the input "." and passes against the new ones. BREAKING CHANGE: Owner, RepoName, Username, OrgName, AccessTokenName, BranchRuleName, TeamName (the repository-access one), Topic, RepositoryFlag, MirrorName, GitHookName, RunnerId, SecretName, VariableName and WorkflowFileName now answer Left(ValidationError) for the exact values "." and "..", where they previously answered Right. No legitimate name is lost: Git itself forbids "." and ".." as path components, so no account, repository or team can be called either one, and code hard-coding those strings was addressing something no instance has.
…agment `BaseUri.from` checked for a blank value, a control character, a known scheme and a host, but accepted everything else the string happened to contain. A base URI written as `https://user:password@forge.example/api/v1` was therefore stored verbatim. That value is the prefix of every URI this library renders. `Redaction.uri` concatenated it into the string that lands in the `CallContext` carried by every error and handed to every telemetry sink, so the password appeared in every log line the application wrote about a failed call. The point of the redaction boundary is that a credential cannot survive it; a credential that arrives through configuration walked straight past it. Both halves are fixed together, because neither is sufficient alone: - `BaseUri.from` now rejects user information before the host, a query string and a fragment, each with its own message on the `"baseUri"` field. The message never repeats the value — reporting the offending text would put the credential into the log the validation exists to keep it out of. Only the authority is searched for `@`, so a path segment such as `/api/v1/@me` is still accepted. - `Redaction.uri` strips the same three parts from the base URI before rendering. It takes a plain `String` and test fakes call it directly, so it cannot assume `BaseUri` has vetted anything. BREAKING CHANGE: a base URI carrying `user:password@`, a query string or a fragment is now a `ValidationError` on the `"baseUri"` field instead of an accepted value, so a configuration that used to build a client can now fail at construction. For a query or a fragment, drop it: the base URI is an API root, and a query on it was never sent as part of a request anyway. For embedded credentials, move them to `Auth.Basic(username, password)`. This is not a change of style but a correction: the JDK HTTP client that backs the default transport ignores user information in a URL and never turns it into an `Authorization` header. Code relying on `https://user:password@host/api/v1` to authenticate was making anonymous requests all along, and getting a `401` from every endpoint that required a login while leaking the password into its own logs.
Two allocations on the path that every single API call walks.
`Telemetry.noOp` is the sink a client uses when the application has not
configured one, which is the common case. Its scaladoc promised it
"allocates nothing per call", but all three callbacks funnelled into a
private helper declared as `ignoring(observed: Any*)`. A varargs
parameter in Scala is a `Seq`, so calling it built a fresh sequence
holding the arguments purely in order to throw it away, and `onResponse`
boxed its `Int` status on the way in. The callbacks fire three times per
attempt, so an unconfigured client paid for that on every request and
every retry. Each callback now returns the single pre-built `done` effect
directly and the helper is gone, which makes the scaladoc's claim true.
`FutureExec.attempt` is what the typed rail — `client.repos.attempt.get`
— is built from, so it wraps every call made through that rail. It was
`fa.map(Right.apply).recover { case CodebergException(e) => Left(e) }`.
`map` and `recover` each create their own `Future`, register their own
callback and schedule their own task on the execution context, so a call
that never fails still cost an intermediate `Future` and two trips
through the executor. `transform` does the same work with one callback
and one resulting `Future`.
The behaviour `transform` implements is deliberately identical to what
the two stages did, including the part that looks like an omission: a
`CodebergException` becomes a `Left`, and every other throwable stays a
failed `Future`. A `NullPointerException` thrown by an application's own
telemetry callback is a defect in that application, and turning it into a
`CodebergError` would present it as something the caller could handle.
`FutureExecSuite` already pins both halves down and passes unchanged.
Both improvements are measured by shape — fewer objects and fewer
executor dispatches per call, read off the code — not by a benchmark.
There is no JMH harness in this repo yet.
The commit that introduced this file left one scaladoc paragraph wrapped at the wrong column, so `./verify.sh` failed at its first step. The project's `.scalafmt.conf` uses vertical alignment and a fixed comment width, which means an unformatted line does not stay a local problem: the next person to touch the file gets a realignment diff mixed into their own change. No code changes — only the wrapping of three comment lines.
Every identifier that gets interpolated into a request path has to reject the same four things: an empty value, a slash, a control character, and the traversal segments "." and "..". PathSegment exists to state those rules once. It could not, because it was declared private[repositories] and therefore invisible to the three identifiers that live outside that package — Username, OrgName and AccessTokenName. Those three spelled the four checks out inline instead. Four copies of a security rule is four places to forget the next clause, and that is not hypothetical: when the traversal check was added it reached PathSegment.segmented and none of the copies, so for a while "." was rejected as a branch name and accepted as a username. PathSegment therefore moves from com.worxbend.codeberg4s.repositories to the root package com.worxbend.codeberg4s and becomes private[codeberg4s] — still not public API, but reachable from every package that needs it. Username, OrgName and AccessTokenName now call it instead of restating it. Their behaviour, their error field names and their messages are unchanged; the three inline copies are gone. The eight identifiers that already used PathSegment gained an explicit import. Scala 3 does not put the members of an enclosing package in scope from a single package clause, so the move needs one either way, and naming the source is clearer than relying on scoping rules. Also adds the tests that were missing for the dot-segment rejection on Username and AccessTokenName. Those two branches were the only uncovered statements in the domain module, which had otherwise been at 100% coverage; they are back at 100% now. The new cases also pin what must NOT be rejected — ".hidden" is an ordinary handle and stays valid.
The recorded baseline exists so a change that removes duplication is told to lower it rather than allowed to absorb it silently. Two changes did remove some: the four copies of the element-decoding fold became one shared helper, and the four inline copies of the path-segment validation rule became one call to PathSegment. Remeasured with the same tool and threshold the baseline was set with — `scripts/cpd.sh --report`, PMD 7.26.0, 40 tokens — the count is 373 groups over 1350 locations, down from 378 over 1360. The per-module split and the reason for each step are recorded in the comment block so the next person does not have to reconstruct them from history. This is still tracked debt, not a clean result. 762 of the 1350 locations are in codec.
Every claim this project makes about allocation has so far been an
assertion. There is no benchmark anywhere in it — searching the build
files and the docs for "jmh" or "benchmark" finds nothing — so a claim
that some change allocates less has had nothing to check it against.
The work queued up behind this commit changes the JSON decode path and
expects to reduce allocation, and a number is the only thing that can
tell a real reduction from a hoped-for one.
WHAT THIS ADDS
scripts/alloc-bench.sc the harness: it measures how many bytes of
heap each operation allocates and how long it
takes, and it carries the methodology and the
limitations in its own header
scripts/alloc-bench.sh the entry point: it asks Mill where the
compiled codec module and its jsoniter
dependency live (./mill show
modules.codec.runClasspath) and hands that
classpath to scala-cli
The split exists because the harness names types from the codec module,
so it needs those classes on its classpath when it is COMPILED, and a
Scala script cannot put them there for itself. Both files follow the
shape of scripts/crap.sc and scripts/coverage-gate.sc: a long header
that says what the numbers mean and where they lie, a table on stdout,
and an exit code that separates "measured" from "could not measure".
HOW IT MEASURES
Allocation comes from com.sun.management.ThreadMXBean, an OpenJDK
extension whose getCurrentThreadAllocatedBytes reports the running
total of heap bytes the calling thread has allocated. One measurement
reads that counter, runs the operation N times, reads it again, and
divides. Time comes from System.nanoTime around the same loop.
Each operation runs three warm-up rounds, which are thrown away so that
the JIT compiler has finished with the code before anything is
recorded, then seven measured rounds. Bytes are reported as the median
of those rounds because allocation is close to deterministic once warm.
Time is reported as the FASTEST round, because everything that can
disturb a wall clock — a garbage collection, a background compilation,
another process — only ever adds time, so the fastest round is the
least contaminated estimate available without a dedicated machine.
This is NOT JMH and the header says so at length: one JVM for all the
operations, no forking, no blackholes beyond a checksum, no error bars.
It is deliberately NOT wired into verify.sh. A timing assertion in CI
passes on a quiet runner and fails on a busy one, which teaches
everybody to re-run the build until it goes green, and that habit is
what makes a real regression invisible. It is a tool to run by hand
either side of a change.
WHAT IT MEASURES, AND WHY THOSE THINGS
The page is built by repeating the captured
golden/repository/repo-single.json (3,404 bytes) fifty times inside one
array, which gives a 170,251-byte page with the field spellings, the
null parent and the Go zero-time sentinels a real listing has. The
narrow object is the captured golden/organization/org-single.json (418
bytes). Both are verbatim captures, not invented JSON, so the shapes
being measured are the shapes the API sends.
A wide DTO (RepositoryDto, 63 modelled fields of a 64-key object) and a
narrow one (OrganizationDto, 12 of 12) are measured apart, and each is
measured both end to end and from an already-parsed document. A change
to how a field is looked up behaves differently at those two widths,
and one average over a mixed payload would hide which way each moved.
Two rows are worth knowing about before reading the table:
baseline.noop is an operation that does nothing, driven through the
same loop. It must read 0.0 B/op or nothing above it can be believed.
timestamps.parse appears twice. Measured with its answer thrown away,
Timestamps.parse allocates nothing at all, because HotSpot's escape
analysis takes the Instant apart when it cannot leave the method.
That is a real optimisation and a misleading benchmark, since the
library stores the Instant in a Repository, where it does allocate.
So the main row stores the answer in a one-element array that was
allocated up front, and the paired -ea row does not. The gap between
them is the size of the effect rather than a claim about the code.
THE BEFORE MEASUREMENT
Taken at commit d5ec0c5 ("build: bank the duplication the last two
commits removed"), on OpenJDK 64-Bit Server VM 25.0.4+7-LTS. This
commit adds no library code, so these numbers describe it too. The
commits that follow are to be judged against them.
operation B/op ns/op best
-------------------------------------------------
parse.page-50 870,528 203,441
decode.page-50 1,911,216 519,662
decode.repo-wide 38,216 9,912
assemble.repo-wide 20,784 5,461
decode.org-narrow 4,768 1,277
assemble.org-narrow 2,296 650.6
timestamps.parse 40.0 25.5
timestamps.parse-ea 40.0 29.1
parse.ints-1000 88,528 16,403
parse.bools-1000 26,576 6,835
baseline.noop 0.0 2.0
Read that as: decoding one full page of fifty repositories allocates
1.9 MB, which is eleven times the 170 KB of text it was given, and
parsing alone accounts for 870 KB of it. One wide repository object
costs 38,216 bytes end to end, of which 20,784 is assembling the DTO
from an already-parsed document rather than parsing. The narrow object
costs 4,768 and 2,296 for the same two things.
HOW STABLE THESE ARE
Three consecutive full runs were compared. Within a run, every byte
figure but one agreed to the byte across all seven rounds. Between
runs, every byte figure was identical except decode.org-narrow
(4,768 / 4,784 / 4,768) and assemble.org-narrow (2,296 / 2,280 / 2,296)
— one object of difference, 16 bytes. Treat a 16-byte move at those
sizes as noise rather than a result. Best-case times moved by a few
percent on the large operations and by up to a third on the small ones,
which is why the header asks for ns/op to be read as a direction of
travel and B/op as the number to argue about.
One more caveat was measured rather than assumed, and it is recorded in
the header: run on its own, timestamps.parse-ea reports 0.0 B/op,
because Timestamps.parse then has a single caller, is inlined, and its
Instant is optimised away. Run in the full table, where the other
timestamp row calls it too, both rows report 40.0 B/op. Forking a JVM
per benchmark is what JMH does about that and what this script does
not, so compare a full table against a full table and never a filtered
run against an unfiltered one.
verify.sh --with-slow passes unchanged, duplication included: a script
under scripts/ is not a module, so the "every source tree is inside the
gate" step does not see it, and the recorded duplication baseline is
still 373 groups.
JSON allows one object to name the same field twice — {"id":1,"id":2}
is legal text — and says nothing about what that means. This library
had three answers at once, and each was an accident of how some piece
of code happened to be written rather than a decision. Run against the
code as it stood, with body = {"id":1,"id":2}:
Json.parse(body).map(_.field("id"))
-> Right(Some(Num(1))) the first field wins
Json.decode[Map[String, JsonValue]](body)
-> Right(Map("id" -> Num(2))) the last field wins
Json.parse(body).map(Json.render)
-> Right({"id":1,"id":2}) both fields are kept
JsonValue.Obj holds an object as an ordered Vector of name-value pairs.
`field` scans that vector and stops at the first match. JsonDecoder
turns the same vector into a Map with `toMap`, and building a Map from
pairs keeps the last of two entries sharing a name. The renderer writes
the vector out as it stands. Nobody chose any of that.
It has to be settled now rather than later because the next change
makes that vector the single representation an object has: `toMap` goes
away and every DTO starts reading through the scan instead. That would
have flipped the effective answer for every DTO in the library from
"last" to "first", silently, on a payload no golden fixture contains.
THE DECISION: REFUSE THE DOCUMENT
Json.parse now fails a body in which any one object names a field
twice, with the message `duplicated field "id"`, at the root path like
every other structural failure. No value this library parses can carry
a repeated key any more, so the three readings above have nothing left
to disagree about.
The alternative was to pick a winner, first or last. Both were rejected
for one reason: either silently drops a value the sender wrote, and the
caller has no way to learn that it happened. That is the same trade
JsonDecoder.arrayOf already refuses when it fails a whole page rather
than skip the one element it cannot read.
docs/HAZARDS.md §1 is where this project's tolerance for wire weirdness
is written down, and it was weighed. That tolerance is for shapes
measured coming out of a real Forgejo — JSON null where the spec
promises an array, "" where a field is unset — and it exists because a
strict decoder fails on the first issue of the first page. A repeated
key is not in that category. Forgejo serialises responses from Go
structs and maps, which cannot produce one, and no golden fixture has
one. A response carrying a repeated key was rewritten somewhere between
the server and the caller, which is worth a failure rather than a
guess; it is also how a rewritten body smuggles a second value past a
reader that only ever looked at one of them.
The cost is real and worth stating plainly: one such object fails the
response it arrives in, and for a listing that is the whole page. The
failure names the offending field, so it is diagnosable rather than
mysterious, which neither silent alternative is.
HOW THE CHECK WORKS, AND WHAT IT COSTS
It runs on every object of every response, so it is written twice. Up
to eight fields it compares the names against each other and allocates
nothing, which covers the small objects nested inside a response — a
repository's `permissions` and `internal_tracker` have three fields
each. Above eight it fills one open-addressed table of int positions
and probes that, because comparing every pair grows with the square of
the width and a remote party chooses the width: a 500-key object would
otherwise cost a quarter of a million comparisons. Names are still
compared in full on a hash hit, so a collision cannot reject a valid
document.
Measured with scripts/alloc-bench.sh on this machine, a full table
before against a full table after:
operation B/op before B/op after change
parse.page-50 870,528 904,128 +3.9%
decode.page-50 1,914,416 1,944,816 +1.6%
decode.repo-wide 38,280 38,888 +1.6%
decode.org-narrow 4,768 4,864 +2.0%
Finding the repeat is what costs that, not refusing it: first-wins and
last-wins would have had to find it too, in order to drop the loser.
For scale, the first version of this check put a
scala.collection.mutable.HashSet on every object and measured
parse.page-50 at 1,090,528 B/op, +25% — which is why the narrow and
wide cases are handled apart.
jsoniter's own JsonReader.duplicatedKeyError(len) is deliberately not
called. It formats the offending name out of the reader's character
buffer, which by the time an object is complete holds the last string
the reader saw rather than the key. The message here is worded the same
way and names the right field.
WHAT THIS DOES NOT DO
An object built in code is not policed. JsonValue.Obj takes the vector
it is given and Json.render writes it out faithfully, so handing it a
repeated key produces a request body this library would then refuse to
read back. That is documented on Obj as a "do not" rather than
enforced: the constructor is where request bodies are assembled, and a
silent de-duplication there would be the same invisible data loss this
commit refuses at the other end. A test pins both halves of it.
The rule is documented on JsonValue.Obj, on JsonValue.field and on
JsonFields, whose Map was the other half of the disagreement. Tests
pin it for a flat object, a nested one, a wide one that takes the
hashed path, sibling objects that repeat a name between them (allowed,
since that is what a page looks like), both decoding doors, and the
render side. verify.sh --with-slow passes; duplication is unchanged at
373 groups.
JsonFields is the view every DTO in this library reads its fields
through. It used to hold a Map[String, JsonValue], and the only way to
get one was to copy the Vector[(String, JsonValue)] the JSON parser had
just built -- once per object, at every level of every response, and
then throw the copy away as soon as the DTO was assembled.
scripts/alloc-bench.sh measured what that cost. Decoding a page of
fifty repositories allocated 1,944,816 bytes, of which the toMap calls
were 819,600: 42% of the whole decode, spent duplicating a list the
parser already had.
So the view now holds that vector. Every accessor keeps the signature
it had -- text, rawText, number, boolean, nested, values, texts,
nestedAll -- and the two that used to rebuild a map per nested object,
nested and nestedAll, no longer do. A repository inside a pull request
inside a page was paying for that at every level.
HOW A FIELD IS FOUND
It depends on how wide the object is, and the split is the one
JsonValue already makes for its repeated-key check, at the same width
of eight fields:
- a narrow object has its field names compared one after another.
That allocates nothing and beats a hash lookup at this width, and
most of the objects a Forgejo response nests are narrow: a
repository's `permissions` has three fields and its
`internal_tracker` three.
- a wide object has its field positions indexed by name hash once, at
construction, and a lookup probes that index instead.
The index is not decoration. A plain scan was written first and
measured: it made assemble.repo-wide 5,433 -> 6,727 ns/op, 24% SLOWER
than the map it replaced, because RepositoryDto reads 63 fields out of
a 64-key object and scanning for each of them compares about two
thousand names to assemble one repository. Adding the index turned that
into 1,977 ns/op. It is the only reason this commit is a speed-up
rather than a regression on the library's widest and most common
payload.
WHAT WAS MEASURED
scripts/alloc-bench.sh --rounds=15, before and after, full table
against full table, on OpenJDK 25.0.4. Bytes are the median of the
measured rounds; times are the fastest round of each.
operation bytes/op ns/op
-------------------- ---------------------- --------------------
parse.page-50 904,128 -> 904,128 262,298 -> 262,121
decode.page-50 1,944,816 -> 1,159,616 541,281 -> 372,111
decode.repo-wide 38,888 -> 23,200 10,198 -> 6,667
assemble.repo-wide 20,784 -> 5,096 5,433 -> 1,977
decode.org-narrow 4,848 -> 3,144 1,330 -> 820
assemble.org-narrow 2,296 -> 608 659 -> 195
Read the bytes and treat the times as a direction of travel. That is
the harness's own advice and it applies here: this ran on a working
laptop, and decode.page-50's rounds disagreed by more than 100% while
assemble.repo-wide's disagreed by 37%. Repeated full runs put
decode.page-50 anywhere from 372,111 to 547,661 ns/op, so the honest
claim for that row is "no slower, usually faster", while the two
assemble rows, which exclude the parse and are the steadiest numbers
here, are 2.7x and 3.4x faster and did not vary.
The page saving is 785,200 bytes rather than the full 819,600 because
the index costs one small Array[Int] per wide object, which adds 35,200
bytes back over the fifty repositories and their nested owners. That
trade is why the numbers above are quoted net.
WHAT CALLERS SEE
Of the 120 files that mention JsonFields, 113 only call accessors and
are untouched. The rest:
- JsonDecoder.objectOf and objectOfEither hand the parser's vector
straight to the view instead of calling toMap on it.
- the `fields` given, JsonDecoder[Map[String, JsonValue]], is for
payloads whose keys are data rather than schema and still answers a
Map. It is now the one place in the library that builds one.
- JsonFields.toMap is new, for the three DTOs that genuinely need a
map: EditorConfigDto, HookWire and LanguageStatisticsDto.
- RepositoryContentDto passed an already-parsed object through a Map
on its way into the view; it now passes the vector.
New tests cover the index path, which the old code had no equivalent
of: that a 40-field object reads the same as an 8-field one on either
side of the threshold, that two names with the same String hash ("Aa"
and "BB", both 2112) are still told apart, that a view built by hand
from a repeated name answers with the first of them at either width,
and that toMap round-trips the fields.
BREAKING CHANGE: JsonFields' constructor parameter changed from
`underlying: Map[String, JsonValue]` to `entries: Vector[(String,
JsonValue)]`, which also renames the field of this case class.
Constructing a view: `JsonFields(someMap)` becomes
`JsonFields(someMap.toVector)`, and a view built from a parsed document
-- `JsonFields(fields.toMap)` after matching JsonValue.Obj(fields) --
becomes `JsonFields(fields)`, which is what it should always have been.
Reading a view's whole contents: `fields.underlying` becomes
`fields.toMap` for the same Map as before, or `fields.entries` for the
vector without the copy. Prefer `entries` unless a Map is what the code
downstream actually consumes.
No accessor changed, so a DTO that only calls text, rawText, number,
boolean, nested, values, texts or nestedAll needs no edit at all.
Every number in a parsed document used to be a BigDecimal. That is
exact, which was the point — the document model before it parsed
numbers as Double and silently rounded anything above 2^53 — but it
is also two objects and about sixty bytes, spent on a value that is
nearly always a row id, a count, or a timestamp offset. A Forgejo
response is mostly numbers, so the whole decode path paid it.
WHAT CHANGES
JsonValue has two number cases now instead of one:
JsonValue.Int64(value: Long) a number written without a
fractional part or an
exponent whose digits fit in
64 bits
JsonValue.Decimal(value: BigDecimal) everything else: fractional,
exponent form, or a whole
number past Long.MaxValue
Which case a parsed number lands in follows the text that was on the
wire and nothing else, so Json.render still writes back exactly what
it read — 102 stays 102 and 102.0 stays 102.0. Forgejo's integer
fields reject 102.0, so that was never negotiable; the existing
round-trip tests pinning it are untouched and green.
The parse calls jsoniter's readNumber rather than readBigDecimal.
readNumber returns a java.lang.Long for a whole number that fits, a
java.math.BigInteger for one that does not, and a
java.math.BigDecimal for everything else — checked against the 2.39.1
jar rather than recalled — which is exactly the distinction the two
cases want to draw.
PRECISION IS UNCHANGED
Nothing here goes near a Double. A Long holds every integer up to
2^63 exactly and anything larger stays a BigDecimal, so
9007199254740993 (2^53 + 1, the first integer a Double cannot
represent) and 9223372036854775808 (one past Long.MaxValue) both
round-trip byte for byte. There is a test for each.
MEASURED, NOT ASSUMED
scripts/alloc-bench.sh, full table, before and after, on OpenJDK
64-Bit Server VM 25.0.4+7-LTS. B/op is heap bytes allocated per
operation.
operation before after change
------------------------------------------------------
parse.page-50 904,128 889,328 -1.6%
decode.page-50 1,159,616 1,134,416 -2.2%
decode.repo-wide 23,200 22,680 -2.2%
assemble.repo-wide 5,096 4,888 -4.1%
decode.org-narrow 3,160 3,096 -2.0%
assemble.org-narrow 608 592 -2.6%
parse.ints-1000 88,528 56,952 -35.7%
parse.bools-1000 26,576 26,560 noise
baseline.noop 0.0 0.0 —
parse.bools-1000 is the control: this change cannot touch an array of
booleans, and it did not move beyond the 16 bytes the harness header
calls noise.
parse.ints-1000 parses [0,1,...,999], whose numbers are far smaller
than a real row id, so a row-id-shaped array was added to the harness
temporarily, measured on both sides of the change with the same
filter, and then removed — a thousand nine-digit integers, the shape
Codeberg's identifiers actually have:
parse.ids-1000 95,080 63,080 -33.7%
That is 32.0 bytes saved per number. It is also why the page rows
move so little: repo-single.json holds 13 numbers against 44 strings
and 35 booleans, so numbers were never most of a repository's weight.
They are most of a page of identifiers or a quota response.
WHAT IS LEFT ON THE TABLE
63.1 bytes an element is still not the 26.6 the same-length array of
booleans costs. About 24 of the difference is a java.lang.Long that
jsoniter boxes in order to return it from readNumber, and that dies
one line later. Removing it needs a look-ahead the reader interface
does not offer; the only route to it is to try readLong behind a mark
and catch the failure, which hands a remote party a document that
throws an exception once per fractional number. Twenty-four bytes is
the cheaper of the two, so the box stays, documented where it
happens.
SOURCE COMPATIBILITY
JsonValue.Num survives as an object with the same four constructors
and a new unapply, so JsonValue.Num(7) still builds a number and
`case JsonValue.Num(value)` still binds a BigDecimal. What it can no
longer be is a type.
The readers on the hot path were moved off it rather than left on it,
because the extractor has to build a BigDecimal for the Int64 case
and that is the cost this commit exists to remove. JsonDecoder.long
and JsonFields.number name the two cases instead, through the new
JsonValue.longOpt; RepositoryContentDto asked numOpt.isDefined and
now asks the new JsonValue.isNum, which allocates nothing; and
EditorConfigDto's renderer matches the cases directly.
TWO CORRECTNESS FIXES ALONG THE WAY
BranchProtectionOptionDto and MigrateRepoOptionsDto built their
numbers as JsonValue.Num(x.toDouble), which rounds above 2^53 and so
could have corrupted a team id — an int64 on the wire. They pass the
value itself now. EditorConfigDto rendered a whole number through
BigDecimal.toLong, which wraps rather than fails for a value too
large for a Long, and uses toBigInt now.
VERIFICATION
./verify.sh --with-slow passes. Duplication is unchanged at 373
groups, so the recorded baseline is left where it is.
BREAKING CHANGE: JsonValue.Num is no longer a case class. A number in
a parsed document is now JsonValue.Int64(Long) or
JsonValue.Decimal(BigDecimal), and JsonValue.Num is an object holding
the constructors and an extractor for both.
Constructing is unchanged: JsonValue.Num(7), Num(7L), Num(7.0) and
Num(BigDecimal(7)) all still compile. They return JsonValue rather
than JsonValue.Num, which no longer names a type, and Num is the
constructor to keep using — it is what decides which of the two cases
a value belongs in, and building a JsonValue.Decimal directly from a
whole number that would fit a Long produces a document that does not
equal the one parsing its own rendering gives back.
Matching with `case JsonValue.Num(value)` is unchanged and still
binds a BigDecimal, but it now builds one for a whole number. Prefer
`case JsonValue.Int64(value)` where a Long was what was wanted, or
the new JsonValue.longOpt.
A match that enumerates every JsonValue case must replace its
`case JsonValue.Num(_)` with the two new cases. The compiler says so:
Num is no longer one of the cases, so such a match stops being
exhaustive.
Every response used to be copied twice on its way to the parser, and
neither copy did anything.
The transport read the body with sttp's `asStringAlways`, so the bytes
that arrived on the socket were decoded into a `String` — copy one.
`Json.parse` then called jsoniter's `readFromString`, which encodes that
`String` straight back into a `byte[]` in order to read it, because
jsoniter (like every JSON parser worth using) reads bytes — copy two. On
a 170 KB listing page that is a third of a megabyte of garbage produced
before a single field is looked at, on a payload that was already in
exactly the right shape when it arrived.
So the body is now bytes from the transport all the way to the parser,
and is decoded to text only where text is genuinely wanted.
WHAT CHANGED
`ResponseBody` is a new type in `modules/core`. It holds the bytes and
the charset the response declared for them, and it is the one place that
knows how to turn one into the other:
* `bytes` — the array, not copied; copying it would reintroduce the
copy the type exists to remove, so the contract is that whoever
builds a body gives up the array and whoever reads it must not write
to it. `BinaryResponse` has always worked this way.
* `text` — the whole body decoded, once, and kept.
* `utf8Bytes` — the bytes for a reader that requires UTF-8 rather than
merely expecting it. See the charset note below.
* `isBlank` — answered on the bytes, so asking it of a 40 MB payload
does not decode 40 MB of text.
* `excerpt(n)` — a bounded excerpt, in characters; see below.
`CodebergResponse.body` is a `ResponseBody` rather than a `String`, and
`Decode[A]` takes one. `Decode` is core's port and `modules/core` still
imports nothing but the standard library — a byte array and a
`java.nio.charset.Charset` are as library-agnostic as a `String` was.
The architecture-boundary check in `verify.sh` confirms it.
`SttpHttpPort` reads every response with `asByteArrayAlways`. That also
made its textual and byte-carrying paths identical apart from which
response type they assemble, so the two `build` methods and the two
`dispatch` methods collapsed into one of each.
`Json` gained `decode`/`parse` overloads taking an `Array[Byte]`, which
read through jsoniter's `readFromArray`. The `String` overloads remain
for callers that genuinely start from text — the error-payload parser
and tests written against a literal.
CHARSET HANDLING
Decoding bytes yourself means choosing a charset, so the choice is
written down rather than assumed. `ResponseBody.charsetOf` reads the
`charset` parameter of `Content-Type` and falls back to UTF-8 when the
header is absent, carries no charset, or names one that is not a legal
or available charset name. That is exactly what sttp's `asStringAlways`
did on this library's behalf, so nothing about text decoding changed
when the body stopped being text.
Forgejo answers `charset=utf-8` everywhere, but that is an observation
about one server, not a licence to ignore the header. The one place
UTF-8 is genuinely *required* rather than expected is JSON — RFC 8259
§8.1 — and that requirement is stated in `utf8Bytes`, which hands back
the socket's own array when the response declared UTF-8 and transcodes
when it declared anything else. A non-conforming server is therefore
read correctly and merely slowly, instead of as mojibake.
THE SNIPPET, WHICH IS THE TRAP
`ApiPipeline.snippetOf` bounds the excerpt a decoding failure carries.
It bounded *characters*, and it still does. Slicing the byte array at
512 bytes and decoding the slice is not the same thing: it cuts a
multi-byte character in half and ends the excerpt in a replacement
character the server never sent, and for a payload of three-byte
characters it would return 170 characters where 512 were asked for.
`ResponseBody.excerpt` slices generously instead — enough bytes that the
requested number of characters is certainly inside, computed from the
widest encoding of one character in that charset — and then applies the
bound to the decoded text, where a character is a character. Whatever
the generous slice damaged at its own tail sits beyond the bound and is
discarded. `ResponseBodySuite` pins that with three-byte characters and
with four-byte ones, which are two characters each.
MEASURED, NOT RECALLED
`scripts/alloc-bench.sh` on this machine (OpenJDK 25.0.4+7-LTS), 11
measured rounds, decoding one 170,251-byte page of 50 repositories into
`Vector[RepositoryDto]`:
before, end to end 1,308,728 B/op 440,695 ns/op
after, end to end 968,160 B/op 360,753 ns/op
That is 340,568 bytes per page saved, 26%. The page is 170,251 bytes and
two copies of it are 340,502 bytes, so the saving is the two copies and
essentially nothing else — which is what a change that removes two
copies and touches nothing else should look like.
Allocation was identical to the byte across two full runs and every
round within them. The times are from the fastest round of each and are
worth reading as a direction, not a figure: the spread between rounds on
this machine reached 35%.
DELIBERATELY NOT DONE
`BinaryResponse` and `BinaryHttpPort` are now near-duplicates of
`CodebergResponse` and `HttpPort` — the reason they existed, that a
response body was text and a ZIP is not, has gone. Merging them changes
the published signature of `ActionDownloadApi` and of `CodebergClient`'s
constructor, so it belongs in its own commit rather than riding on this
one. Both types say so in their own documentation now.
`RepositoryGitApi.getRawFile`, `getMediaFile` and `downloadArchive`
still return `String`, and are still lossy for a binary file. That was a
limitation of core; it is now a choice of that group's signatures, and
changing them is likewise a separate change. Their Scaladoc, and the
Scaladoc on `IssueAttachmentApi`, `RepositoryActionApi`,
`RepositoryAdminApi`, `IssueAttachment` and `ActionArtifact`, has been
corrected — all of it justified a decision by a constraint that no
longer exists, and two paragraphs also claimed the ZIP endpoints were
unimplemented when `ActionDownloadApi` has implemented them for a while.
BREAKING CHANGE: `CodebergResponse.body` is a `ResponseBody` rather than
a `String`, and `Decode[A].apply` takes a `ResponseBody`.
Anyone implementing `Decode` by hand — `body => parse(body)` — takes a
`ResponseBody` now. Ask it for what you need: `body.bytes` for a parser
that reads bytes, `body.text` for one that reads text, `body.isBlank` to
recognise an empty payload.
Anyone constructing a `CodebergResponse`, which in practice means a test
fake standing in for the transport, wraps the body:
`CodebergResponse(200, headers, ResponseBody.utf8("[]"))`, or
`ResponseBody.Empty` for a `204`. `ResponseBody.of(bytes, charset)` is
the form a real transport uses.
Nothing about the endpoints changes. Every method on every API group
returns exactly what it returned before.
The previous commit stopped copying a response body twice on its way to
the parser. The harness could not show that, because every operation in
it started from a `String` — the shape a response no longer has.
Two rows are added, both decoding the same 170 KB page of 50
repositories into `Vector[RepositoryDto]`:
decode.page-50-bytes from the bytes the socket produced, which is
the path a response takes today
decode.page-50-viastring the path a response took before: decode the
socket's bytes into a `String`, which is what
sttp's `asStringAlways` did, then hand that
`String` to the parser, which encodes it back
into a `byte[]` to read it
The gap between the two is the whole of what the change removed, and
measuring both in one JVM run is the only way to get an honest number:
the harness's own header explains why a filtered run cannot be compared
against an unfiltered one, and the same applies to comparing a run of
today's code against a recorded number from a checkout of yesterday's.
`decode.page-50` stays exactly as it was, so numbers recorded before the
change are still comparable with numbers recorded after it.
On this machine (OpenJDK 25.0.4+7-LTS, 11 measured rounds):
decode.page-50-viastring 1,308,728 B/op 440,695 ns/op
decode.page-50-bytes 968,160 B/op 360,753 ns/op
340,568 bytes per page, which is two copies of a 170,251-byte payload
and essentially nothing else.
This is still not a gate and must not become one, for the reasons the
script's header gives at length.
A response model in this library is decoded from a Forgejo payload and never built by a caller: a client call hands one back, and that is the only way one comes into existence. Their constructors were public anyway, which quietly makes every field Forgejo adds our problem. Forgejo grows response fields routinely. Adding one to a public `final case class` changes the generated `<init>`, `apply`, `copy` and every `copy$default$N`, so it is a breaking change. Once the 0.1.0 tag becomes the binary-compatibility baseline, a breaking change costs a major version — which would mean owing one every time the server grows a field. Nothing has been published yet, so closing these constructors today is free; doing it after 0.1.0 would cost the very major version it exists to avoid. That deadline is why this lands now. `private[codeberg4s]` costs almost nothing inside the repository, because every source tree here — the library, its unit tests, the examples and the integration suite — lives under `com.worxbend.codeberg4s`. Each wire DTO's `toDomain` still builds these models and every existing test fixture still compiles. Reading fields and pattern matching stay open to everyone; only `apply` and `copy` are narrowed, which was verified against a probe source in an unrelated package. One honest caveat: a qualified-private constructor still erases to public in the bytecode, so a binary-compatibility checker will keep seeing the synthetic members change shape. What changes is that no code outside the library can have compiled against them, so such a report becomes a filter to write rather than a release to renumber. Left public on purpose: the command models a caller must be able to build in order to make a request — `CreateIssue`, `EditIssue`, `CreatePullRequest`, `MergePullRequest`, `SubmitReview`, every `*Query` — plus the opaque types, the enums and the error ADT. BREAKING CHANGE: `Comment`, `Issue`, `IssueAttachment`, `IssueDeadline`, `IssueSubscription`, `Label`, `Milestone`, `Reaction`, `TimelineEvent`, `TrackedTime`, `ChangedFile`, `PullRequest`, `PullRequestBranch`, `Review` and `ReviewComment` can no longer be constructed or copied from outside the library. Code that built one directly — most plausibly a test fixture standing in for a server response — must obtain it from a client call instead, or decode a recorded payload through the client. That is a real cost, and it is the deliberate trade for not owing a major version every time Forgejo adds a field to a response.
Continues closing response-model constructors, on the same reasoning as the issues and pulls commit: these models are decoded from a Forgejo payload and never built by a caller, so leaving their constructors public means every field the server grows is a breaking change to this library's API. With 0.1.0 about to become the binary-compatibility baseline, that is a bill payable in major versions; closing them before anything is published costs nothing. Which models these are was decided from the codec, not from their names: a class a wire DTO's `toDomain` constructs, and which appears nowhere as a parameter of a public client method, only ever travels from the server to the caller. Every class touched here meets both halves of that test. The quota models are the clearest case for the change. `QuotaInfo`, `QuotaGroup`, `QuotaRule`, `QuotaSizes`, `QuotaUsage` and the `QuotaUsed*` family are a deep tree of small records mirroring Forgejo's quota report, which is a young and moving part of its API. Each one was a field addition away from being a breaking change. Left public: `CreateOrganization`, `EditOrganization`, `CreateTeam`, `EditTeam`, `CreateAccessToken`, `CreateSshKey`, `CreateGpgKey`, `VerifyGpgKey`, `UpdateUserSettings`, `OAuth2ApplicationDefinition`, `CreateRepository`, `ActivityFeedQuery`, `TrackedTimeWindow` and the opaque identifiers — a caller has to be able to build all of those to make a request, and `RepoSlug`, which a caller supplies as part of `CreateAccessToken.repositories` as well as receiving inside a `Repository`. BREAKING CHANGE: `User`, `UserSettings`, `PublicKey`, `Email`, `AccessToken`, `CreatedAccessToken`, `GpgKey`, `GpgKeyEmail`, `HeatmapEntry`, `StopWatch`, `OAuth2Application`, `Organization`, `OrganizationPermissions`, `Team`, both `BlockedUser` models, both `QuotaInfo` models and the whole quota tree beneath them can no longer be constructed or copied from outside the library. A caller that built one directly — realistically a test fixture standing in for a server response — must obtain it from a client call instead. Reading fields and pattern matching are unaffected.
The largest group, and the one the whole exercise was aimed at. `Repository` alone carries 40 fields, every one of them decoded from a Forgejo payload and none of them supplied by a caller. Forgejo's own `Repository` definition has 66 keys and gains more with each release, so with a public constructor this library owed an API break every time upstream added one — and, after 0.1.0 becomes the binary-compatibility baseline, a major version with it. Nothing is published yet, so the change is free today and expensive tomorrow. The same argument applies to everything decoded alongside it, which is why this commit reaches into `access`, `actions`, `admin`, `gitdata`, `hooks` and `publishing`: `Release`, `ReleaseAsset`, `Commit` and its `CommitDetails`/`CommitSummary`/`CommitStats`/`CommitVerification` pieces, `Branch`, `Tag`, `Webhook`, `WikiPage`, `DeployKey`, `BranchProtection`, `TagProtection`, the `Action*` runner and workflow models, and the git-data models `GitBlob`, `GitTreeEntry`, `GitReference`, `GitObjectRef`, `AnnotatedTag`, `GitNote`, `CommitComparison`, `FileChange`, `FileCommit` and `EditorConfigDefinitions`. Two neighbouring pairs are deliberately split down the middle, and the split is the evidence that this was decided per class rather than per package. `GitIdentity` is closed and `GitAuthor` stays open: its own Scaladoc already says one is what a response carries and the other is what a request sets. `BranchProtection` is closed while `BranchProtectionSettings` stays open, because the latter is the builder a caller fills in for `CreateBranchProtection` and `EditBranchProtection`. Also left public: `CreateRepository`, `EditRepository` and its `*TrackerSettings`/`ExternalWikiSettings` parts, `MigrateRepository`, `TransferRepository`, `CreateFork`, `GenerateRepository`, `CreateRelease`, `EditRelease`, `UploadAsset`, `EditAsset`, `CreateTag`, `CreateBranch`, `RenameBranch`, `CreateFile`, `UpdateFile`, `DeleteFile`, `ChangeFiles` and their `CommitOptions`, `CommitIdentity` and `CommitDates` parts, `CreateHook`, `EditHook`, `CreateWikiPage`, `EditWikiPage`, `ApplyDiffPatch`, `DispatchWorkflow`, `CreateVariable`, `UpdateVariable`, `RegisterRunner`, `CreatePushMirror` and every `*Query`. A caller has to be able to build all of those. BREAKING CHANGE: `Repository` and 58 other repository-side response models can no longer be constructed or copied from outside the library; `git show --stat` on this commit lists them. Code that built one directly — most plausibly a test fixture standing in for a server response — must obtain it from a client call instead, or decode a recorded payload through the client. Reading fields and pattern matching are unaffected. The trade is deliberate: the alternative is a major version of this library for every field Forgejo adds to a response.
The last of the response models: notifications, the instance-wide `miscellaneous` models, and the two that sit in the root package. Same reasoning as the three commits before it — each of these is decoded from a Forgejo payload and never built by a caller, so a public constructor makes every field the server adds a break in this library's API, and after 0.1.0 becomes the binary-compatibility baseline that break costs a major version. `ServerApiSettings`, `ServerUiSettings`, `ServerAttachmentSettings` and `ServerRepositorySettings` are the strongest case in the whole change. They are a straight mirror of an instance's configuration, so they grow whenever a Forgejo release adds a setting — which is more often than any other model here changes. Four of these are reached through a companion rather than a DTO, and they are included on the same grounds. `SigningKey.from` and `SshSigningKey.from` read a plain-text response body, `RenderedMarkdown.apply` is handed to the markdown decoder as a function value, and `ServerVersion.apply` is used the same way by `ServerVersionDto`. All four still work, because those companions live inside `com.worxbend.codeberg4s`. `ApiErrorBody` is the payload inside `CodebergError.Api`. The error ADT itself stays exactly as it is — closing a case of it would break every exhaustive match — but the body it carries is a decoded response like any other, and matching on it still works. Left public here: `MarkdownRenderRequest`, `MarkupRenderRequest` and `NotificationQuery`, which a caller builds to make a request, plus `Page` and `CallContext`. Those last two are worth naming because they are not Forgejo models at all: `Page` is this library's own pagination container, and `CallContext` is the call description attached to a failure. Neither grows a field because upstream did, and a caller may reasonably build either one when testing its own code, so the argument for closing them does not apply. BREAKING CHANGE: `NotificationThread`, `NotificationSubject`, `NodeInfo` and its `NodeInfoSoftware`/`NodeInfoServices`/ `NodeInfoUsage`/`NodeInfoUsers` parts, `ServerApiSettings`, `ServerUiSettings`, `ServerAttachmentSettings`, `ServerRepositorySettings`, `GitignoreTemplate`, `LicenseTemplate`, `LicenseTemplateSummary`, `TemplateLabel`, `RenderedMarkdown`, `SigningKey`, `SshSigningKey`, `ServerVersion` and `ApiErrorBody` can no longer be constructed or copied from outside the library. A caller that built one directly — realistically a test fixture standing in for a server response — must obtain it from a client call instead. Reading fields and pattern matching are unaffected.
The compatibility rules said, of every model in the library, that adding a field to a public `final case class` is breaking and never a patch. That is no longer true of response models: their constructors are now `private[codeberg4s]`, so no caller outside the library can be calling the generated `apply` or `copy`, and a new field cannot break anyone's source compatibility. A rule that overstates the cost of a release is as unhelpful as one that understates it — the whole point of closing those constructors was to make tracking Forgejo cheap, and the document that decides what a release costs has to say so. What changed here: - The "minor release may" list gains adding a field to a response model, with the definition of what a response model is and examples. - The "these are breaking" bullet is narrowed to command models, and now says why they are different: a caller has to build one to make a request, so their shape really is part of the API. - A new section states the two limits of the exemption honestly. It covers growth only — removing, renaming or retyping a field is still breaking either way. And a qualified-private constructor is still public in the bytecode, so MIMA will keep reporting those synthetic members once it is wired; the note says those reports are filters to write rather than releases to renumber, and that the reasoning belongs in the commit that adds the filter, so nobody later reads a green build into a rule that was never checked.
`UploadAsset` and `UploadAttachment` describe the file half of a
`multipart/form-data` upload — a release asset, an issue attachment.
Both carried a scaladoc promise the code did not keep, and both had a
builder that wrote a caller's string into an HTTP header without
looking at it.
The promise first. `UploadAsset.of` says its file-name check lives "in
a smart constructor a caller cannot bypass", and `UploadAttachment`
says the same. Neither was true: both types were public `final case
class`es, so Scala generated a public `apply` and a public `copy`, and
either one built an upload with any file name at all — a quotation
mark to close the `filename="…"` parameter, a CRLF to end the
`Content-Disposition` line and open a header of the caller's choosing.
Both constructors are now `private`, which makes the generated `apply`
and `copy` private with them, so `of` really is the only door. Reading
the fields and pattern matching are unchanged.
The second defect is `as`, the builder that states the part's own
`Content-Type`. It performed no validation at all, and its argument
reaches the wire as a header value. An HTTP header value ends at the
first carriage return or newline, so `as("text/plain\r\nX-Bad: 1")`
wrote a header this API never offered, and inside a multipart body it
can open a part nobody asked for. `as` now returns
`Either[ValidationError, …]` and refuses a blank media type or one
carrying any control character, reporting on the `"mediaType"` field.
Both types reach that check through a new `ContentType` in the root
package, placed next to `PathSegment` and for the same reason: a
validation rule only one package can reach gets copied into the
packages that cannot, and copies drift. `ContentType.isSafe` is the
rule itself; `ContentType.from` wraps it in a named `ValidationError`.
`SttpHttpPort.withBody` applies `isSafe` a second time to the media
type of a `RequestBody.Multipart` and answers a `TransportFailure` —
cause `Unknown(SttpHttpPort.UnsafeMultipartMediaType)` — instead of
throwing or sending. The domain type is the primary guard; this is the
last place the value is still ours. `Multipart` is an ordinary enum
case that any code in this library can build from a bare `String`
without going near an upload command, and `withBody` is the final point
at which that string is a Scala value rather than wire bytes.
Threading that refusal outward made `build` fallible, so `send` and
`sendBinary` now share one `dispatch`, which resolves the base URI and
the body up front and short-circuits to an already-completed `Future`
when either is refused. The backend never sees a request it should not
send, which the new transport tests assert directly.
Why now: nothing is published to Maven Central yet, and the 0.1.0 tag
will become the MIMA binary-compatibility baseline. Closing a
constructor and widening a return type are free today and cost a major
version the day after that tag exists.
BREAKING CHANGE: `UploadAsset` and `UploadAttachment` can no longer be
constructed or copied from outside the library. Replace
`UploadAsset(fileName, bytes, mediaType, name)` with
`UploadAsset.of(fileName, bytes)` followed by `.named(…)` and `.as(…)`,
and likewise for `UploadAttachment`. Reading the fields and pattern
matching still work.
`as` now answers `Either[ValidationError, UploadAsset]` and
`Either[ValidationError, UploadAttachment]` rather than the upload
itself, so a chained call goes through `map` or a `for` comprehension:
for
upload <- UploadAsset.of("notes.txt", bytes)
typed <- upload.as("text/plain")
yield typed.named("release-notes")
A caller that never calls `as` is unaffected: the default media type is
still `application/octet-stream`.
This library reads a whole HTTP response into memory as an array of bytes; it never streams. Until now nothing said how many bytes that was allowed to be. The request builder in SttpHttpPort never called sttp's .maxResponseBodyLength, so a compromised, misconfigured or merely misbehaving instance could answer a request with a body limited by nothing but the client's heap: the only thing standing in the way was the read timeout multiplied by the peer's bandwidth, which is not a limit. CodebergConfig grows two settings, both counted in bytes. maxResponseBodyBytes, 16 MiB, applies to every textual response. The number is derived rather than picked. The largest legitimate JSON body Forgejo produces is a file's contents, which carries a repository blob capped by the instance's default_max_blob_size (10 MiB on codeberg.org, captured in docs/HAZARDS.md section 5) and base64-encoded at four bytes per three, so roughly 13.4 MiB on the wire. That number is per-instance configuration, so the scaladoc points at ServerApiSettings for reading back what your own Forgejo reports. maxDownloadBodyBytes, 50 MiB, applies only to the two ZIP-fetching operations under client.repos.actions.downloads. A CI artifact is whatever a workflow uploaded, so default_max_blob_size says nothing about it, and a single shared number would have had to be either too small for ordinary artifacts or too large to bound JSON usefully. 50 MiB is where this project had already drawn the line in prose: SECURITY.md and docs/ROADMAP.md both record "attachment streaming above 50 MB" as out of scope for v1. The default now enforces what those documents describe instead of leaving it to the heap, and SECURITY.md no longer offers "a large artifact exhausting the heap is documented behaviour" as an answer to a report. Exceeding either bound is reported as a new TransportCause case, ResponseTooLarge. Giving it a case of its own is about behaviour, not about vocabulary: RetryEngine.isRetryable treats TransportCause.Unknown as retryable, so folding this failure into Unknown would have made the default policy of three attempts download the oversized body three times over. A bound meant to cap one call at N bytes would have let through 3N. The new case is non-retryable, and the test added to RetryEngineSuite asserts the attempt count rather than asserting the classification, so an edit that made it retryable again fails there. The transport also has to recognise the failure. sttp throws sttp.capabilities.StreamMaxLengthExceededException and hands it on wrapped in an SttpClientException.ReadException, so classification stays a walk down the cause chain; matching only the outermost type would have reported every oversized body as Unknown and quietly made it retryable again. BackendStub does not apply the limit itself, so the new transport tests assert the two things a stub can prove: that each request carries the configured bound in its sttp options, and that the exception a real backend raises is classified as ResponseTooLarge both bare and wrapped. BREAKING CHANGE: CodebergConfig gains two parameters, maxResponseBodyBytes and maxDownloadBodyBytes, and TransportCause gains a case, ResponseTooLarge. Code that calls the full CodebergConfig constructor must pass the two new arguments; the constants CodebergConfig.DefaultMaxResponseBodyBytes and CodebergConfig.DefaultMaxDownloadBodyBytes reproduce the values the one-argument CodebergConfig(auth) uses. Any exhaustive match on TransportCause must handle the new case. Nothing is published to Maven Central yet and the 0.1.0 tag becomes the MIMA binary-compatibility baseline, so making this change now costs nothing, whereas making it after 0.1.0 would cost a major version.
…rt result
`PageWalk.fold` visits at most `MaxPages` (10 000) pages so that an
instance which offers a next page forever cannot hang the caller's
process. Until now, reaching that cap returned the state folded so far —
exactly the same shape a walk that reached the real end of the listing
returns. A caller walking a repository with more pages than the cap got a
short answer with no way to tell it was short.
That is the failure mode the whole pagination design exists to prevent.
README's pagination section already warns at length that an incomplete
result which looks complete is the worst kind of bug in a client library,
and then the library shipped one.
The walk now fails instead. Reaching the cap while the server is still
offering another page fails the `Future` with `CodebergException`
wrapping a new error case:
case WalkTruncated(pagesVisited: Int, resumeFrom: PageParams)
`resumeFrom` is the window the walk was about to request, page size
included, so a caller who genuinely wants more than half a million items
continues with `PageWalk.all(resumeFrom)` rather than starting over. The
case carries no `CallContext` because nothing went wrong on the wire —
every one of those pages arrived; what failed is the walk's promise to
cover the whole collection.
The cap is now tested against a page the server actually offered, so a
listing whose last page happens to be the ten-thousandth and offers
nothing further has reached its natural end and still succeeds. The old
code could not tell those two situations apart at all.
Adding a case to a sealed family is a source-breaking change for anyone
matching exhaustively. Nothing is published to Maven Central yet and the
0.1.0 tag will become the MIMA binary-compatibility baseline, so making
it now is free and making it later costs a major version. That deadline
is the reason this is a new case rather than, say, an `Option` returned
alongside the result.
Everything that matches the family exhaustively was updated:
`CodebergError.describe`, `RetryEngine.isRetryable` and
`RetryEngine.contextOf` (a truncated walk never reaches the retry engine,
and repeating the walk would stop at the same cap, so it is not
retryable), the test telemetry recorder, and the `HandlingErrors`
example — whose compiler error on the missing case is the proof that the
break is visible to a reader rather than silent. The redaction property
in `SecretProps` lists the new case too, so its claim to cover every case
of the ADT stays true.
BREAKING CHANGE: `CodebergError` gains a sixth case,
`WalkTruncated(pagesVisited, resumeFrom)`. An exhaustive `match` on
`CodebergError` no longer compiles until the case is handled; add a
clause for it, or add a `case _` if the distinction does not matter to
you. Separately, `PageWalk.all`, `PageWalk.fold` and `PageWalk.foreach`
now fail the returned `Future` when they hit the page cap, where they
previously returned the pages gathered so far. Code that relied on the
short result should either handle `WalkTruncated` and resume from
`resumeFrom`, or narrow the query so the walk fits inside the cap.
`core.Pagination` was a public class with two methods, `foldPages` and `listAll`, that walked a paginated collection one page at a time. It has zero call sites: a grep across every `modules/*/src` tree finds the class named nowhere but its own definition, and the only files that mention it at all are its two test suites. What actually walks pages in this library is `com.worxbend.codeberg4s.paging.PageWalk`, which is what the README and the roadmap point readers at. `PageWalk` supersedes `Pagination` on every axis that mattered: it takes the listing operation as an argument so one implementation of the termination rule serves every endpoint, it works on the `Future` API a caller actually holds rather than on the internal `Exec[F]` abstraction, and it bounds the walk. Keeping a second walker alongside it means two places where the termination rule can drift apart and a reader having to work out which one is current. Deleting rather than deprecating is the right move because nothing is published to Maven Central yet. A deprecation exists to give existing users a migration window, and there are no existing users — the first released artifact will simply never have contained this class. Waiting would invert that: the 0.1.0 tag becomes the MIMA binary-compatibility baseline, after which removing a public class costs a major version for the benefit of nobody. The two test suites go with it. They tested only this class, so keeping them would mean keeping the class to compile them against. `PageWalk` has its own suite covering the same termination behaviour. Documentation that named the old class is corrected in the same commit: CHANGELOG's pagination bullet and its known-limitations entry now name `paging.PageWalk` and its `all` / `fold` / `foreach`, and the roadmap no longer lists `Pagination` among what `core` provides. The roadmap's property-suite entry claimed the pagination driver, codec laws and retry bounds all lacked property suites; the pagination half of that stops being true in the way it described the moment `PaginationProps` is deleted, so the whole sentence is restated against what the tree actually contains — `JsonProps`, `ApiErrorBodyCodecProps` and `RetryEngineProps` cover two of the three areas, and the walk is the one left open. BREAKING CHANGE: `com.worxbend.codeberg4s.core.Pagination` is removed, along with its `foldPages` and `listAll` methods. Replace `new Pagination[F].listAll(start)(fetch)` with `PageWalk.all(start)(fetch)` and `foldPages(start, zero)(fetch)(step)` with `PageWalk.fold(start, zero)(fetch)(step)`, both from `com.worxbend.codeberg4s.paging.PageWalk`. The replacements return `Future` directly rather than an abstract `F`, and they fail with `CodebergError.WalkTruncated` on hitting the page cap instead of walking until the fetch stops offering pages.
BinaryResponse, RequestBody.Binary and RequestBody.Multipart are case
classes holding an Array[Byte]. In Scala an array's own equals is
identity, so the equality a case class generates compared those bodies
by reference: two responses carrying byte-identical archives answered
false, their hash codes differed, and a Set kept both copies. Nothing
warns about that, so the wrong answer turns up in an assertion or in a
lookup rather than at compile time.
All three now write equals and hashCode over java.util.Arrays.equals
and java.util.Arrays.hashCode, keeping every other field in the
comparison and testing those first because they are the cheap half —
the archive may be megabytes. Each class is final, so the type test in
equals is exactly what the compiler generates for canEqual and the two
cannot disagree; the Scaladoc records that, and records that removing
final would change it.
RequestBody stopped being an enum to make room for this. A Scala 3 enum
case cannot have a body, so a case that has to write its own equals
cannot stay an enum case. The cases are now final case classes under a
sealed trait, which construction, pattern matching and exhaustivity
checking all see exactly as they saw the enum's cases; `ordinal` and
the scala.reflect.Enum supertype are what is lost, and nothing in the
library or its examples used either.
Nothing is published to Maven Central yet, and the 0.1.0 tag becomes
the MIMA binary-compatibility baseline, so reshaping RequestBody is
free today and would cost a major version after that tag. That deadline
is why the change happens now rather than being deferred.
The new core suite ByteEqualitySuite states the rule for all three
types: equal-but-distinct arrays are equal and hash alike, differing
bytes are not equal, and every non-byte field still counts.
BREAKING CHANGE: RequestBody is a sealed trait whose cases are case
classes rather than a Scala 3 enum. Building a body
(RequestBody.Json("…"), RequestBody.Empty) and pattern matching on one
compile unchanged; code that called `ordinal` on a RequestBody or
treated it as a scala.reflect.Enum has to match on the case instead.
Equality also changes meaning: BinaryResponse, RequestBody.Binary and
RequestBody.Multipart now compare their bytes, so anything that relied
on two byte-identical values staying distinct — using one as a map key
and expecting reference identity — has to say `eq` explicitly.
CodebergClient promises that "a Telemetry failure never fails the call it was observing". It did not keep that promise. Telemetry is the only way to see what this library is doing: it has no logging dependency, so an application implements the trait and the client calls it three times per attempt. Those callbacks are the application's own code, running on the library's request path, and they can break in two ways — throw where they stand, or hand back a Future that fails a moment later. Either one used to reach the caller of, say, repos.get as the outcome of a request the server had already answered with a 200. The path was two correct pieces meeting badly. ApiPipeline.observe runs every callback through Exec.attempt and discards the result, and for F = Either that hides the failure completely. FutureExec.attempt, though, materialises only CodebergException and deliberately leaves every other throwable as a failed Future, so that a caller's NullPointerException is never laundered into a CodebergError nobody can act on. Widening attempt would have destroyed that property, which is load-bearing everywhere else, so the fix is not there. It is at the boundary instead. A caller-supplied sink is now wrapped in GuardedTelemetry before it ever reaches the pipeline. The wrapper calls the sink inside a try, so a synchronous throw is caught, and maps the returned Future's outcome to success, so a later failure is caught too. The observation is lost in both cases; that is the intended trade. Fatal throwables still get through on both rails: an OutOfMemoryError, a LinkageError, a ControlThrowable or an InterruptedException says the process is unsound or that someone asked for cancellation, and hiding one to protect a single API call trades a visible crash for a silent corruption, or a cancellation for a hang. Recognising them on the async rail needs one extra step, because completing a promise with such a throwable replaces it with an ordinary ExecutionException wrapping the original — asking scala.util.control.NonFatal alone would hide exactly the failures that must never be hidden. Telemetry.noOp is not wrapped. It is this library's own code, it cannot fail, and it is on the path of every request an unconfigured client makes, so it does not pay for a guard it does not need. TelemetryFailureSuite is the proof. Its four end-to-end tests fail against the previous commit — both failure modes, on both the exception rail and the .attempt rail — and pass now; two further tests pin down that a fatal error and an interrupt are still propagated. ApiPipeline's class Scaladoc overstated what it could promise about a failing callback and now says which failures it swallows and which are the caller's business to wrap.
When a 2xx response body does not match the model, ApiPipeline builds a
CodebergError.DecodingFailed carrying an excerpt of that body. For almost
every endpoint that is right: an excerpt of what arrived is the only thing
that makes a decoding failure diagnosable, and the excerpt is bounded so a
large listing cannot flood a log.
Four responses in this API are the exception, because their success body is
itself a live secret:
- POST /users/{username}/tokens answers a usable personal access token;
- POST /user/applications/oauth2 answers a client secret, and a PATCH of
the same application may re-issue one — the spec does not say, so this
treats it as though it does;
- POST .../actions/runners answers the runner's registration token, and
the registration-token endpoint answers the same credential on its own.
Both exist three times over, for repositories, organisations and the
authenticated account.
On those, a payload that carried the credential and failed to decode for
some unrelated reason put the credential into an error value the
application is about to log. The domain types mask themselves in toString,
but a snippet is the raw body taken before any conversion, so no mask
reaches it.
The Decode port now answers `sensitive`, a concrete `def` defaulting to
false so every existing instance keeps compiling, and `Decode.sensitive`
wraps an instance to set it. ApiPipeline substitutes a fixed placeholder —
`*** (N bytes withheld)` — for the excerpt when it is set. The placeholder
still reports that a body arrived and how large it was, so a truncated
response is still distinguishable from a complete one that did not match.
Deciding this inside the pipeline is what closes the hole. UserTokenApi
already rewrote the failure it received, but Telemetry.onError fires while
the attempt is being settled, before any endpoint sees the result: a
deployment logging raw telemetry errors recorded the token regardless.
That bespoke rewriting is gone, and with it the public
UserTokenApi.RedactedBody.
The OAuth2 decoder is split in two rather than marked wholesale. A read of
an application cannot carry a secret — Forgejo stores it hashed — so
UserAccountDecoders.application keeps its excerpt and only the creation and
the update read through the sensitive `issuedApplication`. Withholding a
body that has nothing to withhold costs diagnosability for nothing.
Tests: ApiPipelineSuite asserts the substitution, that an ordinary decoder
is untouched, and that a telemetry sink observes the redacted failure;
UserTokenApiSuite, UserApplicationApiSuite and RepositoryActionApiSuite
assert that the rendered failure of a malformed credential-bearing response
contains no part of the material. The UserApplicationApiSuite test that
previously pinned the leak as intended behaviour is inverted, and the
Scaladoc on ClientSecret, CreatedAccessToken and RunnerRegistrationToken
that documented the leak is corrected.
Measured duplication fell from 373 groups to 363 (PMD 7.26.0 at 40 tokens),
so CPD_BASELINE_GROUPS in verify.sh is lowered to 363 in this commit rather
than left as headroom.
BREAKING CHANGE: `UserTokenApi.RedactedBody` is removed. A caller comparing
a snippet against it should compare against
`ApiPipeline.redactedSnippet(bytes)` instead, or simply stop special-casing
the token endpoint — every credential-bearing response is now redacted the
same way. Nothing is published to Maven Central yet and the 0.1.0 tag
becomes the MIMA baseline, so removing it now is free and removing it later
would cost a major version.
`CodebergClient.close()` promised to release the HTTP backend when the
client had created it. It called `backend.close()`, and that call did
nothing: the JDK `java.net.http.HttpClient` underneath survived, taking
its connection pool and its selector thread with it. An application that
built a client per instance and closed each one at shutdown was leaking a
pool per client.
Why it did nothing, read off the sttp 4.0.26 sources. When you ask for a
backend with `HttpClientFutureBackend(options)`, sttp decides whether it
is allowed to release the client it just built by testing whether the
`ExecutionContext` you handed it is also a `java.util.concurrent.Executor`:
val executor = Some(ec).collect { case executor: Executor => executor }
HttpClientFutureBackend(
HttpClientBackend.defaultClient(options, executor),
closeClient = executor.isEmpty, ...)
The reasoning is that an executor you supplied is yours to shut down, so
sttp must not. But every ordinary `ExecutionContext` — the global one, one
from `ExecutionContext.fromExecutor`, the one a test framework hands you —
is an `ExecutionContextExecutor`, so `executor.isEmpty` is always false,
`closeClient` is always false, and `HttpClientBackend.close()` takes its
`if (closeClient)` branch straight to `monad.unit(())`. There is no
combination of arguments that makes the old call release anything.
The fix builds the `java.net.http.HttpClient` here and hands it to
`HttpClientFutureBackend.usingClient`, so the decision to end it belongs
to the code that created it. `SttpHttpPort.defaultBackend` now returns a
small wrapper backend whose `close()` calls the client's `shutdown()`
before delegating.
`shutdown()` and not `close()`. Both exist on `HttpClient` (it has been
`AutoCloseable` since Java 21, and this project targets Java 25), but the
JDK's `close()` "waits until all operations have completed execution",
while `shutdown()` "does not wait for previously submitted request to
complete execution". `CodebergClient.close()` is documented as returning
promptly so that an ordinary `finally` block stays cheap, so it must not
be the blocking one. Requests already in flight still run to completion;
new ones are refused.
The client is configured exactly as sttp configured its own, so nothing
else changes: the same connect timeout, no redirect following (sttp's
`FollowRedirectsBackend` wrapper applies the policy itself, and a client
that also followed redirects would apply it twice), the system proxy that
`BackendOptions.Default` reads out of the `http.proxyHost` family of
properties, and the caller's `ExecutionContext` as the client's executor
when it is one. The executor is never shut down — it belongs to the
caller, which was sttp's original concern and remains correct.
Tests use a real JDK client rather than the counting stub, because a stub
cannot tell a `close()` that released a pool from one that returned an
already-completed `Future`. `DefaultBackendSuite` asserts that the client
reports `isTerminated` after the backend is closed and that the caller's
execution context is still running; `CodebergClientSuite` asserts the same
termination through the public `close()`. Both fail against the previous
behaviour, waiting out the full limit and reporting "close() left the JDK
HTTP client running". No test sends a request, so none opens a socket.
Scaladoc corrected where it described the old behaviour:
`CodebergClient.close`, `CodebergClient.apply`, and the ownership notes on
`SttpHttpPort` and `SttpHttpPort.defaultBackend`.
BREAKING CHANGE: a backend from `SttpHttpPort.defaultBackend` now really
does end when it is closed, and code that closed one and kept sending on
it used to keep working by accident. Such code must now close the backend
only once it is finished with it, or build a second one. Nothing is
published to Maven Central yet and the 0.1.0 tag will become the MIMA
baseline, so making this change now costs nothing; making it after 0.1.0
would cost a major version.
The shared-backend example exists to answer "who closes what", and its own answer leaked. It built the pool with sttp's `HttpClientFutureBackend(BackendOptions.Default.connectionTimeout(...))`, and the `backend.close()` in its `finally` block released nothing: that constructor declines to end the JDK HTTP client it created whenever the `ExecutionContext` it was handed is also an `Executor`, which every ordinary execution context is. An example whose closing comment reads "then the thing this program owns" should not be the one that demonstrates a leak. It now calls `SttpHttpPort.defaultBackend(ConnectTimeout, executionContext)`, which is this library's own factory and, since the previous commit, hands back a backend that really does shut its client down. The example teaches the same lesson — clients first, then the backend they were built on — and the teardown at the end now does what the comment beside it claims. The connect timeout still travels with ownership, so the class comment's paragraph about that now points at the `defaultBackend` call rather than at the `BackendOptions` value that is gone. The comment on `close()` also loses its claim that sttp's shutdown is asynchronous, which was a description of the Future being returned rather than of anything the program waits for.
Closing a CodebergClient while a call sat in retry backoff left the Future for that call with no outcome at all: not fulfilled, not failed, nothing. An application shutting down cleanly would wait on that Future for as long as the process lived. Two scaladocs disagreed about this, and the one describing the hang was the accurate one. FutureTimer.close now completes those promises, failing each with a java.util.concurrent.CancellationException. That matches how sleep already reports a scheduler that has been shut down: closing a client while it is still in use is a defect in the calling program, not a remote failure, so it must not be laundered into a CodebergError that the caller would then retry. Reaching the waiting promises took a change of scheduler. ScheduledThreadPoolExecutor does not put the Runnable it was handed onto its queue; it wraps that Runnable in an internal task object, and the wrapper is what shutdownNow() returns. Walking the returned list therefore found nothing recognisable. The timer now runs on a small subclass that overrides decorateTask, which is the supported hook for choosing what goes onto the queue, so every queue entry names the Completing runnable it would have run and close() can fail that runnable's promise. The completion stopped being an anonymous lambda for the same reason: decorateTask has to be able to recognise it. Both scaladocs are reconciled with the new behaviour. A call started after close fails with RejectedExecutionException, a call already waiting fails with CancellationException, and no Future this client handed out is left without an outcome. The test awaits with a bound rather than mapping over the Future, because a continuation on a Future that never completes never runs — a regression would have hung the suite instead of failing it. Making that bound real needed one more fix: an existing test deliberately continues on the scheduler's own thread, close() interrupts that thread, and munit may start the next test on it. An interrupted thread cannot wait, so Await threw at once instead of honouring its timeout; beforeEach now clears the leaked interrupt. This changes the documented contract of a published method. Nothing is on Maven Central yet and the 0.1.0 tag will become the MIMA binary-compatibility baseline, so the change is free today and would cost a major version once that tag exists.
SttpHttpPort built each request by applying the credential from
CodebergConfig first and the request's own headers second. sttp's
`header` defaults to DuplicateHeaderBehavior.Replace, so a name written
twice keeps the value written last, which meant the caller's headers
overwrote the credential — the exact opposite of what the scaladoc on
`withAuth` claimed ("applied after the caller's own headers so
configuration always wins").
No endpoint in this library sets an Authorization header today, so this
was not a live vulnerability. It was a latent one: every per-request
header map is an ordinary List[(String, String)] built somewhere in this
repo, and a new endpoint is one line away from putting a credential
there and silently authenticating as something other than what Auth was
configured with.
Two changes make that outcome unreachable rather than unlikely. The
credential now goes on last, so nothing can replace it. Authorization
and Proxy-Authorization are dropped from the caller's headers before
those are applied, matched case-insensitively because header names are
case-insensitive on the wire — so a request can no longer carry two
credentials under two spellings of one name.
User-Agent joins them in the dropped set. That is what keeps the
existing "the configured user agent wins over one a caller supplied"
contract true once the user agent stops being applied last, and the
observable result is unchanged: one User-Agent header carrying the
configured value.
Every other header still passes through, and still lands after the
body's own Content-Type. POST /markdown/raw depends on that ordering —
it sends markdown as text/plain over a body core models as JSON — and
MiscellaneousApiSuite's case "renderMarkdownRaw sends the markdown
itself as a plain-text body" stays green.
Separately, the user agent and the read timeout move into a `template`
val. An sttp request is an immutable value, so every builder call
allocates a new one, and both of these are read off a config that
cannot change once the port exists. They were rebuilding the same two
values on each call; they are built once per port now.
The three new SttpHttpPortSuite cases fail against the previous code —
the credential one put "token someone-elses-token" on the wire in place
of the configured token — so the fix is pinned by tests that could not
have passed before it.
Both upload commands gained a hand-written equals so that two uploads carrying byte-identical content compare equal — an array compares by identity, so the generated equals answered false for values that are the same in every way that matters. A hand-written equals owns one case the generated one handled for free: being asked to compare against a value of an unrelated type. It must answer false rather than assume the cast succeeds. That branch shipped with no test, and it was the only uncovered statement left in the domain module, which is otherwise at 100%. Getting this branch wrong does not fail loudly at the point of the mistake. It throws a ClassCastException later, out of whatever collection lookup happened to compare the two values, which is a long way from the code that caused it.
Until now nothing in this repository chose a JDK baseline. `javap -v` on a
compiled class reported major version 61 — Java 17 — not because anyone
decided that, but because Java 17 is Scala 3.8.4's built-in default when
no target is given. The number moved whenever the compiler's default
moved, and no file recorded it.
Worse, that accidental 17 was already a lie. `SttpHttpPort.close` calls
`java.net.http.HttpClient.shutdown()`, which the JDK added in Java 21. A
Java 17 JVM would happily load a class file stamped 17 and then die with
a `NoSuchMethodError` the first time somebody closed a client — a failure
at shutdown, far from the dependency that caused it. Proof: setting the
flag introduced here to 21 compiles the whole build, and setting it to 17
fails on exactly that line. So the compiler is now checking the API
surface, not only stamping a number.
What changed:
- `build.mill` gains `-java-output-version:25`. This is Scala 3's name
for `javac --release`: it stamps the class-file version *and* removes
every JDK API newer than the named release from the compiler's view.
- `.mill-jvm-version` pins `temurin:25`. The flag is rejected outright by
a compiler running on an older JDK ("25 is not a valid choice for
-java-output-version"), and Mill was running on a Coursier-provisioned
JDK 21 regardless of the JDK on `PATH`. Mill now downloads and runs on
Temurin 25, which also covers the scoverage sub-modules — those do not
inherit a per-module `jvmId`, so a build-wide pin is the only setting
that reaches them.
- Both CI toolchains move from 21 to 25: the `java-version` default in
`.github/actions/scala-toolchain` (all four GitHub workflows take it)
and the container image in `.forgejo/workflows/ci.yml`. Both Coursier
cache keys now hash `.mill-jvm-version`, because the provisioned JDK is
unpacked into that same cache and a stale key would restore the wrong
one.
- `README.md` states the requirement next to the install coordinates, and
`CONTRIBUTING.md` next to the setup instructions.
This narrows the audience, and that is the point of writing it down. A
consumer on Java 21 could link against the jars this build produced
yesterday; against these jars they get an `UnsupportedClassVersionError`
at class-load time. The trade is deliberate: Java 25 is the current LTS,
a stated floor that fails loudly at load time is kinder than an unstated
one that fails later with a `NoSuchMethodError`, and the flag now makes
the compiler enforce whatever floor we pick instead of leaving it to a
compiler default.
Note for anyone tempted to blame the client-lifecycle work: `shutdown()`
is a Java 21 method, so 25 is a policy floor rather than a technical
requirement. The README says so and invites an issue from anyone who
needs 21 — lowering it is a one-line change.
BREAKING CHANGE: the published jars now require a Java 25 runtime
(class-file major version 69). Java 21 and Java 17 JVMs, which could load
the previously emitted Java 17 bytecode, fail with
`UnsupportedClassVersionError`. Building the project also requires JDK
25; `.mill-jvm-version` provisions it automatically, so no manual install
is needed to compile.
The recent run of breaking changes was timed to land before 0.1.0 freezes the public surface. That freeze means nothing without a tool that enforces it, and until now there was none: no plugin, no baseline, and a section in RELEASING.md saying so. MIMA — the Migration Manager — compares the class files a build produces against the class files of an already-released version and reports every difference that would stop a program compiled against the old jar from linking against the new one. It works on bytecode, so it catches what the compiler cannot see from this side: a copy overload that changed arity, an opaque type whose representation moved. It is now wired through the mill-mima plugin, declared in build.mill's header as com.github.lolgab::mill-mima::0.2.2. The two colons before the version are Mill's "add the platform suffix" spelling; Mill resolves that to mill-mima_mill1_3, the build for the Mill 1.x line that .mill-version pins at 1.1.7. 0.2.2 was the latest stable version in that artifact's maven-metadata.xml on repo1.maven.org. The Mima trait is mixed into Codeberg4sPublishModule rather than into each module, so one declaration covers all five published artifacts: the plugin derives every coordinate from pomSettings().organization, artifactId() and mimaPreviousVersions. Nothing has been published, so there is nothing to compare against. Publish.binaryCompatibleWith is therefore Seq.empty, and ./mill modules.__.mimaReportBinaryIssues stops with the plugin's own "No previous artifacts configured" message. That costs nothing: no other task depends on that command, so compile, test and verify.sh never reach it. It is deliberately absent from verify.sh, which has to run offline and in seconds while this check downloads previous artifacts from Maven Central — the release checklist runs it instead. Arming it once 0.1.0 is on Central is a one-line edit. Two traps are handled here rather than left to be discovered. First, mimaPreviousVersions on a shared trait means every future module inherits the claim that each listed version of *it* exists on Central. A sixth artifact first published in, say, 0.3.0 would send MIMA looking for a 0.1.0 of itself that was never uploaded, and the run would fail on a download error that says nothing about compatibility. The trait's scaladoc carries the override such a module needs. Second, the response models' constructors are private[codeberg4s], and Scala erases qualified private to public bytecode, so the expectation recorded earlier in RELEASING.md was that MIMA would report <init>, apply and copy on all 131 of those classes the moment a field was added. Measured against mill-mima 0.2.2 and Scala 3.8.4 — by publishing modules.domain locally as a throwaway 0.1.0 and re-running the check — that is not what happens. MIMA reads Scala 3's own signature and honours the qualified private: the constructor, copy, every copy$default$N and the companion object's apply are never reported. Exactly one member leaks, the static forwarder Scala 3 emits on the class so Java callers can reach the companion's apply, because a forwarder carries no Scala-side access information. Appending a field to a 2-field model and to a 21-field model each produced one report; inserting the same field at the front produced six, because the Product accessors _1, _2 … are genuinely public and genuinely change result type. The same insertion into a command model with a public constructor produced fourteen, which is the check doing its job. So the filter is one line per model that gains a field, written in the release that adds it, and no standing blanket is added now: a wildcard broad enough to cover 131 classes would also silence a real break in a command model in the same package. RELEASING.md now carries the measured table, the validated filter snippet — ProblemFilter.exclude, singular, where sbt-mima spells it ProblemFilters.exclude — the advice to append new fields rather than insert them, and the procedure for re-taking the measurement when Scala or the plugin moves. CHANGELOG.md loses the claim that MIMA is unwired. The throwaway local Ivy publication used for the measurement was deleted afterwards. Leaving a fake 0.1.0 in ~/.ivy2/local would make a later run silently check against a jar nobody released.
The `./mill` launcher downloaded a ~60 MB executable over the network, made it executable and ran it, with no check that the bytes were the ones Mill published. Everything this project does — compile, test, sign, publish — runs through that executable, so whatever it turned out to be would have been trusted completely. `scripts/cpd.sh` already pins PMD by digest and `.github/actions/scala-toolchain/action.yml` already pins scala-cli by digest; the launcher was the one fetch left unpinned, and it is the one that matters most. A new `.mill-checksums` file records the SHA-256 of each Mill distribution the launcher may run. There is one line per platform because the launcher picks a different native binary per operating system and architecture, plus the portable JVM launcher: five entries for Mill 1.1.7. Every digest was produced by downloading the artifact from Maven Central and checking it against the `.sha1` sidecar published alongside it before recording the SHA-256. The file's header carries the exact shell loop for regenerating it when Mill is bumped. The launcher now checks the file it is about to execute against that digest, and refuses to run anything that does not match or is not listed. The check runs on the already-cached path too, not only after a download: the cache lives outside the repository, is shared with every other project on the machine, and on any machine that ran Mill before this commit it holds a binary that was never verified at all. Hashing 60 MB takes about 35 ms, which is not a reason to trust a file forever because it was fetched once. A version with no recorded digest fails before the download starts rather than after it. `DEFAULT_MILL_VERSION` in the launcher said `1.1.6-104-5bbe1e` — an untagged development snapshot, 104 commits past the `1.1.6` tag, which Maven Central confirms is a nightly-style publication rather than a release. `.mill-version` pins `1.1.7`, and that pin wins in this checkout, so the snapshot only applied to a checkout with no `.mill-version` at all. Even as a fallback, landing on an unreleased snapshot is a build nobody can reproduce, so the default is now `1.1.7` and the two agree. The launcher is vendored from upstream Mill, so both edits are marked `LOCAL CHANGE` and a note at the top of the file says they must be re-applied if the script is ever regenerated. Verified on Linux x86-64: a warm cache runs, a removed cache re-downloads and runs, a byte appended to the cached binary is refused with both digests printed, an unlisted version is refused without downloading, and upstream's `MILL_TEST_DRY_RUN_LAUNCHER_SCRIPT` hook still prints the URL as before. `shellcheck -s sh mill` is clean.
`uses: actions/checkout@v7` names a tag, and a tag is a mutable pointer. Whoever controls that repository can move `v7` to different code at any moment, and every workflow here would fetch and run whatever it now points at. In `release.yml` that job holds `MILL_PGP_SECRET_BASE64`, the key the published artifacts are signed with, so moving one tag would be enough to read the key out of the runner. This is not hypothetical: it is how the `tj-actions/changed-files` compromise reached tens of thousands of repositories in 2025. Every third-party `uses:` in the repository now names a full 40-character commit SHA, which cannot be repointed, with the release it corresponds to in a trailing comment so the line is still readable at a glance. Four GitHub workflows and the composite action, resolved on github.com: actions/checkout v7 -> 3d3c42e5 # v7.0.1 actions/upload-artifact v7 -> 043fb46d # v7.0.1 actions/setup-java v5 -> b6effb05 # v5.7.0 actions/cache v6 -> 55cc8345 # v6.1.0 actions/upload-pages-artifact v5 -> fc324d35 # v5.0.0 actions/deploy-pages v5 -> cd2ce8fc # v5.0.0 `.forgejo/workflows/ci.yml` runs on Codeberg and had the same problem. Its two SHAs come from **code.forgejo.org**, not github.com, because a Forgejo runner resolves a bare `actions/<name>` against its DEFAULT_ACTIONS_URL and that is code.forgejo.org on Codeberg — the same tag on the two forges is not the same code: actions/checkout v4 -> 11d5960a # v4.4.0 actions/cache v4 -> 0057852b # v4.3.0 A comment in that file records which forge the SHAs belong to, so an instance pointed at a different actions registry knows to re-resolve them rather than assume they will work. No SHA was written from memory. The GitHub ones were resolved with `gh api repos/<owner>/<repo>/commits/<tag>` and confirmed independently with `git ls-remote --tags`, which showed the same SHA carrying both the major tag and the exact release tag named in the comment. The Forgejo ones were resolved with `git ls-remote --tags` against code.forgejo.org and confirmed against that instance's `/api/v1/repos/.../tags/v4`. Nothing was left unresolved. `uses: ./.github/actions/scala-toolchain` is deliberately left as-is. A `uses:` starting with `./` is not fetched from anywhere — it runs the copy of that action in this repository at the commit being built, so the checkout above already pins it. The header of `release.yml` says so, in case a future reader reads the unpinned line as an oversight. Behaviour is unchanged: the same action versions run as before, they are now identified by content rather than by a name someone else can reassign.
The previous commit replaced every action tag with a commit SHA, which
is what stops someone repointing the code CI runs. It also freezes those
pins: a SHA never moves, so a bug fixed upstream never arrives, and the
pins age until nobody remembers whether they are deliberate or forgotten.
Pinning without an update path trades one problem for another.
`.github/dependabot.yml` adds the update path. Once a week it checks the
pinned actions, and when one has a newer release it opens a pull request
that rewrites both the SHA and the trailing `# v7.0.1` comment. A human
reviews and merges it, so the pins move forward on purpose.
Checked first that nothing else already does this: there is no
`dependabot.yml`, no `renovate.json`, no `.renovaterc`, and no
Scala Steward workflow anywhere in the tree. `docs/READINESS.md` listed
dependency automation as a gap rather than as something configured, so
this is the first such config, not a second one competing with an
existing one.
Two `updates` entries, both for the `github-actions` ecosystem:
* `directory: "/"` — for this ecosystem that means `.github/workflows`,
which is the four workflow files.
* `directory: "/.github/actions/scala-toolchain"` — the composite
action. A root-level entry does not reach into `.github/actions/`,
and its two pins are the ones the release job's toolchain is built
from, so leaving them out would leave the pins that matter most
unattended.
Both entries group all matching updates into a single pull request.
Six separate pull requests to advance six `actions/*` pins is six
reviews of the same decision, and the predictable outcome is that none
of them are read. Commit messages come out as
`ci(deps): bump actions/checkout from 7.0.1 to 7.0.2`, matching the
Conventional Commits format the rest of the history uses.
Deliberately out of scope, and said so in the file so the next reader
does not think it was missed:
* Scala and Mill versions. Dependabot has no Mill support at all;
`mill mill.scalalib.Dependency/showUpdates` reports those today and
`docs/READINESS.md` still names Renovate as the eventual automation,
partly because Renovate also runs on Codeberg.
* `.forgejo/workflows/ci.yml`. Dependabot cannot see it, and should
not touch it: its SHAs come from code.forgejo.org while Dependabot
resolves against github.com, where the same tag is different code.
That file stays hand-maintained, as its header now states.
`docs/READINESS.md` is updated in the same commit so the gap list stops
claiming there is no automation for the part that now has some.
`modules/codec` declared two jsoniter-scala artifacts, `-core` and `-macros`, and imported only the first. The macros artifact exists for one thing: `JsonCodecMaker.make`, the compile-time macro that *derives* a `JsonValueCodec[A]` from a case class. This library derives no codecs at all. `JsonValue.scala` hand-writes a single `JsonValueCodec` over a small document model, and every DTO assembles itself from that document through `JsonFields`, because the Forgejo API needs "every field optional, `null` and absent identical, unknown kinds tolerated" — a shape derivation cannot express (docs/HAZARDS.md §1). The evidence is a grep: every `com.github.plokhotnyuk` import under `modules/` resolves to `jsoniter_scala.core`, across seven import lines in two files, and none to `jsoniter_scala.macros`. Why it matters to someone who has never seen this repository. Mill's `mvnDeps` is compile *and* runtime scope, so a declared dependency is written into the published POM and every consumer resolves it transitively. jsoniter-scala-macros is roughly a megabyte of derivation machinery, so anyone depending on `codeberg4s-client` was downloading and shipping a jar that no line of code on either side of the boundary touches. README.md advertises "sttp client4 and jsoniter-scala, that is the list"; that claim is now literally true of the generated POM rather than approximately true. Nothing at all changes for callers: no source file is edited, and `./mill modules.codec.pom` now emits three dependencies (jsoniter-scala-core, scala-library, and the two sibling modules) instead of four. docs/VERSIONS.md loses the macros row per its own §7.5, and ADR-0003 gains a correction: it had argued the macros module was "compile-time only", which is true of the macro expansion but not of the dependency scope Mill assigns it.
Resolved from the canonical resolver — `maven-metadata.xml` for `com/github/plokhotnyuk/jsoniter-scala/jsoniter-scala-core_3` lists 2.40.1 as the newest stable, two releases past the pinned 2.39.1. What is in those two releases, from the project's own notes: 2.40.0 adds a key-sorting option to the circe booster and reduces allocations when `JsonReader.readNumber` parses small integers; 2.40.1 makes encoding of sorted-key JSON objects faster in that same booster. No API is removed or renamed, and the circe booster is a separate artifact this build does not depend on, so the only change that reaches this code is the `readNumber` allocation work. That one needed checking rather than assuming, because `JsonValue` matches on the *concrete* class `readNumber` hands back and its Scaladoc claims to know the complete set. Reading the 2.40.1 sources (`jsoniter-scala-core_3-2.40.1-sources.jar`, `JsonReader.readNumber`), the return paths are still exactly three: `java.lang.Long` for a whole number that fits in 64 bits, `java.math.BigInteger` for a whole number that does not, and `java.math.BigDecimal` for anything with a fraction or an exponent. The Scaladoc's version reference moves to 2.40.1 so it keeps naming the version somebody actually looked at. `./verify.sh --with-slow` passes: 3658 unit tests, coverage gate green, duplication unchanged at the recorded 363 groups.
Resolved from the canonical resolver: `maven-metadata.xml` for `org/scoverage/scalac-scoverage-reporter_3` (and its `-serializer` and `-domain` siblings, which move in lockstep) lists 2.5.2 as the newest stable. The build was pinned at 2.3.0, six releases behind. What this artifact does, for anyone who has not met scoverage. Since Scala 3.4 the coverage *instrumentation* is part of the compiler, not a plugin, so `scoverageVersion` no longer selects how code is measured. It selects the libraries that read the compiler's raw measurement files and turn them into the XML and HTML reports — which is what `verify.sh` step 7 and `scripts/crap.sc` consume. A bump here therefore changes reporting, not the numbers being reported. The one breaking change in the range is 2.4.0, which drops support for Scala 2.13.15-and-earlier and 2.12.16. This project is Scala 3 only (3.8.4), so nothing in the build can be affected by it. The rest are 2.4.1 (fixes instrumentation of pattern-matching assignments), 2.4.2 and 2.5.1 (add newer Scala 2 versions), 2.5.0 (dependency updates) and 2.5.2 (incremental coverage support). Evidence that reporting did not shift: `./verify.sh --with-slow` reports the same per-module coverage as the run before the bump, to the statement — domain 3968/3968, core 794/822, codec 6053/6358 — with the same CRAP table over 2217 methods and duplication unchanged at 363 groups.
Resolved from the canonical resolver: `maven-metadata.xml` for `org/scalameta/munit_3` lists 1.3.5 as the newest stable, one patch past the pinned 1.3.4. munit is the test framework the five `*.test` modules run under, and it is a test-only dependency — it appears in no published POM, so nothing here reaches a consumer. The 1.3.5 release notes are dominated by the project migrating its own build from sbt 1 to sbt 2 and dropping JDK 8 from its build matrix. That reads alarming and is not: those are changes to how munit is built, not to the API a test suite compiles against. Nothing in the notes removes, renames or redefines an assertion, a fixture or a tag, which is the surface this repository uses. The remaining entries are compilation error reporting that no longer depends on terminal capabilities, and a Java source formatter. The same 3658 unit tests run and pass, with the same coverage, duplication and CRAP results as before the bump. `munit-scalacheck` stays at 1.3.0, which is its own newest stable — the two artifacts version independently and this is not a mismatch.
Resolved from the canonical resolver: `maven-metadata.xml` for `org/scalameta/scalafmt-core_3` lists 3.11.5 as the newest stable, one patch past the 3.11.4 pinned in `.scalafmt.conf`. This is a `style:` commit on its own, and deliberately so. `.scalafmt.conf` sets `align.preset = most`, which lines up tokens vertically across neighbouring lines; a formatter release that changes how that alignment is computed can rewrite every file in the repository. Mixing that churn into a dependency or logic commit would bury the real change in thousands of whitespace lines, so docs/VERSIONS.md §7.4 requires the bump to travel alone with the whole tree reformatted in the same commit. Reformatting the whole tree under 3.11.5 changed nothing: `./mill mill.scalalib.scalafmt/` reports 850 of 850 files formatted and `git status` lists no modified Scala source. That is why this diff is four text files and no code. The 3.11.5 release is a documentation website migration plus four fixes — inverted offsets on empty trees, a tolerated CLI error that could mask a real one, the runner naming the failure it exited on, and a brace/colon oscillation in `RemoveScala3OptionalBraces` — none of which this configuration hits. The zero-file result doubles as re-verification of the note in `.scalafmt.conf` above `rewrite.imports.groups`, which explains that the catch-all `.*` pattern can safely precede the `scala\..*` group because scalafmt assigns an import to its longest matching pattern rather than its first. Had 3.11.5 changed that, `scala.*` imports across the tree would have moved groups and the reformat would not have been a no-op. So the comment's "verified against" version moves to 3.11.5, as do the formatter rows in SCALA_CODE_STYLE.md and docs/CONSTITUTION_MAPPING.md.
CHANGELOG.md is what the 0.1.0 tag will carry, and a first-time reader would have acted on several things in it that were not true. The counts were wrong. It described "seven endpoint groups ... plus version"; CodebergClient exposes nine accessors, and `downloads` — the two ZIP-fetching Actions operations — was missing from the list entirely. It said "61 REST operations" and, under Known limitations, "Endpoint coverage is 61 of 439 in-scope operations"; the real figure is 439 of 439, which docs/API_INVENTORY.md section 0 and docs/ROADMAP.md have both said for a while. It claimed 1,072 unit tests; ./verify.sh reports 3,658. It listed `GET /repos/issues/search` as deferred when IssueApi implements it and the inventory ticks it. The architecture-boundary sentence named the wrong things. verify.sh bans sttp, upickle, ujson and the Future family below `client`, and sttp inside `codec`. It does not ban jsoniter-scala below `client`, and could not: `codec` is below `client` and jsoniter is what it is built on. The remaining figures — 54 golden fixtures, the coverage rates, the CRAP result, the duplication count — are now the ones this commit measured rather than ones carried forward. Where something is still unproven it says so: there is no mutation score for this repository, and the ScalaCheck suite still has no property test for the page walker. The larger gap was that this branch changed a great deal that a release note has to describe, and none of it was described. Three new sections cover it. "Changed" lists the breaking changes, with a note at the top of the entry explaining why a first release lists any at all: nothing is on Maven Central, so these are not a migration path from an earlier release, they are the decisions taken before the tag freezes the surface. Closed response constructors, the sixth CodebergError case, the removal of core.Pagination, byte-carrying response bodies, the two JSON number cases, JsonFields over a vector, the upload builders, the two new config bounds and the Java 25 floor. "Security" covers the four fixes: a base URI carrying credentials is rejected rather than concatenated into every logged URI, dot segments are rejected in single-segment identifiers, a credential-bearing response body is withheld from a decode-failure snippet, and a response body is bounded rather than limited only by the heap. The configured credential is also applied after caller headers now, which was latent rather than live and is recorded as such. "Fixed" covers the lifecycle bugs: the JDK HTTP client the library created was never actually shut down, so an application building a client per instance leaked a connection pool per instance; and a retry sitting in backoff when the client closed was left with no outcome at all, so a clean shutdown waited on that Future forever. "Performance" carries the measured numbers and nothing rounder than they are. Decoding a 170,251-byte page of fifty repositories allocates 1,133,632 B/op against 1,944,816 before, which is 41.7 %; the end-to-end path allocates 963,360 B/op against 1,303,928 for the same decode via a String, which is 26.1 % and works out as exactly the two copies of the page that were removed. Every figure names scripts/alloc-bench.sh and the JVM it ran on, and repeats that harness's own warning that its wall-clock times are a direction of travel rather than a figure.
The top of this file has said "439 of 439 in-scope operations, 100 %"
for a while. The "Distance to 0.1.0" table at the bottom still said
"61 / 439 ... the surface is 14 % of the way there", so the same
document answered its own headline question two different ways, and the
pessimistic answer was the one a reader looking for release readiness
would land on.
Every figure in that table is now one measured on this commit rather
than one carried forward:
- endpoints: 439 / 439, from docs/API_INVENTORY.md section 0
- unit tests: 3,658, from the count ./verify.sh prints, not 1072
- coverage: domain 100.00 % statement / 100.00 % branch, core
96.59 % / 92.48 %, codec 95.20 % / 91.47 %, so the row saying the
floors "have not yet been asserted against a fresh report" is
replaced by the report they were asserted against
- CRAP: 2,217 methods measured, worst 28.0 against a limit of 30
- duplication: 363 groups, not 323
Two clauses are deliberately left saying "no". The mutation score does
not exist for this repository and is called unproven rather than
quietly dropped. And CPD is not clean: 363 groups is what the tool
reports, and ./verify.sh --with-slow passes only because it compares
that count against a recorded baseline and fails on an increase. The
table says so in those words so that a green gate is not read as an
absence of duplication.
The Phase 4 checkboxes this branch completed are ticked, with what is
still missing spelled out beside each:
- PMD CPD: ticked, because the tool runs and the gate holds. The
threshold is still 40 tokens and every group is still reported.
- CRAP and the coverage gate: ticked, both now run against a freshly
generated scoverage report rather than only implemented.
- Nightly spec-drift: ticked. It moved to .github/workflows/nightly.yml
behind a real schedule: trigger, where the previous copy was gated
on a schedule event its workflow never emitted and so had never run.
It still only warns on a sha mismatch instead of opening an issue,
which is recorded as the remaining piece.
- Maven Central and MIMA: still unticked, but "no MIMA setup at all —
no plugin, no baseline" is no longer true. MIMA is wired through
mill-mima 0.2.2 on the shared publish trait. What remains is the
publication itself.
- Stryker4s: still unticked. The note that scalameta parsed all 196
production files now says those modules hold 503 today, so the
reader can see that even the parse half of that proof predates the
current sources.
Three older figures elsewhere in the file were stale for the same
reason and are corrected: the build declares seven modules rather than
six (examples joined it), CI is no longer only .forgejo, and the 50 MB
attachment line under "Out of scope" is now enforced by
CodebergConfig.maxDownloadBodyBytes rather than only written down.
The two Actions endpoints that answer with a ZIP archive live on `ActionDownloadApi`, and `CodebergClient` exposes that class as `client.downloads` — a top-level accessor, alongside `client.repos` and `client.issues` rather than underneath one of them. Ten places across the scaladoc, the README and the self-hosting guide told readers to reach it as `client.repos.actions.downloads` instead, which is a path that does not exist: `client.repos.actions` is `RepositoryActionApi`, and that class has no `downloads` member. Anyone who typed what the documentation said got a compile error. The reference page `site/src/reference/api-groups.md`, the FAQ, SECURITY.md and the CHANGELOG already said `client.downloads`, so the wrong form was the minority spelling, not a rename that half-landed. Documentation only — no signature, no behaviour and no test changed.
`docs/VERSIONS.md` still explained the codec layer in upickle's vocabulary, two commits after upickle was removed from the build. Two passages were wrong. The "JSON library" note ended by saying the style guide's rule applies "translated to jsoniter-scala `ReadWriter`s". `ReadWriter` is upickle's codec type; it does not exist in this repository, and it has no jsoniter-scala counterpart by that name — the jsoniter type is `JsonValueCodec[A]` from `jsoniter_scala.core`. The same paragraph also claimed the style guide's jsoniter examples "do not apply to this repository", which was true when the code was upickle and is now only half true: the library is the right one, but the examples derive a codec per data-transfer object with `JsonCodecMaker`, and this build derives none. The rewrite says which half applies and why the other half does not, pointing at `docs/HAZARDS.md` §1 for the reason (the pinned spec marks no field required, never says `nullable`, and live payloads send `null` where an array is promised — a derived codec answers all three by failing). The "Not adopted" table listed "circe / jsoniter-scala | rejected", so the document rejected the library the project runs on. It is now four rows that match reality: circe rejected, upickle removed with a pointer to the ADR that reversed it, and `jsoniter-scala-macros` rejected on its own terms, since that is the artifact this build declines rather than jsoniter itself. The cats-effect row's "four artifacts" was left over from when the macros jar was declared; §3 of the same file lists three.
ADR-0003 records a decision to move the JSON boundary off upickle and onto jsoniter-scala. A blanket rename of the library name had replaced every occurrence of "upickle" in the file, including the ones describing what was being moved away from, which left the document arguing with itself: it said it superseded an earlier ADR "which chose jsoniter-scala", in order to choose jsoniter-scala; it credited sttp with a "first-party jsoniter-scala integration" that "was never used" when the unused integration was the upickle one; and it listed "Keep jsoniter-scala" as a rejected alternative. A reader had no way to recover what the decision actually was. Six occurrences are restored to "upickle" or "ujson" — the two that name the superseded choice, the two grounds it rested on, the removed sttp integration, the leniency `JsonDecoder[String]` no longer inherits, the tracing visitor that produced per-field parse paths, and the rejected alternative. Occurrences that genuinely mean jsoniter are untouched. ADR-0005's closing note said the style guide's "Ox and jsoniter examples no longer match the code". Half of that expired when ADR-0003 landed: the guide's jsoniter examples now name the library in use. What still diverges is narrower and worth stating precisely — those examples derive a codec per data-transfer object with `JsonCodecMaker`, and this build derives none. No decision is changed or reopened; this restores the record of one already made.
The artifact step was labelled "Coverage HTML" and promised that "the gate reports a percentage, and the report says which lines". No step in CI produces HTML. `verify.sh`'s coverage step runs `mill <module>.scoverage.xmlReport` and stops there, because XML is what `scripts/coverage-gate.sc` and `scripts/crap.sc` parse. Anyone who downloaded the artifact looking for a browsable report found none. Rather than add a `mill` call here — the header of this file rules that out, and it is the rule that keeps `./verify.sh` locally and CI in step — or charge every local `./verify.sh` run for rendering a page nothing reads, the step now says what it uploads. The XML still answers the "which lines" question, statement by statement; the comment points at `./mill modules.__.scoverage.htmlReport` for a reader who wants it as a web page. The upload path narrows from `out/modules/*/scoverage/` to `out/modules/*/scoverage/xmlReport.dest/scoverage.xml`. The old glob swept up the whole task-cache directory — roughly forty JSON files of Mill bookkeeping per module, alongside the one file worth keeping. The artifact is renamed `scoverage-xml-<run id>` so a downloaded zip is not mistaken for the HTML the old name implied.
The architecture-boundary step forbade a set of imports below the client module. Its JSON half read `upickle|ujson` and had done since before commit 48f64fe replaced upickle with jsoniter-scala, so it was checking for two packages that no longer appear anywhere in the repository while the package that does appear, `com.github.plokhotnyuk.jsoniter_scala`, went unmentioned. Adding that import to `modules/domain` or `modules/core` — the two modules whose whole definition is that they know neither the transport nor a JSON library (PLAN.md §3.1) — would have printed "boundaries clean". Verified before and after. With `import com.github.plokhotnyuk.jsoniter_scala.core.JsonValueCodec` temporarily added to `modules/core/.../Exec.scala`, the old pattern matched nothing and the step passed; the new one fails with core must not know about sttp, a JSON library or Future modules/core/.../Exec.scala:3:import com.github.plokhotnyuk... The violation was reverted before committing and the full `./verify.sh --with-slow` is green. Three changes: - the vendor prefix `com.github.plokhotnyuk` is matched rather than the exact package, so a `jsoniter_scala.macros` import — the artifact docs/adr/0003 deliberately does not declare — trips the same rule; - the JSON alternatives move into their own `FORBIDDEN_JSON` variable and gain circe, play-json, zio-json, Jackson, json4s and Gson. upickle and ujson stay. None of these is a dependency, which is the point: a pattern for a library that is not there costs one alternation and earns its keep on the day somebody adds one; - `modules/transport` gets the mirror-image check. docs/adr/0003 gives "the JSON library stays out of the transport" as the reason the unused sttp-upickle integration was dropped — the transport reads every body as a `String` and hands it to `modules/codec` — and nothing was holding that true. `modules/codec` is exempt, as it must be: it is the module that owns the JSON library. Its own boundary, that it must not import sttp, is unchanged.
Twenty-four tracked files cite `PLAN.md` as normative — `docs/ROADMAP.md` seven times, `verify.sh` in its header, `docs/VERSIONS.md`, `CONTRIBUTING.md`, `README.md`, five of the six ADRs, and every script under `scripts/` — and `.gitignore` listed `PLAN.md` among the AI-generated scratch documents. So the file existed only on the machine where it was written. Anyone who cloned this repository read "PLAN.md §3.1", "PLAN.md ADR-3", "PLAN.md §6.6" and had no way to open any of them. A document that other documents treat as the source of truth cannot be untracked. It is tracked rather than dissolved into the docs it is cited from, because the citations are by section number: rewriting them all into prose would touch two dozen files, lose the section anchors, and cost the record of what was decided before any code existed. Read in full before committing. It holds no credentials, no tokens, no personal data and nothing about anyone's infrastructure — it is a design document. The one environment variable it mentions, `CODEBERG_TOKEN`, is an illustrative `sys.env` call in an API sketch, and the same name is already documented in README.md and CONTRIBUTING.md. The file is a snapshot of the plan as written before implementation, and parts of it were overruled while the code was built. Committing it unannotated would install a stale document as a second, contradictory source of truth, so a "Status of this document" header is added at the top: it says the document is historical, that an ADR wins wherever the two disagree, and it tables the six divergences that matter (upickle, the `codeberg4s.*` package prefix, the `sttp-transport` module name, softwaremill/retry, the Gherkin acceptance pipeline, quicklens), each with a link to the ADR or doc that settled it. Nothing in the body is edited; the plan reads as it was written. `.gitignore` keeps the whole scratch-document list — PLANS.md, PLANNING.md, SPEC.md and the rest are still ignored, and the section's `git add -f` note is generalised rather than dropped. Only `PLAN.md` comes off, with a comment saying why it is the exception.
This document listed as blocking several things that have since been built, which makes it worse than no document: a contributor reading it would conclude the project has no contribution path, no CI and no binary-compatibility policy, and all three exist. Rechecked every claim against the repository rather than against memory. Moved to a "Resolved" section, each verified by opening the file: the release process, the community health files and issue templates, the GitHub workflows with SHA-pinned actions, the MIMA wiring, the examples module inside the gate, and the ten guides with mdoc compiling their snippets. The duplication gate is green now, so the entry saying it is red is gone too. What remains blocking is shorter and sharper. Nothing is published — the version is still a SNAPSHOT and no tag exists. And no mutation score exists, which matters more than it looks: coverage says a line ran, not that breaking it would be noticed, and that is the one number PLAN.md asks for that this repository still cannot show. Two entries are deliberately hedged rather than claimed. The site workflow depends on a repository setting that cannot be seen from a clone, so hosted documentation is recorded as unproven rather than done. And the duplication gate passing is recorded as passing against a baseline of 363 tracked groups, not as clean code, because those are different statements and the second one is not true.
|
Important Review skippedToo many files! This PR contains 251 files, which is 151 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (251)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CI has never passed on this repository. The first run against this branch failed twice, and both causes predate it — the same failures are on main. The verify job died at the coverage step. `scripts/coverage-gate.sc` and `scripts/crap.sc` are scala-cli scripts, and scala-cli compiles through a Bloop server by default. Starting one means downloading an entire Scala 2.12 toolchain — bloop-frontend, monix, scalaz and the rest — from repositories that answer slowly or not at all, and the handshake times out before it finishes. The error surfaces as "FAILED at step 7: coverage thresholds", which points at the gate rather than at the compiler that never started. `scripts/site.sh` already knew about this and passes `--server=false`, which makes scala-cli compile in-process. The two calls in verify.sh did not, and neither did the allocation harness. They do now. A script of a few hundred lines does not need a build server, and the flag costs nothing locally: the full gate still runs in well under two minutes. The site job died sooner and more plainly — `'cs' is not on PATH, and this script needs it`. The shared toolchain action installs scala-cli and warms a Coursier *cache*, which is not the same thing as putting the Coursier *CLI* on PATH, and site.sh calls `cs fetch` twice to resolve the mdoc and scaladoc classpaths. The action now installs it, pinned by version and by sha256 exactly as scala-cli already is, since this action also runs in the job that holds the signing key. The digest was taken by downloading the v2.1.24 x86_64-linux asset and running the binary, not copied from anywhere.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Hardens the library across security, correctness and allocation, and finishes the
build and release plumbing that a 0.1.0 release needs. 57 commits, one logical
change each.
Eighteen of them are breaking. That is deliberate and time-boxed: nothing is
published to Maven Central yet, so the 0.1.0 tag will become the binary-compatibility
baseline. Every breaking change here is one that would otherwise cost a major
version later, and this pull request is the last moment it is free.
./verify.sh --with-slowpasses: 3658 tests, 100 % domain coverage, CRAP withinlimits, duplication green.
Why
An audit of the whole codebase turned up 31 verified defects. The ones that
motivated this work:
A password in the configured base URI reached every log line.
BaseUri.fromaccepted
https://user:password@host/api/v1, andRedaction.uriconcatenatedthat value verbatim into the URI carried by every
CallContext— which isattached to every error and handed to every telemetry sink. The library's headline
promise is that no error can carry a credential.
Three endpoints could echo a live credential into an error. The 2xx bodies of
token creation, OAuth2 application creation and Actions runner registration carry
secret material. If one failed to decode,
CodebergError.DecodingFailedcapturedan excerpt of that body.
Decoding allocated far more than it needed to. The JSON parser builds an
ordered field vector and the decoder immediately threw it away by calling
.toMap, at every level of every object in every response. Measured, that singlecall was 43 % of all decode allocation.
Two lifecycle bugs made clean shutdown impossible. The JDK
HttpClientwasnever released — sttp only closes the client it created, and it never creates one
when handed an
ExecutionContext. And a retry waiting on backoff when the clientclosed produced a
Futurethat never completed at all, so an application shuttingdown cleanly would wait on it for the life of the process.
How it works
Security.
BaseUrinow rejects user information, a query and a fragment, andRedactionstrips them defensively as well, because it is reachable from testfakes and cannot assume validation ran.
Decodegained asensitiveflag thatsubstitutes a fixed placeholder for the body excerpt on the three
credential-carrying endpoints, keeping the failure diagnosable without disclosing
anything.
PathSegmentmoved to the root package asprivate[codeberg4s]so thethree identifiers that had copy-pasted its rules call it instead — that copying is
why
.and..were rejected as a branch name and accepted as a username.Multipart media types are validated for blanks and control characters, in the
domain type and again in the transport. Response bodies are bounded, with a
non-retryable
TransportCauseso an oversized body is not re-downloaded on everyattempt.
Correctness.
RetryEngine.giveUpnow consultsisRetryable, so a call thatsaw a retryable 503 and then a terminal 404 reports the 404 rather than claiming
the policy was exhausted.
PageWalksignals truncation instead of returning ashort result indistinguishable from a complete one.
BinaryResponse,RequestBodyand both upload commands compare their bytes rather than their array identity.
FutureTimersubclassesScheduledThreadPoolExecutorand overridesdecorateTaskso
shutdownNow()'s returned list can be walked and every waiting promise failed.SttpHttpPortbuilds the JDKHttpClientexplicitly andCodebergClient.close()calls
shutdown()on it.Allocation.
JsonFieldsreads from the parser's own vector,JsonValue.Numholds a
Longfor whole numbers, arrays are collected with a builder that stops atthe first failure, the Link header is parsed once in linear time, the redacted URI
is built once per call into one
StringBuilder, response bodies travel as bytes sothe parser no longer re-encodes a string the transport just decoded, and the no-op
telemetry sink stops allocating.
Build.
-java-output-version:25makes the JDK baseline enforced rather thanaccidental. MIMA is wired on the shared publish trait, ready to arm once 0.1.0
exists. Every CI action is pinned to a commit SHA, the Mill launcher verifies a
checksum before executing what it downloads, and the unused
jsoniter-scala-macrosdependency is gone.
How to test it
Expected: green,
3658 tests executed, domain at 100.00 %, duplication363 groups · baseline 363,CRAP ok.The allocation claims are reproducible:
Read the header first — it is a single-threaded allocation counter, not JMH, and
it documents the two ways that misleads.
Measured against the baseline recorded when the harness landed:
To see the security fixes reject what they now reject:
Notes for reviewers
Parsing got 2.2 % worse. Duplicate-key detection costs about 19 KB on a large
page. The library previously disagreed with itself —
fieldreturned the firstvalue for a repeated key,
.toMapreturned the last — and the new rule is toreject the document. Forgejo serialises from Go structs and cannot emit a repeated
key, so a response carrying one was rewritten in transit. The cost is stated rather
than absorbed into the headline number.
Duplication is green against a recorded baseline of 363 groups, not clean. The
gate had been red since the jsoniter rewrite, against a stale 323. It is now
measured, and two refactors paid 15 groups off rather than absorbing them. 762 of
the 1350 reported locations are still in
modules/codec.Response-model constructors are now
private[codeberg4s]. 131 classes. Thelibrary's own tests and examples share that package prefix so nothing internal
changed; an external caller constructing a
Repositoryby hand in a test fixturemust now obtain one from a client call. The trade is that Forgejo adding a response
field stops being a major version.
The codec keeps its two-step design. Single-pass decoding straight from
JsonReaderwas considered and rejected: it would rewrite 112 DTO files to capturethe last 5 % of a gap the
JsonFieldschange captured 95 % of, and it needs aprecomputed field-name hash per DTO field, where a wrong literal silently drops a
field with no compile error.
Java 25 is a policy floor, not a technical one. The code's actual requirement
is Java 21 — that is where
HttpClient.shutdown()arrives, verified by compilingagainst 21 and 17. Worth knowing that this narrows the audience.
Still missing, and not fixed here. There is no mutation score.
scripts/mutate.shworks and the Stryker4s runner is proven against these sources, but a scored run
needs roughly 1400
mill testinvocations and has never completed. Coverage is nota substitute: it says a line ran, not that breaking it would be caught.
Two audit findings were wrong and are recorded as such. A reported
TeamNametraversal conflated two same-named types — the path-reaching one was already
validated. And a claimed directory traversal was never demonstrated against a real
deployment, so the commit that fixes the validator says only that the type's
documented promise was unmet, not that an exploit exists.