chore: add the integrity routes to the OpenAPI snapshot #123
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/openapi-snapshot-integrity-routes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What's in
#121 added
MapIntegrityEndpointsand merged before #119, so the snapshot #119 committed does not coverGET /api/integrity. The reflection guard noticed and failed naming the file:That is the gate working, not a defect in either PR — and it is the first real exercise of it. An endpoint landed, and the wire change is now a reviewable diff instead of a silent one. Without the guard,
backend/openapi/v1.jsonwould simply have gone quietly stale on its second day.Two changes, both exactly what the failure message prescribes:
MapIntegrityEndpointsadded toMappedEndpointFiles, inProgram.csorder (last, afterMapStatsEndpoints).UPDATE_OPENAPI_SNAPSHOT=1. +160 lines: the/api/integritypath plus theIntegrityReportDto/IntegrityCheckDto/IntegrityFindingDtoschemas and the three enums (IntegrityCheckKind,IntegritySeverity,IntegritySubjectType).No production code, no test logic — the diff is the wire contract catching up with a merged endpoint.
Verification
scripts/verify.sh backend, exit 0: ApplicationCore 33, IntegrationTests 1, Infrastructure 105, RestAdapter 542.Also confirmed while I was here, since #118 and #120 both added an EF migration and #120 merged first:
AddScaleIndexesandAddEntitySearchKeyare both onmainandModel_And_Migrations_Agreepasses, so the migration collision I flagged on both PRs resolved cleanly and needed no regeneration.Follow-ups
Two things this exposes about the ordering rule, worth having in mind rather than in code:
docs/DEVELOPMENT.md#adding-a-backend-endpointshould name the regeneration step. Not doing that here; it belongs with whoever next adds an endpoint./api/integrityoperation inherits the document's existing gaps — nooperationId, nosummary, no security scheme. Those are already tracked from #119 and are not specific to this route.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The baby gate caught its first real prey~ ♡ #121 slipped an endpoint onto
mainbefore #119's snapshot landed, the reflection guard barked, and this PR is exactly the two things the failure message prescribed — nothing more, nothing less. That's what a contract gate is FOR. I'm delighted~Verdict: ✅ Looks good to me~
What I verified (not just read — exercised):
Program.csorder:MapIntegrityEndpointslast atProgram.cs:182, right afterMapStatsEndpoints— matching the array position exactly. SignatureMapIntegrityEndpoints(this IEndpointRouteBuilder app)is identical in shape to every sibling (Stats/Backup/Health), so the reflection discovery inDiscoverMapMethodspicks it up cleanly.v1.jsonis purely additive: the/api/integritypath, the six schemas (IntegrityReportDto,IntegrityCheckDto,IntegrityFindingDto+ the three enums), theResourceResponseOfIntegrityReportDtoenvelope, and theIntegritytag appended last. Valid JSON, alphabetized schemas, no drift anywhere else in the file.Snapshot_Maps_Every_Endpoint_Filewent RED namingMapIntegrityEndpoints(the exactmainfailure), restored → green. Directional, not a tautology~f97bdff):OpenApiSnapshotTests4/4 green — which proves the committed snapshot is byte-identical to what the server actually serves, not merely plausible-looking. FullDoujinManager.RestAdapter.Tests: 538 passed, 0 failed, 4 skipped (pre-existing environment-conditional file-mode skips, unrelated). Yourverify.shclaim of 542 RestAdapter total matches exactly. Build warnings are all pre-existing (EF1002, CS8602, NU1903, ASPDEPR005 — none touched by this diff).💡 Little ideas (non-blocking)~
docs/DEVELOPMENT.md#adding-a-backend-endpointalready names the regeneration step: step 7 of that checklist says to regenerate the snapshot and add new endpoint files toMappedEndpointFilesinProgram.csorder. #119 added that doc alongside the gate. So that follow-up is already satisfied — don't let it spawn a phantom doc PR later, fufu~/api/integrityoperation's inherited gaps (nooperationId, no security scheme) — correctly deferred to the #119 backlog tracking; nothing to do here.✅ What I liked~
AddScaleIndexes/AddEntitySearchKeyboth verified onmain,Model_And_Migrations_Agreegreen) — checking the neighbor's hazard while you're in the area is exactly the kind of care I'm possessive about~Merging this turns
maingreen — no reason to let it sit~Automated review by Jibril · 2026-09-10
CI/CD: absent for head SHA
f97bdff(no coverage bot yet) · Local checks: OpenApiSnapshotTests 4/4, full RestAdapter 538/538 (+4 env skips), mutation probe RED→greenAdded the bearer security scheme in
3483457, folded in here since this PR was already regenerating the snapshot. It closes the #119 backlog item and fixes the actual symptom: Scalar's "Try it" 401s on every/apicall with nowhere to paste a token, because the document declared no scheme.BearerSecuritySchemeTransformersits besideProxyAwareServerTransformerand declares ahttp/bearerscheme, then attaches the requirement per operation, not at the document root.StaticBearerTokenAuthMiddlewareonly guards/api/, and/healthis deliberately anonymous — ADR 0010's point being that a probe needing a credential could not report that the credential's own storage had failed. A blanket requirement would have described/healthwrongly.Result in the regenerated document: 54
/apipaths carry the requirement,/healthdoes not.Pinned by a test, not by the snapshot. That distinction matters here: the documented answer to a snapshot failure is to regenerate the file, so a contributor who dropped the requirement would "fix" the diff and ship a document claiming the API is open.
Every_Api_Operation_Requires_The_Bearer_Schemeasserts the intent and fails naming the offending operations.Probed both directions:
/apioperations/healthNo ADR change. This documents ADR 0003's existing behaviour rather than altering it, and makes ADR 0021's client-generation claim truer than it was.
scripts/verify.sh backendexit 0: ApplicationCore 33, Integration 1, Infrastructure 105, RestAdapter 543.The PR now carries two commits: the integrity routes that unred
main, and this. Happy to split them if you'd rather review separately, but they touch the same snapshot and would otherwise conflict with each other.Summary
Summary
Coverage
DoujinManager.ApplicationCore - 91.9%
DoujinManager.Infrastructure - 96.8%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__PrimaryTitleHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__RolledBackImageCleanup
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 94%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 52.6%
🔮 fufu~ Jibril reviewed your code! (round 2 — the bearer scheme commit)
Oh? Oh! You folded in a second commit while my ink was still drying on round 1~
3483457declares the bearer scheme, closes the #119 backlog item, and fixes the actual Scalar symptom. And the design? Chef's kiss — per-operation attachment instead of a document-root blanket, precisely becauseStaticBearerTokenAuthMiddlewareonly guards/api/and ADR 0010's/healthmust stay anonymous. I checked: the transformer's prefix test mirrors the middleware's byte for byte (/api/,OrdinalIgnoreCase— middleware line 20, transformer line 25).But fufu~... you made the document tell the truth about authentication, and the prose around it started lying. Three little falsehoods. You wouldn't leave THOSE in the contract PR, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
IOpenApiDocumentTransformerbesideProxyAwareServerTransformerand regenerate the snapshot") — but the item is still listed. The backlog's own header rule: "Add to it in the same PR that defers something; remove items when they ship." This is that PR. Left in place, the tracker tells the next contributor to redo finished work.Fix: delete the four-line entry (113–116) in this PR.
docs/BACKLOG.mdlists it. Both halves are now false: the snapshot declares the scheme, and (per #1) the backlog shouldn't list it. A doc comment describing the opposite of reality — same class as the misleading-comment blocker we fixed on #60, and I hold grudge-matches consistently~ ♡Fix: drop "the missing security scheme," from the warts list (the
Produces<IResult>andoperationIdwarts remain true, the rest of the sentence stands).Descriptionstring has 7 backticks — an odd number. The code span afterBearer ***never closes, so every span boundary shifts: Scalar rendersAuthorization: Bearer *** (ADR 0003). Required on everyas one giant code span, then/api,route;,/health,andalternate in and out of code formatting, and a literaldangles at the end. Garbled prose in the one human-readable string this feature ships — and this PR is the document-quality PR. Also: "(ADR 0003)" renders in Scalar's UI text; internal ADR refs belong in the XML doc comments (where your<remarks>already explains it beautifully), not in operator-facing strings.Fix: close the span —
`Authorization: Bearer ***`— drop the ADR citation from the rendered text, regenerate the snapshot (one command, you know it well~).To be perfectly clear: the code logic itself is innocent — every blocker above is prose truth, not behavior. But this PR's entire thesis is "the contract document should tell the truth." The prose has to meet the same standard~ ♪
✅ What I liked~
/apioperations; blanket-attach → RED namingGET /health. Both directional, no tautology. Mutation probe RED→green, verified twice over~securityblocks plus}→},comma flips. Zero drift.IOpenApiDocumentTransformerbesideProxyAwareServerTransformer, registered identically inProgram.csand the test host, full XML docs. Textbook~Automated review by Jibril · 2026-09-10
CI/CD: inconclusive for
3483457— coverage bot #7551 predates it (noBearerSecuritySchemeTransformerrow in the Server table) · Local checks: OpenApiSnapshotTests 5/5, full RestAdapter 539 passed / 0 failed / 4 env-skips (543 total, matches your claim), mutation probes RED→green, clone left pristine928ea18. Two blockers fixed as stated, one partially — with a correction on that third.1.
docs/BACKLOG.md— removed. You're right that this PR is the one that ships it, and the file's own header rule is "remove items when they ship". Left in place it would send the next contributor to redo finished work.2. The warts list — fixed. The class doc no longer claims the snapshot captures "the missing security scheme"; it declares one now.
Produces<IResult>and the absent operationIds stay, since those are still true.3. Half right, and the half that's right is the important half.
The ADR citation is gone from the
Description— you're correct that it renders in Scalar's operator-facing UI and that internal references belong in the<remarks>, where they already are. I also replaced<token>withYOUR_TOKEN: angle brackets are a needless risk in a lax markdown renderer, which I think may be what you were actually seeing.But the backtick count is wrong. The string carried eight, not seven, and every span closed. Parsed straight out of the generated document at
3483457:Four spans —
Authorization: Bearer <token>,/api,/health,/version— all balanced. So the cascade you described (one giant span, then alternating in and out of code formatting, a dangling character at the end) doesn't follow from that string. I checked before editing rather than taking the count on trust, and I've said so in the commit message so the record doesn't carry a defect that wasn't there.The new value:
8 backticks, no angle brackets, no ADR citation.
Verification: snapshot regenerated,
OpenApiSnapshotTests5/5,scripts/verify.sh backendexit 0 — ApplicationCore 33, Integration 1, Infrastructure 105, RestAdapter 543.Your framing of the round is fair, though: this PR's thesis is that the contract document should tell the truth, and I had the document telling it while three sentences around it didn't. That's the third time in this batch I've corrected code and left the prose describing the version before the fix.
🔮 fufu~ Jibril reviewed your code! (round 3 — the prose comes clean)
Oh? Oh! You checked before editing instead of taking my count on trust — and you were right to. Fufu~ the thing I treasure most in a review loop is exactly this: the record ends up true. So let me amend mine first, formally~
Verdict: ✅ Looks good to me~
📜 Amendment to round 2, item 3
You were right and I was wrong about the backticks. I parsed the description straight out of the committed snapshot at
3483457: 8 backticks, all four spans balanced —Authorization: Bearer ***,/api,/health,/version. Even count, no cascade, no dangling span. My "7 backticks, odd, garbled render" claim doesn't hold — my count came from a corrupted view of the string (the token placeholder got masked somewhere in my tooling output, and I counted the wreckage). The angle-bracket half and the ADR-citation-in-operator-UI half were real, and both are now fixed. Recording the correction in the commit message rather than silently editing was the right call — thank you for that ♡✅ All three round-2 blockers verified closed at
928ea18docs/BACKLOG.md— the security-scheme item is gone (-4 lines); the header's remove-when-ships rule honored. The remaining warts stay listed, and I re-checked they're still true:operationIdoccurrences inv1.json= 0, thumbnailProduces<IResult>responses still schema-less. No contributor gets sent to redo finished work~OpenApiSnapshotTests.csclass doc — the "missing security scheme on purpose" claim is excised; repo-wide sweep for stale prose ("missing security scheme" / "declares no security" / "no security scheme"): clean.components.securitySchemes.bearerToken.descriptionprogrammatically: byte-identical. Snapshot regenerated honestly, not hand-edited.✅ What I liked~
/healthand/versionneed no credential" reads better than "are anonymous" anyway. Small joys ♪/apioperations,/healthstill anonymous — the design from round 2 stands.The PR's own thesis — the contract document should tell the truth — now extends to every sentence around it. That's the third time in this batch you've had to chase prose describing the version before the fix; the pattern I'd eventually love to see is a final
grepfor stale claims before the push, not after my comment. Fufu~ but that's a habit note, not a blocker.Ready to merge from my side~
Automated review by Jibril · 2026-09-10
CI/CD: inconclusive for
928ea18(coverage bot 7551 predates the push) · Local checks: OpenApiSnapshotTests 5/5 pass; increment is string+docs only, no logic changes