chore(agents): the roles describe the workspace they are in, and money-safety gains a ninth - #214
Merged
Merged
Conversation
…y-safety gains a ninth #182 moved five role files in from ~/.claude so the standards would travel with the clone. It was a copy rather than a review: they were last written when there were four providers and no .claude/skills/, and three of them still sent a contributor to a path only one machine has — in the commit whose stated purpose was closing that gap. Counts. kasapay-provider said "Four exist and each solved a different problem — read the closest one before writing anything" and listed four; kasapay-paypal landed in #113, five days before that file was committed. So an agent asked to add an order-then-capture operation read "four" and never opened the one adapter that had already solved that shape. kasapay-core said four adapters. kasapay-specs listed three fetchers of five and none of the analysis scripts, which is a fair part of why coverage.py has no PayPal section while its own docstring counts one. One fact with two answers, inside the role files. Five said to check `gh pr checks` a couple of times and then write the report; kasapay-verify said to poll with a shell loop and report at the end. The outlier is the role that also holds live credentials, and it was missing the two rules it needs most — account for every removed line after rewriting a branch, and one worktree each — because a verification run rebuilds a branch repeatedly as entries move out of UNVERIFIED.md. kasapay-review is read-only now, and registered without the tools to write. agent-briefs is unambiguous about why: an auditor that can fix what it finds does, and a fixed finding stops being a finding — the report gets shorter than the thing it audited and nobody can tell whether that is because the code was good. It had one line saying not to edit under sixty-seven about how to commit. kasapay-specs owns .github/workflows/ and was never told what is in there: every `uses:` pinned to a commit, the two actions that read their own ref so pinning means passing `toolchain:`/`tool:`, which workflow holds the crates.io token, and that anything pip installs into a writable job is the same mutable pointer an action tag is. It is also told that compare_specs.py pairs by what latest.yaml names rather than by path, and not to undo that. New role, kasapay-release. Everything in #197 — a guard that could not fire, a push that was not atomic, a changelog check that passed on an empty section, and a partial publish nothing described — lived in files two roles half-owned and neither was accountable for. It leads with the mistake it owns: 0.0.3 went to crates.io with no GitHub release, because a tag pushed with a workflow's own GITHUB_TOKEN starts no workflow. money-safety gains a ninth class: a value that must not be logged reaching a log or a URL. Its own rule says to write a rule down when the class appears twice. That happened at #111 and nobody wrote it, so #177 found it again four days later in a worse sink, and #183 found it a fourth time from the other direction — an unauthenticated stranger's string reaching a URL path. §5 also gains the sentence that makes its own remedy safe: reading the provider's list of refunds only works if the list is complete, which #144 and #187 are both about. And §3 now names the instance this workspace shipped, fixed in #200. sandbox-verification gains a step 0: confirm the entry still describes the code before spending a call on it. UNVERIFIED.md's D1 named Provider::cancel as Mollie's DELETE for two changes after it stopped being that, so a run following the file as written would have closed the register on the wrong function. CLAUDE.md said eight ways, two shipped, six roles. Nine, four and seven. Closes #199 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…write Left over from when it could: not compiling "before pushing", the staging and branch-rewriting rules, and the count money-safety no longer has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 1 minute Limit details: You’ve used all 10 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Comment |
productdevbook
added a commit
that referenced
this pull request
Aug 20, 2026
#214 added the ninth class and named the fourth shipped defect, and left the frontmatter saying eight and two — which is what a reader deciding whether to open the file sees, and the only part of it some tools ever read. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 20, 2026
productdevbook
added a commit
that referenced
this pull request
Aug 20, 2026
…cause it was measured (#221) All from an adversarial review of the range that landed today, most of them written by the change that was correcting something else. Corrections carry a higher defect rate than original writing, because a small local edit gets less scrutiny than new code — every long-standing count in this repository verified, and the three wrong ones were written yesterday. money-safety §7 said "answer 200 anyway" for an unsigned delivery, flatly. Every Mollie delivery is unsigned; that is the premise of its whole webhook path. So an agent told to read this skill before writing a handler got the rule #207 had just spent a commit removing. It now carries the exception, which is that a `verify` reaching the network has an `Err` meaning two different things. §3's count was wrong in both halves: nine status fallbacks across five adapters, not eight, and PayTR's ninth refuses an undocumented status outright rather than landing on `Pending` — which is the best of the three answers and the sentence excluded it. `scripts/requirements.txt` said the two named packages pull nineteen; twenty lines, so eighteen. `coverage.py`'s docstring said four provider sections report; three do, and PayPal has none at all — which is the thing #214's own commit message noticed and then fixed in the role file rather than in the script. Two doc comments still said `fraudStatus` is read "the way this crate has always read it", in the change that altered how it is read. The CHANGELOG entry for that named `Provider::charge` and `reporting` as the paths it reached and not `Provider::capture`, whose `/payment/postauth` answer carries the same field. The CHANGELOG for the amount fallback called `price` "the correct figure". It is the basket total; `paidPrice` is what was collected, and they differ by an instalment surcharge this crate models itself. #220 is where that is argued rather than assumed, because changing it means changing #172's decision. `kasapay-review.md` said the role "is registered without the tools to" write. `Edit` and `Write` are absent; `Bash` is not, and `git commit` is reachable through it. Stated as a fence it was not. UNVERIFIED.md D5 records what `Webhook::verify`'s character class assumes about Mollie identifiers. Every comparable reading landed this week got an entry and this one did not, and it is on the delivery path. And the seam detector stays narrow. The review asked it to skip a bare `///` between two blocks, since rustdoc takes the first paragraph either way. Widened that far it matches **380** doc comments, because a summary ending in a full stop, a blank `///`, then a paragraph is the *correct* shape — the rule would have flagged nearly every well-formed comment in the workspace. Fifty means the rule is wrong somewhere; 380 means it is very wrong. The limit is recorded with the number that decided it, so nobody widens it again. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes #199.
#182 moved five role files in from
~/.claudeso "the standards for taking real money travel with the repository". It was a copy rather than a review: they were last written when there were four providers and no.claude/skills/, and three of them still sent a contributor to a path only one machine has — in the commit whose stated purpose was closing exactly that gap.The counts, and what they cost
kasapay-providersaid "Four exist and each solved a different problem — read the closest one before writing anything" and listed four.kasapay-paypallanded in #113, five days before that file was committed. So an agent asked to add an order-then-capture operation read "four" and never opened the one adapter that had already solved that shape.kasapay-coresaid four adapters.kasapay-specslisted three fetchers of five and none of the analysis scripts — which is a fair part of whycoverage.pyhas no PayPal section while its own docstring counts one.One fact with two answers, inside the role files
Five said to check
gh pr checksa couple of times and then write the report.kasapay-verifysaid to poll with a shell loop and report at the end. The outlier is the role that also holds live credentials — and it was missing the two rules it needs most: account for every removed line after rewriting a branch, and one worktree each. A verification run rebuilds a branch repeatedly as entries move out ofUNVERIFIED.md, which is precisely whengit reset --mixedquietly reverts somebody else's paragraph.The auditor is read-only now
Registered without the tools to edit.
agent-briefsis unambiguous about why: an auditor that cannot edit will not be tempted to fix what it finds — which is what keeps its findings honest. A fixed finding stops being a finding; the report gets shorter than the thing it audited and nobody can tell whether that is because the code was good.It had one line saying not to edit, sitting under sixty-seven about how to commit. Its standing rules now match a role that cannot write: no cargo at all (there is nothing to format), no checkout, read a revision with
git show.kasapay-specsowns the workflows and was never told what is in themEvery
uses:pinned to a commit; the two actions that read their own ref, so pinning means passingtoolchain:/tool:; which workflow holds the crates.io token; and that anythingpipinstalls into a writable job is the same mutable pointer an action tag is. It is also told thatcompare_specs.pypairs by whatlatest.yamlnames rather than by path — and not to undo that.New role:
kasapay-releaseEverything in #197 — a guard that could not fire, a push that was not atomic, a changelog check that passed on an empty section, a partial publish nothing described — lived in files two roles half-owned and neither was accountable for.
It leads with the mistake it owns: 0.0.3 went to crates.io with no GitHub release, because a tag pushed with a workflow's own
GITHUB_TOKENstarts no workflow.money-safety gains a ninth class
A value that must not be logged reaching a log or a URL. The skill's own rule is to write a rule down when the class appears twice. That happened at #111 and nobody wrote it — so #177 found it again four days later in a worse sink, and #183 found it a fourth time from the other direction: an unauthenticated stranger's string reaching a URL path.
§5 also gains the sentence that makes its own remedy safe: reading the provider's list of refunds only works if the list is complete, which #144 and #187 are both about. And §3 now names the instance this workspace shipped, fixed in #200 — its example used to be character-for-character the live code, which nobody noticed.
sandbox-verification gains a step 0
Confirm the entry still describes the code, before a credential is touched.
UNVERIFIED.md's D1 namedProvider::cancelas Mollie'sDELETEfor two changes after it stopped being that — a run following the file as written would have closed the register on the wrong function.CLAUDE.md
Said eight ways, two shipped, six roles. Nine, four and seven.