fix: audit ledger self-tamper defect, plus review fixes across CI, docs and code hygiene - #675
Closed
ginccc wants to merge 4 commits into
Closed
fix: audit ledger self-tamper defect, plus review fixes across CI, docs and code hygiene#675ginccc wants to merge 4 commits into
ginccc wants to merge 4 commits into
Conversation
…rflow AuditLedgerService caps its sequence table at 50,000 conversations and cleared the whole thing on overflow, on the reasoning that re-seeding from countByConversation was "correct, only slower". It is not: entries sit in the queue for up to a flush interval, so a conversation with queued entries has consumed positions the store cannot see yet, and the re-seed hands the same position out twice. The verifier grades a duplicate exactly like a gap (BROKEN), and unlike a gap there is no exculpatory record for it — so the ledger accused the deployment of tampering because its own table wrapped around. On a busy deployment the queue is never empty, so essentially every overflow produced duplicates. Evict only counters whose positions are all accounted for where a re-seed can see them (persisted, or attributed in `undelivered`); retain the rest. A ReentrantReadWriteLock spans "position consumed" to "entry visible in the queue" against eviction, since that window contains HMAC and Ed25519 signing. flush() publishes its in-flight batch — between poll and a successful append those positions are in neither queue nor store — and is synchronized so the scheduled writer and the @PreDestroy flush cannot poll interleaved halves. Still full after eviction yields UNSEQUENCED, which degrades the window to UNAVAILABLE rather than to an accusation. Both regression tests are mutation-checked: with the retain set emptied the duplicate test fails with expected <[0, 1]> but was <[0, 0]>. Also from the same review: - AgentGroupStore rejected HUMAN_DECIDES as needing "human group members (I6), which are not available yet" — 150 lines below its own I6 matrix that validates those members. They shipped in 10c; the missing piece is the resume path a paused tie-break needs. The test pinned the word "I6", so it now asserts on the offered alternatives instead. - SafeHttpClient claimed an overall wall-clock timeout across all hops, but the budget is only checked between hops, so a hop that trickles its body hung forever. Bound both the initial and redirect hops by one default and state what is actually true. - ToolRateLimiter.refill() overflowed `elapsedNanos * limit` after ~107 idle days, driving tokens negative so the bucket refused every later call. - ConversationStepRunner registered the in-flight conversation outside the try whose finally unregisters it.
The second half of a full-repo review. Nearly every problem was in something no automated check covered. CI - install.sh/install.ps1 (95 KB, the README's `curl | bash` path) were in no path filter, so an installer-only PR skipped the whole pipeline — and a skipped required check still satisfies branch protection. There was no shellcheck, lint, or syntax parse anywhere. New `shell-lint` job: bash -n, ShellCheck at warning severity (runner's own binary, no new action to pin), a PowerShell parse-only check, and PSScriptAnalyzer. install.sh is already warning-clean, so the gate lands green. - auto-approve treated an absent check as a passing one: a merge-conflicted PR never triggers CI/CD, so nothing failed and it would have approved with a body claiming all checks passed on a commit that was never built. Require Build & Test and Integration Tests by name. Docs-only PRs still approve — a job skipped by its `if` does report a check-run. Dead code and stray state - CannotExecuteException: no reference in main or test. - ILogoutEndpoint: JAX-RS interface with no implementing class, declaring /user/isAuthenticated and /user/securityType to OpenAPI, served by nothing. - grafana-data/ was orphaned by the move to a named Docker volume: it held a committed 1.4 MB grafana.db plus superseded provisioning copies. Its eddi-operations.json was a richer dashboard provisioned by nothing, so it moved to docs/monitoring/, is mounted next to the existing one, and both installers now fetch it. The rest is removed and the path git-ignored. Consistency - 366 inline FQNs across 168 files, against AGENTS.md 4.7. Not disambiguation: each simple name resolved to one class, and IConversationService imported java.util.List then wrote it fully-qualified anyway. The two real cases (mongo.HistorizedResourceStore extends datastore.HistorizedResourceStore and its Modifiable twin) were detected and left. Verified with a clean test-compile, since incremental reuses stale classes and hides such breaks. - .githooks/** now pinned to eol=lf. *.sh does not match an extensionless hook, so the force-push guard was LF by luck; a CRLF hook does not execute on Linux/macOS and would silently disarm itself. - Three `new Random()` on request paths in @ApplicationScoped beans → ThreadLocalRandom.current(). Docs - 38 broken links → zero. Every planning/*.md computed ../ and ../../ as if it lived under docs/planning/, so ../../AGENTS.md pointed outside the repo (32 of them); two were stale paths. The .gitbook/assets/ directory does not exist, which broke the onboarding tutorials specifically — those now import /openapi into Postman instead of a committed collection that can rot, and the broken lead diagram is a Mermaid rendering of the real pipeline. - SUMMARY.md was missing security-review.md and release-notes-6.0.2.md. - PR-template CONTRIBUTING links are absolute; no relative form is correct in both the rendered body and the file's blob view. Tests - McpToolFilterCoverageTest pinned both directions of the MCP allowlist but both started from a hand-maintained class list, so a new Mcp*Tools class would be invisible AND green. It now compares that list against the compiled classes on disk. Mutation-checked by dropping McpDocTools.
Writing the tests found a defect in the fix they were written for. flush() published its batch to inFlightBatch AFTER draining the queue, so between those two statements the entries were in neither collection — and an eviction landing in that window read their conversations as fully persisted and re-seeded them, reintroducing the duplicate through a narrower door. The drain and publish now happen under the read lock submitters take; no I/O is inside it. Every fix on this branch now has a regression test that was mutation-checked — the fix reverted, the test confirmed to fail: sequence eviction expected <[0, 1]> but was <[0, 0]> in-flight retention expected <1> but was <-1> eviction still reclaims guards a "fix" that never evicts rate-limiter overflow bucket denies every call after a long idle HUMAN_DECIDES message fails if it claims HUMAN members are unavailable MCP class-list drift names the Mcp*Tools class missing from the list Two guard a class of bug rather than one defect: - DocumentationLinksTest resolves every relative link in every markdown file, plus SUMMARY.md completeness. Link rot is invisible to every other check here. It immediately found one the manual sweep missed — the README banner is repository-root-relative, which a naive resolver sends to the filesystem root. The link was fine; the resolver was wrong. - ImportStyleTest enforces AGENTS.md 4.7. Writing it showed the original audit under-counted: its pattern required a segment after `java.util`, so java.util.List never matched. The real total was 575, not 141; the remaining 209 are cleaned up here. The one genuine disambiguation case is an explicit allowlist. Two seams widened deliberately: RateLimitBucket (a ~107-day idle bucket cannot be reached through the public API, and reflection would pin a field name), and SafeHttpClient.withDefaultTimeout (so its cases run without an embedded server, unlike SafeHttpClientTest which binds a loopback socket in @beforeeach). Every string literal across all 273 mechanically-changed files was compared against origin/main and is byte-identical, so the refactor never reached inside a reflective name, config key, or log format. The only six files whose literals changed are hand-edited ones.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
|
Important Review skippedToo many files! This PR contains 302 files, which is 202 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. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (302)
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 caught what the local run could not. `planning/agentic-improvements-plan.md` linked to `../security.md`, which on Windows and macOS resolves to the security POLICY at the repository root — while its own link text said `docs/security.md`, and that is plainly what it meant. On the Linux runner, and for every reader of the published docs, it was simply broken. The link is now `../docs/security.md`, but the more useful half of this commit is the test: DocumentationLinksTest compared existence only, so it inherited exactly the blind spot of the developer who wrote the link. It now also compares the on-disk casing via toRealPath, so a case-only mismatch fails identically on every platform and reports the real filename: ../security.md (case mismatch — the file on disk is 'SECURITY.md') Verified by reverting the link locally on Windows: the test now reproduces the CI failure rather than passing.
This was referenced Aug 12, 2026
Member
Author
|
Closing in favour of two smaller PRs, so automated review can actually run. At 302 files this exceeded both CodeRabbit's limit (>100) and Copilot's (>300), so neither reviewed it — and the part crowding them out was a 263-file mechanical refactor that does not need line-by-line reading.
Same commits, same green CI, reviewable. |
ginccc
added a commit
that referenced
this pull request
Aug 12, 2026
Four findings it raised in the review body rather than inline. Two were real bugs, one is a genuine constraint worth documenting, and one I disagree with on the evidence — recorded here rather than silently ignored. install.sh: `rm -f` cannot remove a DIRECTORY, and what is in the way is most likely exactly that — the stale mount path a previous failed run left behind. Under `set -euo pipefail` the failed rm becomes the script's exit point, so the user sees "rm: cannot remove ...: Is a directory" instead of the fail message explaining what to do, and the stale path survives to break the next run too. Reproduced before fixing; `rm -rf` restores the intended message. install.ps1 had the same shape: `Remove-Item -Force` does not remove a directory without -Recurse. Changelog placement violated AGENTS.md rule 8 — it must land on the branch carrying the work it documents. After the #675 split, the entries here still claimed the 575-name refactor and ImportStyleTest, which ship on chore/inline-fqn-cleanup. Those claims are removed; the branch no longer records work it did not carry. The sequence-table cap is a threshold, not a hard ceiling: the check in nextSequence is deliberately not atomic with the insert that follows, so concurrent submitters can overshoot slightly. Making it exact means serialising the insert path on every new conversation to enforce a bound that is a memory heuristic, not a correctness property. Said so where it is checked and where it is declared, instead of implying a guarantee that does not hold. On the fourth — that pinned undelivered conversations could leave every new conversation UNSEQUENCED until restart — the retention is real but that consequence is not reachable, and the suggested remedy is unsound. Re-seeding a conversation with a dead-lettered gap cannot work: countByConversation counts persisted rows, so with sequences 0-9 where 3 and 5 never landed the count is 8 while the next free position is 10, and it would hand out 8 and 9 again. Accounting for the highest undelivered position does not help either (max(8, 6) is still 8); a sound re-seed needs a maxSequence() the store interface does not expose. The pin is therefore correct, and bounded: undeliveredTracked counts SEQUENCES, so at most MAX_TRACKED_UNDELIVERED (10,000) conversations can be pinned against a 50,000 table, leaving 80% evictable. undeliveredPinCannotExhaustTheTable pins that headroom so raising one cap past the other fails the build rather than silently stranding conversations.
ginccc
added a commit
that referenced
this pull request
Aug 12, 2026
AGENTS.md rule 8 requires the changelog entry to land on the same branch as the work it documents. Splitting #675 left these claims on fix/review-defects, which does not carry the refactor; they were removed there and belong here.
ginccc
added a commit
that referenced
this pull request
Aug 13, 2026
…cs and hygiene (#677) * fix: audit ledger self-tamper defect, plus review fixes across CI and docs Split out of #675 so automated review can actually run — that PR carried a 263-file mechanical refactor alongside these changes, and both CodeRabbit (>100 files) and Copilot (>300) declined to review it. The refactor follows in a stacked PR. The one that matters: AuditLedgerService capped its sequence table at 50,000 conversations and cleared the whole thing on overflow, reasoning that re-seeding from countByConversation was "correct, only slower". It is not. Entries sit in the queue for up to a flush interval, so a conversation with queued entries has consumed positions the store cannot see yet, and the re-seed hands the same position out twice. The verifier grades a duplicate exactly like a gap (BROKEN) and, unlike a gap, `undelivered` cannot exculpate it — so the ledger accused the deployment of deleting audit records because its own bookkeeping wrapped around. On a busy deployment the queue is never empty, so essentially every overflow produced them. Eviction now drops only counters whose positions are all accounted for. A ReentrantReadWriteLock spans "position consumed" to "entry visible in the queue" against eviction, and flush() publishes its in-flight batch under the same lock — writing the tests showed that publishing after the drain left a window where entries were in neither collection. Also: - AgentGroupStore rejected HUMAN_DECIDES as needing "human group members (I6), which are not available yet", 150 lines below its own I6 matrix that validates them. They shipped in 10c; the missing piece is the resume path. - SafeHttpClient claimed a wall-clock timeout across all hops, but the budget is only checked between hops, so a hop that trickles its body hung forever. - ToolRateLimiter.refill() overflowed after ~107 idle days, latching the bucket shut. - ConversationStepRunner registered the in-flight conversation outside the try whose finally unregisters it. - install.sh/install.ps1 (the README's `curl | bash` path) were in no CI path filter and had no lint at all. New shell-lint job. - auto-approve treated an absent check as passing; a conflicted PR that never triggers CI would have been approved as "all checks passed". - 38 broken doc links, 32 from one cause (planning/ computing ../ as if under docs/). Dead code and a committed 1.4 MB grafana.db removed; an orphaned richer dashboard rescued into docs/monitoring/ and wired up. Every fix has a mutation-checked regression test, plus two class-of-bug guards: DocumentationLinksTest (which caught a link that only resolved because Windows is case-insensitive) and the MCP tool-class drift check. * test(docs): prune skipped directories instead of walking then filtering Addresses Copilot's review comment on #677. markdownFiles() used Files.walk(root) and filtered SKIPPED_DIRS afterwards, so it paid the full traversal cost of every directory it then discarded. That is not cosmetic here: target/ always exists when tests run and holds tens of thousands of class files, .git/ is comparably large, and this repository's own agent worktrees live under .claude/ — so the walk also recursed through nested checkouts of the repository itself. Files.walkFileTree with SKIP_SUBTREE prunes those roots instead. The test drops from ~1.9-4.4s to ~0.6s, and detection is unchanged: reverting a link fix still fails it with the file and target named. * fix: address CodeRabbit review — eviction throttle, installer safety, doc-test gaps All seven findings on #677, two of them real defects rather than polish. Eviction rescans could stall the submit path (Major). With every tracked conversation live, a scan evicts nothing — and the next submit for an unseen conversation would take the write lock and traverse the whole queue (bounded at 100k) to reach the same conclusion, with every other submitter blocked behind it on the read lock. Only a flush can change the answer, so a barrier keyed on a flush generation counter now bars the rescan until one happens. Mutation-checked: removing the barrier turns 1 futile scan into 55. Monitoring downloads failed soft, but every one of those files is bind-mounted as a FILE by docker-compose.monitoring.yml — so a missing one is worse than it sounds: Docker creates a *directory* at the mount path and Grafana then fails to provision, including the dashboards that did download. Now fatal in both installers, matching the reasoning already written for the Keycloak realm. DocumentationLinksTest had two holes: - normalize() collapses '..' but does not confine the result to the repo, so `../../thing.md` could land on a real file outside the checkout and pass — which is the exact mistake that put 32 planning/ links outside the repo. - The SUMMARY.md check used Files.list, which sees only direct children. Made recursive, and it immediately found docs/templates/baa-template.md unreachable from the table of contents. Now listed. Also: - Copilot: markdownFiles() walked target/, .git/ and .claude/ before filtering them. Pruned with SKIP_SUBTREE; the test drops from ~1.9-4.4s to ~0.6s. - scripts/preflight-local.ps1 was as unlinted as the installers were; the PowerShell parse and PSScriptAnalyzer steps now cover scripts/ too. - The OpenAPI import instructions hard-coded localhost, which only works when Postman runs on the EDDI host. - The changelog claimed every fix had a mutation-checked test. Several do not, for stated reasons; the claim is narrowed and the exceptions spelled out rather than left to imply more than was done. * fix(ci): PSScriptAnalyzer -Path takes one string, not an array The shell-lint job I added in this branch failed on its own first real run — which is the job working, just at my expense. Extending it to cover scripts/ passed `@('./install.ps1', './scripts')` to -Path, and that parameter is a single String: Cannot convert 'System.Object[]' to the type 'System.String' required by parameter 'Path'. Loop over the targets instead. Also skip Install-Module when the runner already ships PSScriptAnalyzer, which it does — installing over it only emits a "version is currently in use" warning. Both PowerShell steps were run locally under pwsh this time rather than reasoned about: the parse step passes for install.ps1 and scripts/preflight-local.ps1, and the analyzer reports 9 warnings and 0 errors, so the gate passes while still surfacing them in the log. * fix: address Copilot's suppressed review comments on #677 Four findings it raised in the review body rather than inline. Two were real bugs, one is a genuine constraint worth documenting, and one I disagree with on the evidence — recorded here rather than silently ignored. install.sh: `rm -f` cannot remove a DIRECTORY, and what is in the way is most likely exactly that — the stale mount path a previous failed run left behind. Under `set -euo pipefail` the failed rm becomes the script's exit point, so the user sees "rm: cannot remove ...: Is a directory" instead of the fail message explaining what to do, and the stale path survives to break the next run too. Reproduced before fixing; `rm -rf` restores the intended message. install.ps1 had the same shape: `Remove-Item -Force` does not remove a directory without -Recurse. Changelog placement violated AGENTS.md rule 8 — it must land on the branch carrying the work it documents. After the #675 split, the entries here still claimed the 575-name refactor and ImportStyleTest, which ship on chore/inline-fqn-cleanup. Those claims are removed; the branch no longer records work it did not carry. The sequence-table cap is a threshold, not a hard ceiling: the check in nextSequence is deliberately not atomic with the insert that follows, so concurrent submitters can overshoot slightly. Making it exact means serialising the insert path on every new conversation to enforce a bound that is a memory heuristic, not a correctness property. Said so where it is checked and where it is declared, instead of implying a guarantee that does not hold. On the fourth — that pinned undelivered conversations could leave every new conversation UNSEQUENCED until restart — the retention is real but that consequence is not reachable, and the suggested remedy is unsound. Re-seeding a conversation with a dead-lettered gap cannot work: countByConversation counts persisted rows, so with sequences 0-9 where 3 and 5 never landed the count is 8 while the next free position is 10, and it would hand out 8 and 9 again. Accounting for the highest undelivered position does not help either (max(8, 6) is still 8); a sound re-seed needs a maxSequence() the store interface does not expose. The pin is therefore correct, and bounded: undeliveredTracked counts SEQUENCES, so at most MAX_TRACKED_UNDELIVERED (10,000) conversations can be pinned against a 50,000 table, leaving 80% evictable. undeliveredPinCannotExhaustTheTable pins that headroom so raising one cap past the other fails the build rather than silently stranding conversations. * test(audit): prove the undelivered-pin invariant instead of asserting arithmetic github-code-quality flagged undeliveredPinCannotExhaustTheTable as a useless comparison, and it was right about the shape: both operands are compile-time constants, so `10_000 < 50_000` folds to `true` and the assertion executes nothing. It would still have failed had someone edited a constant — the test recompiles against the new value — but that is a weak guarantee to rest an invariant on, and it cannot be mutation-checked at all. Replaced with the behavioural test it suggested. A store outage dead-letters a conversation so position 0 is consumed but never persisted; the store then recovers and the table is filled and flushed. The test asserts the persisted conversations are reclaimed while the pinned one is retained, that a later conversation still receives a real chain position rather than UNSEQUENCED, and that the pinned chain resumes at 1 rather than reusing the dead-lettered 0. That last assertion is the one the arithmetic could never make. Mutation-checked by dropping `undelivered.keySet()` from the retain set: the counter survived, so the chain resumes past the dead-lettered 0 rather than reusing it ==> expected: <1> but was: <0> which is the duplicate this whole mechanism exists to prevent.
ginccc
added a commit
that referenced
this pull request
Aug 13, 2026
Mechanical follow-up to the review, stacked on fix/review-defects and split out of #675 so the substantive changes there could be reviewed at all — both CodeRabbit and Copilot decline a PR this size, and this is the part that does not need line-by-line reading. AGENTS.md 4.7 says to reference types by their simple name with a top-level import. These were not the permitted disambiguation case: PendingApprovalSummary, HitlDecision, ToolApprovalsConfig, ConversationMemorySnapshot and ControlSignal each resolve to exactly one class, and IConversationService imported java.util.List on line 17 while writing java.util.List<...> fully-qualified on line 355. The two genuine cases — mongo.HistorizedResourceStore extending datastore.HistorizedResourceStore, and its Modifiable twin — are detected and left alone. ImportStyleTest lands here rather than with the fixes, since it is what keeps this from re-accumulating: they build up precisely because nothing fails when one is added. Writing it showed the original audit had under-counted — its pattern required a package segment after `java.util`, so java.util.List never matched, making the real total 575 rather than 141. Verified beyond a green build: - Clean test-compile, not incremental, since a reused stale class hides exactly this kind of break in an unedited caller. - Every string literal in all changed files extracted and compared against origin/main: byte-identical. A rewrite that reached inside a literal — a reflective class name, a config key, a log format — would compile, pass every test, and show up nowhere else.
ginccc
added a commit
that referenced
this pull request
Aug 13, 2026
AGENTS.md rule 8 requires the changelog entry to land on the same branch as the work it documents. Splitting #675 left these claims on fix/review-defects, which does not carry the refactor; they were removed there and belong here.
ginccc
added a commit
that referenced
this pull request
Aug 13, 2026
* chore: replace 575 inline fully-qualified names with imports Mechanical follow-up to the review, stacked on fix/review-defects and split out of #675 so the substantive changes there could be reviewed at all — both CodeRabbit and Copilot decline a PR this size, and this is the part that does not need line-by-line reading. AGENTS.md 4.7 says to reference types by their simple name with a top-level import. These were not the permitted disambiguation case: PendingApprovalSummary, HitlDecision, ToolApprovalsConfig, ConversationMemorySnapshot and ControlSignal each resolve to exactly one class, and IConversationService imported java.util.List on line 17 while writing java.util.List<...> fully-qualified on line 355. The two genuine cases — mongo.HistorizedResourceStore extending datastore.HistorizedResourceStore, and its Modifiable twin — are detected and left alone. ImportStyleTest lands here rather than with the fixes, since it is what keeps this from re-accumulating: they build up precisely because nothing fails when one is added. Writing it showed the original audit had under-counted — its pattern required a package segment after `java.util`, so java.util.List never matched, making the real total 575 rather than 141. Verified beyond a green build: - Clean test-compile, not incremental, since a reused stale class hides exactly this kind of break in an unedited caller. - Every string literal in all changed files extracted and compared against origin/main: byte-identical. A rewrite that reached inside a literal — a reflective class name, a config key, a log format — would compile, pass every test, and show up nowhere else. * docs(changelog): record the FQN cleanup on the branch that carries it AGENTS.md rule 8 requires the changelog entry to land on the same branch as the work it documents. Splitting #675 left these claims on fix/review-defects, which does not carry the refactor; they were removed there and belong here.
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
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.
A full critical review of the repository, and the fixes for what it found.
Most of what the review looked for was not there — no swallowed exceptions, no non-thread-safe
statics, no mutable state in the singleton lifecycle tasks, zero
@Disabledtests across 14,000+.The findings are few, but one of them matters, and the pattern in the rest is worth naming: nearly
every problem sat in something no automated check covered.
The one that matters
The audit ledger could report itself as tampered under load.
AuditLedgerServicecaps its sequence table at 50,000 conversations and used toclear()the wholething on overflow, on the stated reasoning that re-seeding from
countByConversationwas "correct,only slower". It is not. Entries sit in the in-memory queue for up to a flush interval — longer
while a failing store is retried — so a conversation with queued entries has consumed chain
positions the store cannot see yet. Re-seeding from the store count hands the same position out
twice, and the verifier grades a duplicate exactly like a gap:
The
undeliveredtable exists so the ledger's own back-pressure cannot read as tampering, but itonly exculpates gaps; duplicates had no such channel. On a busy deployment the queue is never
empty, so essentially every overflow produced them →
ChainStatus.BROKEN, i.e. the ledger accusingthe deployment of deleting audit records because its own bookkeeping wrapped around.
Now only counters whose positions are all accounted for — persisted, or attributed in
undelivered— are evicted. A
ReentrantReadWriteLockspans "position consumed" → "entry visible in the queue"against eviction (that window contains HMAC and Ed25519 signing).
flush()publishes its in-flightbatch, because between the poll and a successful append those positions are in neither queue nor
store. Still-full-after-eviction yields
UNSEQUENCED, degrading the window toUNAVAILABLE("cannot be established") rather than to an accusation.
Also fixed
HUMAN_DECIDESblamed a feature that ships. The rejection read "needs human group members(I6), which are not available yet" — ~150 lines below the same file's I6 matrix that validates
those members. They shipped in 10c; the real blocker is the missing resume path for a paused
tie-break.
SafeHttpClientdocumented a guarantee it did not provide. It claimed an overall wall-clocktimeout across all hops, but the budget is only checked between hops, so a hop that trickles its
body hung forever. Both initial and redirect hops are now bounded.
elapsedNanos * limitoverflowed in longarithmetic after ~107 idle days, driving tokens negative so
tryAcquirerefused every later call.ConversationStepRunnerregistered the in-flight conversation outside thetrythat unregisters it.The gaps outside the build
install.sh/install.ps1had no verification of any kind — 95 KB of shipped script, theREADME's headline
curl | bashpath, in no path filter (so an installer-only PR skipped thewhole pipeline) with no lint, test, or syntax parse anywhere. New
shell-lintjob.CI/CD, so nothing failed and it would have approved with a body claiming all checks passed on a
commit that was never built. Required jobs are now checked by name.
planning/*.mdcomputed../and../../as if it lived under
docs/, so../../AGENTS.mdpointed outside the repository. The rest brokethe onboarding tutorials, which linked into a
.gitbook/assets/directory that does not exist.CannotExecuteException(no references) andILogoutEndpoint(a JAX-RS interfacewith no implementation, advertising
/user/isAuthenticatedto OpenAPI and serving nothing).grafana.dbwas committed, in agrafana-data/directory orphaned by the move to anamed Docker volume. Its
eddi-operations.jsonturned out to be a richer dashboard than theprovisioned one (21 panels, "Operations Command Center") being maintained while visible to nobody
— so it was rescued into
docs/monitoring/and wired up, rather than deleted with the rest.name resolved to exactly one class, and
IConversationServiceimportedjava.util.Liston oneline while writing it fully-qualified on another).
Verification
Every fix has a regression test that was mutation-checked — the fix reverted, the test confirmed
to fail with the message it would show whoever broke it. Writing them found a further defect in the
eviction fix itself:
flush()published its batch after draining the queue, leaving a window whereentries were in neither collection.
Two tests guard a class of bug rather than one defect:
DocumentationLinksTestresolves everyrelative link in every markdown file (link rot is invisible to every other check here — it caught one
the manual sweep missed), and
ImportStyleTestenforces §4.7 (writing it revealed the original audithad under-counted 141 vs the real 575, because its pattern required a segment after
java.util).The 575-name refactor was verified with a clean
test-compile— an incremental build reusesstale classes and hides exactly this kind of break — and by extracting every string literal from all
273 mechanically-changed files and confirming they are byte-identical to
origin/main, so nothingreached inside a reflective name, config key, or log format.
Local suite: 14,473 tests, and the only failures are this sandbox's inability to open loopback
sockets — identical in count and cause to the pre-change baseline. CI is the arbiter for those.