N ladder lane: per-book authorization in ledger, an unattached BoardModel that said stoull, and two coverage audits - #433
Merged
Merged
Conversation
The fifteen `_projectIdStr.has_value()` guards in `board_model.cpp` all carry
the message a client can act on -- "<Action>: handler was never attached via
OpenBoard", the shape `polls::PollModel` answers with -- and over a server not
one of them fires. Every action answers the bare string `stoull` instead: the
`what()` of the `std::invalid_argument` `std::stoull("")` throws, delivered as
though it were a domain error.
No in-process test had reached that state, because they all attach before
acting and the two writers of `_projectIdStr` only disagree when they do not.
These cases enter it deliberately -- `attachActionLog(log, {})` is character
for character the call `ModelFactory::create` makes on every newly constructed
holder when a process-wide default action log is installed, which kanban's
`App` does.
Red as committed, which is the point; the fix is the next commit. Nine of the
wire surface's actions report "stoull" here, and so does a handler that *had*
been opened and then had a log attached with an empty key.
Refs #368
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
`BoardModel::_projectIdStr` answers two questions that are not the same one:
which project this handler is attached to (written by `execute(OpenBoard)`),
and what entity key to stamp on journal entries (written by
`attachActionLog`). Every `execute()` overload asks the first as
`_projectIdStr.has_value()` before reaching `std::stoull(*_projectIdStr)`, and
the second writer answered it too.
`ModelFactory::create` attaches the process-wide default action log to every
newly constructed holder with an *empty* `entityKey` (`morph/core/model.hpp`),
and kanban's `App` installs such a log. So a registered-but-never-opened
handler had the optional engaged with an empty string: `has_value()` was true,
all fifteen guards fell through, and `std::stoull("")` threw
`std::invalid_argument`, whose `what()` -- the bare string `stoull` -- went on
the wire as though it were a domain error. It named neither the action nor the
mistake, and it leaked the id representation into the wire contract.
An empty key identifies no project, so it no longer becomes one. That is the
whole change: one place, and it restores the invariant the fifteen guards were
already written against -- engaged implies non-empty implies attached. Both
other writers are untouched. `Remote::attachLogIfConfigured` already declines
to attach on an empty `contextKey` (`morph/core/remote.hpp`), so the key it
passes is a real project id and still attaches; `OpenBoard` is unchanged. As a
second consequence, attaching a log no longer un-attaches a handler that
`OpenBoard` had already pointed at a board.
The scenario pin asserted the old replies per action, including that the two
guarded actions did *not* mention `OpenBoard`; it now asserts each action's own
message and that nothing on the surface says `stoull`. Verified failing against
the unfixed binary before the change, in-process and over a live server.
Closes #368
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
…-empty
Found by the high-effort review gate on the previous commit, then reproduced
over a live server before being fixed.
Declining an *empty* `entityKey` closed only half the hole. The other writer of
`_projectIdStr` is `Remote::attachLogIfConfigured`, which forwards the client's
`contextKey` verbatim off the wire, and kanban's `App` installs a `LogProvider`
for every model type -- so the key is arbitrary text. Registering as
`client fresh model=BoardModel contextKey=foo` made `_projectIdStr == "foo"`,
passed all fifteen attach guards, and reached `std::stoull("foo")`: the same
bare `stoull` on the wire that #368 is about, one step over from the empty key.
Worse, `stoull` stops at the first non-digit and reports success, so a key like
`7x` parsed as 7 and silently attached the handler to project 7 -- a board the
client never named. Per-action `requireRole` still gated access, so this was a
wrong-target and wrong-error bug rather than an authorization hole, but nothing
told the client which board it had got.
So the guard now asks the question the fifteen `stoull` sites actually need
answered: does this key parse, whole, as a project id? `std::from_chars` rather
than `stoull` in a `try`, because only the former rejects a trailing tail --
and, on an unsigned type, also rejects leading whitespace and a negative that
would otherwise wrap to a huge id. Every existing caller passes
`std::to_string(*projectId)` or nothing, so none is affected.
The header's stated invariant moves with it: engaged now implies *parses as a
project id*, which is what makes `has_value()` sufficient before the
dereference. It previously claimed non-emptiness was enough, which was not true
and which a later change would have leaned on. Its remaining honest caveat is
recorded there too: for a key this now discards, the holder still stamps the
raw `contextKey` on its own entries while this instance stamps `""`, because
one member still serves both purposes. Separating them is the larger change
#368's triage deferred.
The unit test now drives all fifteen guards rather than the nine the issue
lists -- `AddAttachment`, `GetAttachments`, `RemoveAttachment`, `CreateRule`,
`DeleteRule` and `ApplyTagMutation` were covered by neither the test nor the
scenario, so a regression re-opening the fall-through in one of those six would
have passed every file written to prevent it. Verified: 24 of its 28
assertions fail with the guard removed.
`*_actions.jsonl` joins `.gitignore`. Every rung's server writes its
`FileActionLog` into the *current directory*
(`std::filesystem::current_path() / "<rung>_actions.jsonl"`, see each
`src/server/main.cpp`), so running one from the repo root drops a journal
there -- and one was committed by accident from a manual run while this branch
was being verified.
Refs #368
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
`RunReportJob` refuses every principal but `kReportRunnerPrincipal`, so a client can submit a report and poll it but never advance it; `UndoTransaction` takes a `journalId` that no action in the rung's wire surface returns, so the only outcome a client can reach is the not-found refusal. Both facts are already pinned -- by `test_ledger_reports.cpp`'s "RunReportJob refuses any principal but the report runner's", by the `submit-a-report-and-poll-it` and `undo-needs-a-journal-id-nothing-hands-out` scenarios, and by `coverage_allowlist.json`'s two entries -- but nothing a client author reads said so. The README now does, beside the `CreateLedger` bootstrap note that describes the same surface. Documentation only: no behaviour changes. Whether `UndoTransaction`'s missing id-return is closed by a new action or recorded as deliberate stays open. Closes #362 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
…there The rule's comment said a journal "was committed by accident before this line existed". Nothing matching `*.jsonl` appears anywhere in this repository's history -- `git log --all --diff-filter=AD -- '*.jsonl'` is empty and `git ls-files | grep jsonl` finds nothing -- so the sentence sends a reader hunting for a commit that does not exist. The near miss it describes was on a working branch. The rule itself is unchanged and still worth keeping. Refs #368 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
Any principal holding a valid token could read, write and post into any book, including one another principal had just created over the wire with `CreateLedger`. The signed-token check and the per-action empty-principal gate both held; neither says whose book this is, and `GetLedger` had no principal check at all. `ledgers` grows a nullable `owner` column (migration 20260819000015). `CreateLedger` stamps its caller, and every action that reaches a book now compares that owner against `Context::principal` before doing anything -- `OpenAccount`, `StoreTransaction`, `UndoTransaction`, `ImportLedgerChunk`, `SubmitReport`, `SetCategory`, the four `BudgetModel` mutations and both `RuleModel` ones, plus the three reads that were wide open (`GetLedger`, `GetBudgetReport`, `GetReportStatus`). An action naming a child row gates on the book that row belongs to. `RunReportJob` keeps its own, stricter gate: the server's report runner is the only principal it admits. The rule lives in the models, through the relation (`ledger/db/book_access.hpp`), per `examples/IMPLEMENTATION.md` rule 4 and the `bank::db::loadOwned` shape rung 1 established -- not at the authorizer. `authorizeInstance` compares one register-time owner against the caller, and `LedgerModel`'s instances are keyed by `ledgerId` and shared across every client that opens the same book, so it has no single owning caller to compare against. That is the constraint the issue records, and it is why the naive fix does not fit. Backfill: SQLite cannot add a NOT NULL column to a table that may already hold rows and there is no principal to attribute existing books to, so NULL means "created before ownership existed" and such a book stays shared, exactly as it was -- the same reading `params_json` already has on a report job. Nothing writes a new one. The scenario corpus's fixture books are seeded by raw INSERT and are unowned for this reason, which is why the whole corpus still passes. Verified: `test_ledger_book_ownership.cpp`'s cross-principal refusals all fail against the models as they were (12 failed assertions, Bob reading and writing Alice's book); 143/143 ledger tests and 2037/2037 ctest pass with the fix; the ledger scenario corpus passes twice against one database over the wire; and the issue's own two-principal reproduction, re-run against a live `ladder_ledger_server`, now refuses Bob on all three actions while leaving Alice's book and Bob's own book working. Closes #382 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
Review of the ownership gate found two ways past it, both because the gate was placed after work that already discloses the book. `StoreTransaction`'s exactly-once path answers a repeated `opId` with the stored `GetLedgerResult` -- every account and balance in the book -- and returns before the gate was reached. Guess an opId and a non-owner reads the whole book, which is exactly what the new `GetLedger` gate withholds. The account lookups just below were the same hazard in weaker form, a "does account N belong to book B" oracle. The gate now runs immediately after `validate()`, before either. `setCategoryImpl` gated the account's book but not the category's, so a caller could file its own account under another principal's category -- `GetBudgetReport` selects legs by exactly that link, so every entry posted against the account would then be summed into the other principal's budget report. It now checks both rows, the way `LinkAccountToCategory` already did. Also corrects `LinkAccountToCategory`'s comment, which claimed more than the two checks deliver: they refuse a link across an ownership boundary, not a link across two books the same principal owns. Verified: with these three source files reverted to the previous commit and everything else kept, `ladder_ledger_tests "[ownership]"` fails exactly the two new cases (7 cases, 2 failed -- "Replaying another principal's opId does not hand back its book" and "SetCategory refuses a category in another principal's book"); with them restored, 145/145 ledger cases and 162/162 crm cases pass. Refs #382 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
`namesAProject` accepted any string `std::from_chars` consumed whole, which
includes "007" for project 7. `_projectIdStr` has a second reader:
`logAction`/`logFailure` stamp it verbatim as each entry's `entityKey`, and
`execute(GetActivity)` reads entries back by that exact string. A client
registering with `contextKey=007` therefore passed every attach guard and
every `requireRole`, worked correctly against project 7, and journaled under a
key no other client -- and no `GetActivity` -- ever asks for, silently
splitting one board's activity stream in two.
Comparing the key against `std::to_string(value)` closes it, and makes the
header's stated contract ("the complete decimal spelling of one") literally
true rather than approximately true.
Verified: with `board_model.cpp` reverted and the new case kept,
`ladder_kanban_tests "[unattached]"` fails exactly that one case (9 cases, 1
failed); with the fix, 9/9 pass.
Refs #368
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
… describe
Three line citations in `examples/kanban/README.md` had drifted off the code
they name. Verified by reading each cited line, not by assuming an offset:
:849 -> `ApplyTagMutationResult BoardModel::execute(const ApplyTagMutation&)`
:858 -> a doc-comment line about ApplyTagMutation's RBAC
:835 -> a closing brace
They now cite, on this branch:
:966 -> `requireColumnBelongsToProject(mapper.Get(), project, action.columnId);`
:998 -> `throw NotFound{"swimlane does not belong to this project"};`
:984 -> `throw Conflict{"MoveTaskPosition: target column is at its WIP limit"};`
Each citation now also names what is on the line, so the next drift leaves a
reader something to search for. Nothing in CI checks a line number:
`scripts/check_spec_citations.sh` verifies cited paths and section headings
only, which is why this rotted silently.
Every other line citation in that README was checked the same way, not just the
three named: `MembersView.qml:70`/`:101` and `RulesView.qml:105`/`:113` are all
`ComboBox {` as the rule-2 table claims, and the six `morph::offline` /
`observability.hpp` citations all land on what the prose describes.
The sweep the issue asks about was done, across every `examples/*/README.md`.
Only two rung READMEs carry line citations at all. kanban's three were wrong;
pastebin's one was too -- `include/morph/core/model.hpp:145` for
`IModelHolder::recordIfAttached`, which is a doc-comment line about
`attachActionLog`; that method is defined at `:221`, and the citation is
corrected here. No other rung README cites a line number.
Closes #421
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
`ContactModel::attachActionLog` and `SavedViewModel::attachActionLog` were called by no test at all, while `AccountModel`, `LeadModel`, `OpportunityModel` and `QuoteModel` each carry an explicit "journals its edits against the attached identity" case. Both models' journaling paths were therefore claimed by their headers and executed by nothing -- the state lims was in before its own audit trail turned out to be recording verifications under an empty entity key. Each model now has that case. Both read entries back through `log->entries(<key>)` rather than through the model, which is what makes the attached key load-bearing: an entry stamped with a different key, or with none, does not come back from that call. `SavedViewModel`'s covers the delete as well as the create, because a saved view is the one crm entity whose row is removed rather than versioned, so its journal entry is the only surviving record that it existed. A second case pins that `ListSavedViews` and `RunSavedView`, both `Loggable::No`, record nothing. No defect found on either path: both models journal both of their mutations, under the attached key, with the caller's principal and a stamped payload schema. Verified: with the two `attachActionLog` calls removed and everything else kept, both cases fail (2 cases, 2 failed); with them, the crm suite is 165 cases / 445 assertions, all passing, of which the `[audit]` subset is 25 cases. This is morph#412's first clause. The component-coverage clause -- crm to 89% and the per-miss audit of its uncovered lines -- is not in this commit. Refs #412 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
…as counting `codecov.yml` scores `examples/common` at 90.39% against a 95% target derived from 95.85%, and morph#411 reads that as a five-point regression with real code arriving untested. Both halves needed establishing before anything was written, because the same tree also measured 96.06% locally. Both figures were right. `llvm-cov report` counts a line that executed as covered; Codecov scores `hits / (hits + misses + partials)` over the uploaded LCOV and calls a line with a one-sided branch a partial. On the same profile the component read 95.22% one way and 90.79% the other -- 52 partials apart, not a contradiction and not the compiler-cache contamination that made an earlier local measurement unusable (morph#426, fixed on master and gated by scripts/check_coverage_roots.sh, which reports 610 files all under the checkout here). Splitting the component by whether a file existed at the 95.85% measurement (morph#142) answers what the ticket asked and could not: the code measured then still measures 95.78%. Nothing regressed. `qml_surface.cpp`, `process_pool.hpp`, `journey.hpp` and `step_executor.hpp` all arrived after it, are 594 of today's 1281 lines, and measure 85.02% between them -- `qml_surface.cpp` alone carrying 70 of the 118 non-hits. So the tests here go to the largest of those, and to arms that are promises rather than lines. `QmlSurfaceAudit`'s own header lists the drift directions it covers; eleven of them were held up by inspection only, including the signal-called-as-an-invokable diagnostic, the whole `onX:` property-handler syntax, single-quoted and template-literal blanking, `addDirectory()`, and the "declares no QML-visible members at all" vacuity guard. `event_poller.hpp` documents an answer for a null `exception_ptr` and for a non-`std::exception` throw; `handleError` produces neither, so both were unexecuted, and both helpers are free functions that can simply be called. Verified by mutation, one change at a time, rebuilding between each: deleting the quote-state arms, the longer-alias guard, the property-handler loop, the signal-vs-invokable branch, the read-resolves-to-a-method branch, the no-visible-members guard, and the body of `addDirectory()` each fails at least one new case; so does each of the three ways to get a classifier's null or non-std answer wrong. Two mutations were *not* killed and are recorded as unreachable rather than papered over: `signalNameOf`'s shape guards, which neither caller can reach because both patterns match `on[A-Z]\w*`, and the no-`target:` skip, whose removal is equivalent. examples/TESTING.md gains the audit itself -- the two units and how to reproduce each, the pre/post-#142 split, and every remaining uncovered line classified as measurement artifact, unreachable-by-design, untested, or dead API. `codecov.yml`'s seven named artifact lines are still correct and are no longer the whole set: `step_executor.hpp:89` and `qml_surface.cpp:599` are two more of the same closing-brace shape, and `backend_rig.hpp`'s `-Wswitch-default` pair plus four `qml_surface.cpp` guards are unreachable through their callers. `ProcessPool::killAll` and `ClientProcess::waitForFinished` have no caller anywhere in the tree. examples/common now reads 96.94% by llvm-cov and 93.68% by Codecov's arithmetic, from 95.22% and 90.79%. The remaining gap is named line by line rather than closed by tests written to close it. Verified: ladder_common_tests is 153 cases / 587 assertions, all passing, of which 13 cases are new here (11 in test_qml_surface.cpp, 2 in test_event_poller.cpp); the full coverage ctest run is 2468 tests, 100% passing; scripts/check_coverage_roots.sh, check_coverage_objects.sh, check_rung_filters.sh, check_spec_citations.sh, check_test_type_names.sh, check_journal_stamps.sh and check_automoc_includes.sh (against build/clang-coverage) all pass. Closes #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
…none
morph#412's second clause is the per-miss audit `codecov.yml` says crm does not
carry: "does NOT yet carry a per-miss audit establishing which of the 148
uncovered lines are unreachable-by-design rather than merely untested". Ledger's
and lims's entries each end by naming two or three defensive guards no caller
can reach. crm's answer is not that shape, and the difference is the finding.
Of the 141 uncovered lines this measured, four are unreachable and 92 are the
rung's own rejection contract -- every `throw ValidationError{"<Action>: X is
required"}` and every `throw NotFound{"<Action>: no such Y"}` across the eight
model translation units, on twenty-five actions, executed by nothing. Those are
the lines a malformed or stale client reaches first. Writing them down as a
ceiling would have been wrong; so would generating one "it threw" case per
guard, which pins the exception type and not the thing that matters, that the
two refusals stay distinguishable for a caller deciding whether to retry.
The remaining 39 are enumerated individually in examples/crm/README.md, and one
of them was not a branch: `QuoteModel::execute(const GetQuote&)` is registered
on the wire by `BRIDGE_REGISTER_ACTION` and driven by no test, no presenter and
no scenario -- the whole action, all thirteen lines. `ListQuotes` runs the same
`toView`/`fetchLines` pair in aggregate, so a `GetQuote` that fetched the wrong
row or answered an unknown id with a default-constructed `QuoteView` would have
looked identical to a working one from anywhere else in the suite. It now has
two cases, and they are the per-model shape the audit argues for rather than a
line-count exercise.
Verified by mutation, rebuilding between each: making `GetQuote` return an empty
view instead of refusing, ignore its id and return the first row, drop its
`validate()` guard, or return no lines each fails one of them. The
`validate()`-guard mutation does not merely change a message -- it aborts the
process on an unset `QuoteId`, which is recorded.
The audit also finds two guards that *are* unreachable, and says why:
`alreadyDecided`'s empty-key check, unreachable because
`QueuedOpportunityUpdate::validate()` requires a non-empty `operationKey` and
`execute` runs it first -- the identical guard, for the identical reason, that
lims's audit records and that this function's own doc comment already cites as
its model -- and `MoveOpportunityStage`'s corrupt-ledger-entry error. A third
class is not a coverage problem at all: `journalEntries()` is declared on all
seven models and called from one place in the rung, so six of them have no
caller in tests, presenters, app code or GUI.
Two claims in `codecov.yml` are retired by this and by 0d2fc0b, and the
replacement text is on the ticket: `ContactModel::attachActionLog` and
`SavedViewModel::attachActionLog` are no longer called by no test, those models'
journaling paths are no longer unverified, and no defect was found on either --
a negative result against the lims precedent the same paragraph invokes, worth
recording rather than leaving implied as open. `codecov.yml` is not this lane's
to edit.
Measured, all on the same clean profile (scripts/check_coverage_roots.sh: 610
files, all under the checkout): the crm component moves from 86.33% to 87.15% by
Codecov's arithmetic and from 91.27% to 92.08% by llvm-cov's line count, with
128 uncovered lines left, each of them classified above. The percentage clause
of morph#412 is superseded by the mutation reframe; the audit and the
journaling clause are what this closes.
Verified: ladder_crm_tests is 168 cases / 463 assertions, all passing, of which
2 are new here; the full coverage ctest run is 2470 tests, 100% passing;
check_coverage_roots.sh, check_coverage_objects.sh, check_rung_filters.sh,
check_spec_citations.sh, check_test_type_names.sh, check_journal_stamps.sh and
check_automoc_includes.sh (against build/clang-coverage) all pass.
Closes #412
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
The crm component's comment carried three things that are no longer true after morph#403 and morph#426: it lacked the per-miss audit (now written, in examples/crm/README.md), it implied ContactModel's and SavedViewModel's journaling paths were an open finding (they were tested and no defect was found), and its figures predated a profile with zero foreign roots. Also records the arithmetic, because two correct measurements disagreed for a whole round over it: llvm-cov counts an executed line as covered, Codecov counts hits/(hits+misses+partials) and scores a one-sided branch as a partial. This target is checked against the second. Serialised by the manager rather than taken in a lane: codecov.yml is a repo-root file every lane collides in, and the measurements it records are N ladder's. Refs #412 Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
The clang-format gate found six violations in test_event_poller.cpp and test_qml_surface.cpp, both added by #411. Mechanical reformat only; no assertion, name or behaviour changed. Refs #411 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Sprint round 1–2, lane
N ladder— the lane's whole queue in one PR so CI runsonce for it rather than seven times.
Closes #368
Closes #362
Closes #382
Closes #421
Closes #411
Closes #412
The security fix
#382 — any authenticated principal could read and write any book.
ledgersgrows a nullable
owner;CreateLedgerstamps its caller; every action reachinga book compares that owner against
Context::principalfirst — including thethree reads that were wide open (
GetLedger,GetBudgetReport,GetReportStatus). The rule lives in the models through the relation(
ledger/db/book_access.hpp), not atauthorizeInstance, which cannot express itfor instances keyed by
ledgerIdand shared across clients — the constraint theissue records, and it held.
NULL owner means "created before ownership existed" and stays shared; nothing
writes a new NULL.
Reproduced before fixing: with the model
.cppfiles reverted and the new testkept, 3 of 5 cases failed, 12 assertions — Bob read and wrote Alice's book
exactly as reported. The
/code-review highpass then found two real holes inthe fix itself, both closed with their own prove-it-fails runs: the ownership
check sat after
StoreTransaction's opId replay early-return (guess an opId,read the whole book), and
setCategoryImplgated the account's book but not thecategory's.
Verified: 145 ledger cases / 724 assertions; 2376/2376 full
ctest; thescenario corpus green twice against one database; and the issue's own
two-principal reproduction re-run over a live
ladder_ledger_server— 17 steps,23 assertions, Bob refused on
GetLedger/OpenAccount/SubmitReport, Alice'sbook intact.
The correction that matters most
#411 was filed on a false premise, and the work found it. I wrote that
examples/commonhad "fallen 5.46 points below the figure its target was derivedfrom". Nothing regressed.
The two figures were never in conflict — they are different arithmetic.
llvm-cov reportcounts an executed line as covered; Codecov countshits / (hits + misses + partials)and scores a one-sided branch as a partial.That is the entire 96.06% vs 90.39% gap, and the same arithmetic reproduces
codecov.yml's own recorded framework figure to within 0.01.And the drop is a denominator change: splitting the component by whether a file
existed at the 95.85% measurement gives 95.78% for the code measured then and
85.02% for the four files added since — 594 of 1281 lines,
qml_surface.cppalone carrying 70 of 118 non-hits. The code that was at 95.85% is still there.
examples/common, llvm-cov linesexamples/common, CodecovWhat was written, and how it was proven
#411 — 13 cases, each justified from a promise the code makes and verified
by mutating that code, rebuilding between each: 7 mutations in
qml_surface.cpp, 3 inevent_poller.cpp, all killed. Two mutations were notkilled and are recorded as unreachable rather than papered over.
#412 — the per-miss audit
codecov.ymladmitted was missing, inexamples/crm/README.md. All 141 lines classified: 52validate()rejections,40
NotFoundrejections, 39 other branches enumerated individually, 6 in ajournalEntries()with no caller, 4 unreachable by design.crm's answer turns out to be unlike ledger's and lims's: 92 of 141 lines are
the rung's rejection contract across 25 actions — reachable from the wire by any
malformed client, executed by nothing. That is not a ceiling and the config now
says so.
One of the 39 was not a branch at all:
QuoteModel::execute(const GetQuote&),a
BRIDGE_REGISTER_ACTIONed wire action, all 13 lines, driven by no test, nopresenter and no scenario. It has two cases now; four mutations kill them.
Dropping its
validate()guard does not change a message — it aborts theprocess on an unset
QuoteId.A negative result, recorded as one: no defect on
ContactModel's orSavedViewModel's journaling paths, contra the lims precedent that motivated thecheck. Both journal both mutations under the attached key with the caller's
principal.
Also in it
#368 — an unattached
BoardModelsaidstoull; the two guards written toprevent that now fire. Review found a second hole:
attachLogIfConfiguredforwarded the client's
contextKeyverbatim andstoull("7x")succeeds as7,silently attaching a handler to a board the client never named.
#362 —
RunReportJobandUndoTransactionrecorded as client-undrivable inthe rung README. The
UndoTransactionhalf needs a decision and is split out as#428.
#421 — all three
board_model.cppcitations in kanban's README were wrong;:849was a tag guard and:858a closing brace. Everyexamples/*/README.mdwas swept: only two carry line numbers, and pastebin's was wrong too.
Manager note
The
codecov.ymlcommit is mine, not the lane's. It is a repo-root file everylane collides in, so it is serialised rather than taken in a lane; the
measurements it records are this lane's.
/simplifydid not run at any phase. It forks into the primary checkout andedits with
--fix. Stating that rather than letting silence read as "foundnothing". Both
/code-reviewpasses did run, scoped by branch name.Full coverage
ctest: 2470 tests, 100% passing.🤖 Generated with Claude Code
https://claude.ai/code/session_0154xzWuBMPveLcdeUgydifb