Skip to content

doctor(Consent): every refused signature gets a message, and a recorded one never reads as failed - #2227

Merged
peterdrier merged 10 commits into
mainfrom
section-doctor/2026-10-31T003917Z
Nov 1, 2026
Merged

peterdrier merged 10 commits into
mainfrom
section-doctor/2026-10-31T003917Z

Conversation

@peterdrier

Copy link
Copy Markdown
Owner

Requested by Peter · project thread

What

Section-doctor re-run on Consent (unattended daily routine). Run file: docs/health/runs/2026-10-31-Consent.md; target: src/Sections/Humans.Consent/Docs/health.md. Next up: Users, Preapprovals, Grants, Feedback.

Why

Since its August run the section gained the Consent Coordinator's verdicts and the reminder stamp, and its target no longer described it. Reading it whole turned up two small bugs on the signing page.

  • Fixed: a signature refused because the document version was not found left the member on /Consent with no message; if the consent was recorded but the follow-up report to Onboarding threw, the member was told their signature failed. The Review page now gates on the same name check Submit refuses on, from the already-loaded user (one user read instead of two). Two new controller tests; reviewer-approved.
  • Surface: SyncLegalDocumentsJob and ILegalDocumentSyncRunner are now internal (nothing outside Consent names them; the sibling reminder job already was). Reviewer-approved.
  • Docs: health.md rewritten with the member-review lane; IConsentWrite's summary no longer says the caller audits (the check write audits itself); Consent.md no longer calls the suspension job Consent's, and drops removed-nav narration and dated labels; authorization.md lists the policy Consent registers for Onboarding's queue; restating comments struck, mostly in tests.
  • Ledgered: CONSENT-1, no test pins that a Consent Coordinator is refused on the legal-document admin pages.
  • Skipped: the append-only trigger test (needs Postgres, integration tests only); the long GitHub sync method (sequential, splitting is cosmetic). Issue comment posted on Move post-save follow-ups (profile, consent, dietary) into the Onboarding orchestrator #2103 (shipped, recommend close).

No live render happened (cloud run); the views did not change.

Existing surface checked

No new durable surface; two public types made internal.

Checklist

  • Review tier stated: tier:standard.
  • No migration edited by hand (none in this PR).
  • No NOT NULL column added to an existing table (no schema change).
  • Targeting main on peterdrier/Humans, branched off origin/main.
  • Build + test pass locally: full dotnet test Humans.slnx -v quiet green; dotnet format whitespace --verify-no-changes clean.
  • Reuse-first checked: surface shrinks.
  • Section labeled, NuGet, new project rule, nav coverage: not applicable (no new pages, packages or rules).

Needs Peter

  1. (finding 11) In-app notices for a new legal document version go to every active member, while the email goes only to the document's team. Narrow the notice to the team too?
  2. (finding 12) Strip the unused DocumentVersion.ConsentRecords navigation, proven by an empty migration?

https://claude.ai/code/session_0186neA4NJZtkCuc6yysafYp


Generated by Claude Code

Only Consent's Section.cs names SyncLegalDocumentsJob; its sibling job is already internal.
Rule: memory/process/reuse-first-change-discipline.md (public surface needs a reason).
…ne never reads as failed

A NotFound refusal showed nothing; a failed onboarding report after a recorded consent showed
'submit error'. Review gates on the same name check Submit refuses on, from the loaded user.
Rule: memory/code/comments-earn-their-place.md (comments kept to the why).
IConsentWrite's summary said the caller audits; RecordConsentCheckAsync audits itself. Consent.md
named the suspension as Consent's job (it is Users'), narrated removed navs three times and carried
lane and dated labels. authorization.md now lists the policy Consent registers for Onboarding.
Rule: memory/code/comments-earn-their-place.md.
@peterdrier peterdrier added the tier:standard Normal review cycle label Oct 31, 2026
@peterdrier peterdrier self-assigned this Oct 31, 2026
@peterdrier

Copy link
Copy Markdown
Owner Author

Cost

Component Phase Model Fresh in Out Cache write Cache read ~$
worktree phase1 opus 22 2,194 13,730 898,847 0.59
select phase2 opus 29 5,305 48,902 1,455,876 1.17
assess phase3 opus 152 105,370 232,583 14,081,846 11.13
Conformance (subagent) phase3 haiku 18 11,454 154,564 284,184 0.28
Tests (subagent) phase3 opus 24 7,512 77,042 560,159 0.95
Controller intent (subagent) phase3 opus 18 4,005 76,865 423,942 0.79
Freshness (subagent) phase3 opus 28 6,943 68,532 622,937 0.91
History (subagent) phase3 opus 12 2,703 63,363 227,130 0.58
Comments (subagent) phase3 opus 20 7,524 103,389 449,727 1.06
Inbox (subagent) phase3 opus 22 4,981 81,187 518,575 0.89
verify-debt phase3f opus 5 2,331 5,555 688,081 0.44
Prose & surface (subagent) phase3f haiku 56 34,720 123,180 1,653,014 0.49
cut-sync-runner-surface strike-1 opus 49 10,183 38,571 6,266,018 3.63
review sync runner internal (subagent) strike-1 fable 580 18,200 166,587 1,137,377 4.14
consent-submit-errors strike-2 opus 51 20,931 36,850 6,823,645 4.17
review consent submit errors (subagent) strike-2 fable 676 20,285 158,065 1,340,558 4.34
doc-and-comment-truth strike-3 opus 25 5,775 29,701 3,648,702 2.15
strike doc and comment truth (subagent) strike-3 sonnet-5 28 6,220 57,178 634,709 0.33
bookkeeping phase5 opus 32 11,525 42,491 5,089,168 3.10
pr phase7 opus 16 5,860 11,302 2,827,585 1.63
total 1,863 294,021 1,589,637 49,632,080 42.76

API-equivalent $, list rates; run under subscription quota. Measured Phase 1 to PR creation; PR create/backfill and Phase 8 excluded.

Peak main-thread context: 317,523 tokens (pr).
Compactions detected: none.


Generated by Claude Code

@peterdrier

Copy link
Copy Markdown
Owner Author

Section review

1. Purpose, vision, intention

Docs/Consent.md has no purpose or vision heading; its one-line summary is "Legal documents synced from GitHub, per-version consent records (append-only), the Consent Coordinator audit/review queue." The code matches that line, with one drift: the review queue lives in Onboarding, Consent stores only the verdicts. Draft for Peter to rule on (not committed):

Draft. Consent is where the association proves, to a regulator or a court, that every member agreed to the current text of every document that applies to them. It keeps the legal documents in step with the legal team's GitHub repository, shows each member what they still owe, records each signature as a permanent tamper-proof entry, and reminds members before a deadline lapses. It also stores the Consent Coordinator's verdict on each new member (cleared, flagged, rejected with a reason).

Good looks like: a signature can never be altered or lost, a member is never asked to sign something they already signed (including under a merged account), and a new document version reaches the right people the night it is published.

Deliberate choices: Spanish is the binding text and the only one hashed; completing consents never gates app access and provisions nothing (the system-team sync reconciles membership); the Coordinator's verdict is an annotation, not a gate.

Out of scope: deciding who is a member (Governance), suspending for missing consents (Users), the Coordinator's review queue and workflow (Onboarding), sending mail (Email).

2. Public surface

Contracts interfaces

Id Item Who uses it Why it exists Verdict
S1 IConsentService (eight reads: signed versions, batch consent map, required rows, pending names, record count, review detail, one verdict, all verdicts) Agent, Debug, Governance, Notifications, Onboarding, Scanner, Surveys, Teams, Users, Shell The section's read face keep
S2 IConsentSubmission.SubmitConsentAsync Onboarding (widget), Development (persona seeder) The one consent write that leaves the section keep
S3 IConsentWrite (record check, set pending, record rejection) Onboarding, Tickets (gate terminal seeder), Development Coordinator verdict writes keep; its summary comment was wrong about audit and is fixed in this PR
S4 ILegalDocumentService (statutes) Governance Statutes page content keep
S5 ILegalDocumentSyncService.GetRequiredDocumentVersionsForTeamAsync Governance's membership calculator Which versions a team requires keep
S6 ILegalDocumentSyncRunner nobody outside Consent Was public only because the sync job was made internal in this PR

Public types

Id Item Who uses it Verdict
S7 ConsentReviewInfo, ConsentCheckStatus Debug, Governance, Notifications, Onboarding, Surveys, Teams, Users, Tickets, Development keep
S8 ConsentReviewPredicates (needs review, is flagged) used through extension calls in Onboarding keep
S9 ConsentReviewFormViewModel, ConsentReviewDetail Onboarding's widget keep
S10 ConsentRoles.ConsentCoordinator Notifications, Onboarding, Shell keep
S11 RequiredConsentRow, ConsentSubmitResult, RequiredDocumentVersionSnapshot, LegalDocumentVersionSnapshot, LegalDocumentDefinition returned by S1, S2, S5 keep
S12 ConsentResource Governance, Onboarding views keep (must stay public for localization)

Routes

Id Route Audience Verdict
S13 /Consent, /Consent/Review, /Consent/Submit any signed-in human keep
S14 /Consent/MyData any signed-in human keep
S15 /Legal/{slug?} anonymous keep
S16 /Legal/Admin/Documents (list, create, edit, archive, sync, version summary) Board or Admin keep

Shell contributions

Id Item Verdict
S17 Admin nav "Consent / Legal documents" (Board or Admin) keep
S18 Policy ConsentCoordinatorBoardOrAdmin, used by Onboarding's review queue keep; authorization.md now names it
S19 User parts: the Coordinator verdict card (admin detail) and the rejection reason (member's own account-status wall) keep
S20 Issue queue Legal (Consent Coordinator) keep; key stays Legal for stored rows
S21 Data migration consent.profile-review keep until it has run in production, then retire with the legacy fallback
S22 Audit actions (seven) and notification sources (three, one retired) keep

Sections it depends on: Users, Teams, Governance, Notifications, Email, AuditLog, Gdpr, Onboarding (contracts only), HumanLifecycle (via Users).

3. Jobs

Id Job Schedule What it does Gating config
J1 SyncLegalDocumentsJob daily 04:00 UTC Pulls every document folder from GitHub, adds a version when the Spanish file's commit changed, notifies and emails affected members GitHubSettings
J2 SendReConsentReminderJob daily 04:00 UTC Emails members whose consent deadline falls within the warning window, once per cooldown EmailSettings.ConsentReminderDaysBeforeSuspension, ConsentReminderCooldownDays

Both run half an hour before Users' suspension job.

4. Section debt

Id Debt Why it is still there What fixing it takes
D1 Legacy review fallback (ConsentReviewService.LegacyReview, UserInfo.IsRejected's legacy arm, the profile review columns) Waits on the consent.profile-review data migration in production (ledger row CENTRAL-92) Run the migration on prod, then a removal PR and a column-drop PR
D2 New-version in-app notices go to every active member, while the email goes only to affected teams Asked in the August run, never ruled on A ruling; then narrow the notice to the same team members
D3 DocumentVersion.ConsentRecords navigation is declared but never walked It is the sole declaration of the FK and its delete rule Re-express the FK from the record side and prove with an empty migration; needs your OK for the migration
D4 The append-only DB triggers have no test The repository tests are in-memory and cannot run a trigger A Postgres integration test (integration tests run only nightly)
D5 No test pins that a Consent Coordinator is refused on /Legal/Admin Policy is checked at attribute level only One authorization test
D6 SyncFolderBasedDocumentAsync is one long method A sequential sync body; splitting would be cosmetic Nothing unless it changes
D7 Open issue #2103 (post-save follow-ups into Onboarding) Shipped in #2110 but still open Close it

No Docs/debt.yml exists for the section; nothing in it to verify.

5. Questions

  1. Should the "new legal document version" in-app notice go only to members of the document's team (like the email), instead of every active member? (D2)
  2. Is the draft purpose above right enough to formalize in Consent.md?
  3. OK to strip the unused DocumentVersion.ConsentRecords navigation, with an empty migration as proof? (D3)
  4. Close Move post-save follow-ups (profile, consent, dietary) into the Onboarding orchestrator #2103 as shipped? (D7)

Generated by Claude Code

@peterdrier

Copy link
Copy Markdown
Owner Author

@codex review


Generated by Claude Code

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-31T01:06:15.258346Z a04eff7 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown

Reviewed commit 9b5bace — no issues found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a04eff7614

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

PR Surface Report

Compared ed543724...e7ab06af. Scored with reforge 0.31.0.

Summary: 30 changed file(s) | EF migrations: 0 added file(s), max 0/1 per context

Reforge Surface Score

metric base head delta
total 61704 61657 -47
surface 14489 14439 -50

Section Deltas

section base head delta
Consent 1079 1032 -47

Section Size & Complexity Deltas

section LOC delta files delta classes delta cognitive p95 delta
Consent +5 0 0 0
Corpus +5 0 0 0

Corpus is the solution-wide rollup (same source as Corpus Size & Complexity below), not a sum of the section rows above - cognitive p95 in particular doesn't compose across sections.

Rule Deltas

rule base head delta
oneImplementationInterface 360 336 -24
backgroundJob 204 192 -12
exportedWriteMethod 1680 1668 -12
methodComplexity 18078 18084 +6
methodCount 16863 16860 -3
exportedUnusedExternally 782 780 -2

Corpus Size & Complexity

metric base head delta
locProd 180972 180977 +5
methods 7713 7712 -1

At head: largest class SurveyService, most complex method DevelopmentDashboardSeeder.SeedAsync.

Published Write Surface

17 of 51 sections publish write capability, 51 interfaces (-1).

No longer published

  • Consent -> ILegalDocumentSyncRunner

Interface Surface

No new interfaces or interface members.

Diff Size

bucket added deleted
code 51 50
tests 48 28
docs 333 100
other 15 0

New Files

  • docs/health/runs/2026-10-31-Consent.md
  • src/Sections/Humans.Consent/Docs/debt.yml

…nger reports the signature as failed

The decorator already evicted every alias before reloading; the reload error now logs instead of
propagating to the controller's error flash. Codex round 1 on #2227.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186neA4NJZtkCuc6yysafYp
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186neA4NJZtkCuc6yysafYp
@peterdrier
peterdrier merged commit 130aa17 into main Nov 1, 2026
6 checks passed
@peterdrier
peterdrier deleted the section-doctor/2026-10-31T003917Z branch November 1, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tier:standard Normal review cycle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants