Plan: Comment-Accuracy Audit Remediation

On this page

Status

Step Description Status

1

#908 fix(authz): engine never emits authz.cache_miss / authz.cache_refreshed audit events

Done (2026-07-16) — decision recorded (user, after the #907 triage exposed the 2026-06-30 no-trail closure and reopened #908): wiring DEFERRED — the emission path needs a real design (the engine holds no outbox tx; ADR required) and is re-scoped in #908 as a feature; the false-emission comments corrected in this MR (error.rs + engine.rs ×2; lib.rs by B2a c6be0dd5, invalidation.rs already truthful); builders stay in crate::audit, unit-tested

2

#909 fix(auth): introspection serve-on-outage grace is 1× not the documented 2× cache_ttl_max_seconds

Done (2026-07-15) — decided docs-to-1× (ADR-029 + archived plan + internal docs + birth-commit history all pin 1×; 2× would double stale-authz exposure); corrected all 5 public surfaces, amended ADR-029’s understating wording, added the grace_window pin test

3

#910 fix(financial,security): i64→i32 narrowings justify safety with validation that does not exist

Done (2026-07-03) — MR !877 (removed the unjustified i64→i32 narrowings)

4

#911 fix(rules): evaluate() reason= claims a timeout-bounded await + fallback compile that do not exist

Done (2026-07-15) — comment half already remediated on main by 8f1bbe7d (809 removed the false [expect]; surviving docs honestly defer to #784); timeout half is #784 (related, open); #911 closed citing that SHA

5

#912 fix(cases): effective_disposition doc promises multiple concurrent-authority rows but returns one (LIMIT 1)

Done (2026-07-16) — decided single-row/newest-wins is correct (the multi-row sibling all_effective_dispositions was deliberate at birth 0eb6980a and deleted as dead code in #239/8bf4218c; every consumer singular); rewrote the doc to describe LIMIT 1 newest-wins + a restore pointer, added the concurrent-decision newest-wins integration test

6

#913 fix(web): /security/* pages documented "(admin only)" but mounted require_admin_or_supervisor

Done (2026-07-16) — decided the mount is right (deliberate #476 decision; backing craig-security policies admit supervisors in both jurisdictions; effe418e fixed alerts.rs identically and missed the 5 siblings): corrected the 5 route docstrings + same-vintage stale surfaces (ui-overview role table + mermaid, developer-guide RBAC bullet, rbac.spec.ts comment); supervisor nav-gap already filed as #1036

7

#914 docs(xtask): correct 51 stale/inaccurate comments

Done (2026-07-16) — all 51 findings resolved: 43 corrected in this pass (each verified against current code by a fresh-context agent before editing), 7 already fixed on main (3 by 442f910f/958, 4 intra-block duplicates), 1 (docker.rs stale [allow(dead_code)] on a live field) split out as a non-comment change to its own fix issue

8

#915 docs(craig-web): correct 48 stale/inaccurate comments

Done (2026-07-05) — B2b bundled sweep effe418e

9

#916 docs(craig-cli): correct 32 stale/inaccurate comments

Done (2026-07-06) — B2c bundled sweep 6721d3f2

10

#917 docs(craig-cases): correct 31 stale/inaccurate comments

Done (2026-07-16) — all 31 findings resolved: 20 corrected in this pass (each verified against current code by a fresh-context agent; includes the security-sensitive encryption-claim surfaces), 11 already fixed on main (mostly by the C3/ADR-049 encryption-module rewrite)

11

#918 docs(craig-intake): correct 29 stale/inaccurate comments

Done (2026-07-05) — B2b bundled sweep effe418e

12

#919 docs(craig-test-lib): correct 21 stale/inaccurate comments

Done (2026-07-05) — B2a bundled sweep c6be0dd5

13

#920 docs(craig-authz): correct 19 stale/inaccurate comments

Done (2026-07-05) — B2a bundled sweep c6be0dd5

14

#921 docs(craig-auth): correct 18 stale/inaccurate comments

Done (2026-07-16) — all 18 findings resolved: 16 corrected in this pass (each verified against current code by a fresh-context agent; understated-by-default for the token-validation crate — incl. the per-peer ActorJwksRegistry correction, the 2-dots/4-dots token-shape fix, and the exp/iat/TTL gate wording matching the actual Validation config), 2 intra-block duplicates covered by sibling edits

15

#922 docs(craig-exchange): correct 17 stale/inaccurate comments

Done (2026-07-05) — B2b bundled sweep effe418e

16

#923 docs(craig-common): correct 13 stale/inaccurate comments

Done (2026-07-05) — B2a bundled sweep c6be0dd5

17

#924 docs(craig-security): correct 12 stale/inaccurate comments

Done (2026-07-05) — B2b bundled sweep effe418e

18

#925 docs(craig-api): correct 10 stale/inaccurate comments

Done (2026-07-16) — all 10 findings resolved: 5 corrected in this pass (each verified against current code by a fresh-context agent — incl. two residual dead ClientIpSource::XForwardedFor references the earlier B2a rewrite itself introduced, verified against the vendored axum-client-ip 1.3.1 enum; the SIGHUP claim dropped from the shutdown_signal reason=; the bootstrap test-doc rewritten to the typed-variant pinning), 5 already fixed on main (c6be0dd5/#796 B2a sweep + 08e4609a/#795 helper split)

19

#926 docs(craig-placement): correct 9 stale/inaccurate comments

Done (2026-07-16) — 8 of 9 findings resolved in docs (6 corrected in this pass, each verified against current code by a fresh-context agent; 2 already fixed on main); finding 1 (the foster-home license-type constraint-test header) escalated to fix #1041 — verification showed the test is vacuous (422 from payload-shape rejection, never reaches license_type validation), so no truthful comment exists short of fixing the test

20

#927 docs(craig-financial): correct 9 stale/inaccurate comments

Done (2026-07-05) — B2b bundled sweep effe418e

21

#928 docs(craig-seed): correct 9 stale/inaccurate comments

Done (2026-07-06) — B2c bundled sweep 6721d3f2

22

#929 docs(craig-reference): correct 9 stale/inaccurate comments

Done (2026-07-05) — B2a bundled sweep c6be0dd5

23

#930 docs(craig-store): correct 8 stale/inaccurate comments

Done (2026-07-16) — all 8 findings resolved: 6 corrected in this pass (each verified against current code by a fresh-context agent), 2 already fixed on main

24

#931 docs(partner): correct 30 stale comments across craig-partner-* adapters

Done (2026-07-06) — B2c bundled sweep 6721d3f2

25

#932 docs(misc): correct 31 stale comments across smaller crates

Done (2026-07-06) — B2c bundled sweep 6721d3f2

Epic: &64 — Comment-accuracy audit remediation (2026-06-29) (25 children linked via epic_id).
Uncertain-findings triage: #907 (child of &64) holds the 27 verifier-uncertain findings for human triage.
Issues: latent-bug fixes #908–#913 (fix:); comment doc-drift #914–#932 (docs:).
Branch: one feature/{desc} branch + MR per issue; the comment-only docs: steps MAY batch per crate. Exception: Step 3 (#910) spans two services (financial + security) — it may ship as one MR touching both, or split into two MRs under the one issue.
Labels / weights: fix/docs + priority (P1-high/P2-medium/P3-low) + weight are applied on every child (see the Files Touched roll-up).
Dependencies: none between steps. Steps 1, 3, 4, 5, 6 are decision-gated — each is a comment-vs-code disagreement whose resolution is either "fix the code to match the comment" or "fix the comment to match the code"; resolve the behaviour question first, then implement. Where a step gives no recommended default (Steps 4, 5, 6 pose a genuine product/behaviour question), the behaviour decision is a human call — raise it on the issue and block the step until answered. For Step 1: if the decision is to wire event emission, a new ADR is required (it is an architectural choice); if the decision is to defer, no ADR is needed and the comments are corrected to match. Steps 7–25 are pure comment edits with no decision.

Context

A repository-wide comment-accuracy audit ran on 2026-06-29 against main, on the hypothesis (since confirmed) that successive refactor passes had left source comments describing behaviour the code no longer has. Every comment class was checked against the current code: line comments (//), doc-comments (///, //!), block comments (/* */), and the prose strings inside attributes (reason="…" in [expect(…)]/[allow(…)], note="…" in #[deprecated(…)]).

Method — a multi-agent workflow over all 1,008 Rust files (~239K LOC), partitioned into 123 balanced groups. Each group received two independent auditor lenses (A = guarantees/semantics: doc-comments + reason= strings asserting runtime guarantees; B = references/drift: stale see foo(), renamed identifiers, wrong counts/limits, resolved TODOs), unioned and de-duplicated; then a skeptical per-group verifier that defaults to not-stale unless the current code clearly contradicts the comment (the false-positive filter); then an independent second confirm on every high-severity / false-guarantee survivor. 413 agents total.

Funnel — 575 raw flags → 421 confirmed stale comments → 125 rejected as false positives → 27 uncertain. The high-stakes escalation upheld 49 of 51 (2 recanted and were dropped). 151 of 421 were flagged independently by both lenses.

The material finding — most of the 421 are harmless prose drift, but 15 are not: the comment promises behaviour the code never performs. These 15 findings are grouped into 6 fix: issues (#908–#913) — several bundle related comments at one site (#908 = 4 authz comments, #909 = 2 introspection docstrings, #910 = financial + security, #913 = 5 /security/ routes). They are filed separately from the doc-drift because correcting the *comment would paper over the gap; the behaviour is the question. These are the J5/J7 class the pre-commit protocol targets (a false reason=/doc guarantee compiles clean and clippy stays silent — only judgment catches it).

The audit’s per-finding detail (verbatim comment / problem / suggested correction) is captured in the 25 child issues (#908–#932); the 27 uncertain findings are recorded in tracking issue #907 for triage; the scratch report artifact produced by the run is superseded by this plan and those issues.

This plan complements the parallel &62 — Code-quality audit remediation (2026-06-28) (a code-smell/architecture audit); the two are disjoint — that one found logic/structure defects, this one found comment-vs-code disagreement.

Scope

In scope:

  • Resolve the 15 latent code-behaviour-gap findings, grouped into 6 fix: issues (#908–#913): decide, per gap, whether the code or the comment is wrong, then implement the fix (code change with a test, or a corrected comment) — no silent comment edit over a real gap.

  • Correct the 406 confirmed doc-drift comments (#914–#932): comment-only edits bringing each comment in line with the current code, or deleting the obsolete comment. Grouped by crate for incremental shipping.

Out of scope:

  • The 27 uncertain findings (verifier low-confidence) — enumerated in tracking issue #907 § Uncertain; they need human triage before they become work. File follow-up issues only if triage confirms them.

  • The 125 rejected findings (confirmed false positives) — no action.

  • Any code refactor beyond what a specific fix: step’s decision requires (e.g. function decomposition, dependency changes) — out of band; file separately.

  • Re-running the audit / building a recurring comment-lint gate — a candidate follow-up, not part of this remediation.

Design

Triage taxonomy

Every confirmed finding is exactly one of two kinds, and the kind dictates the issue type and the discipline:

Kind Meaning Discipline

Latent code gap (fix:, 15 findings grouped into 6 issues → #908–#913)

The comment asserts behaviour/guarantee the code does not perform.

Resolve the behaviour question FIRST. Either wire the missing behaviour (with a test that asserts it) or correct the comment to describe reality — never edit the comment to hide the gap. Where the comment claimed a security/audit/availability guarantee, default to treating the gap as the defect until proven a deliberate deferral.

Doc-drift (docs:, 406 findings → #914–#932)

The code is correct; the comment is stale/wrong (renamed identifier, dead see X, wrong count, resolved TODO, stale step numbering).

Comment-only edit. The diff for these steps must contain NO production-logic change — only comments, doc-comments, and attribute reason=/note= strings. git diff of the step shows comment lines only.

Grouping of the docs: steps

The 406 doc-drift findings are grouped one docs: issue per crate for crates with ≥ 8 findings (Steps 7–23), plus two buckets: craig-partner-* adapters (Step 24, 30 findings across 10 adapter crates) and remaining smaller crates (Step 25, 31 findings). Each issue body carries its findings as a tickable checklist (file:line · severity · category · current comment · suggested fix), so a step is self-contained against its issue.

Verification discipline

  • docs: steps: comment-only. After edits, cargo fmt --all + cargo clippy --all-targets — -D warnings clean, and the existing test battery still green (no behaviour changed). Confirm the staged diff touches only comment text.

  • fix: steps: the decision determines the change. A code change ships with a test that asserts the now-true behaviour (e.g. an integration test that the authz cache-miss event is staged; a try_into/checked_* narrowing with a property/range test; a tokio::time::timeout with a timeout-path test). A comment-only resolution (decision: the deferral is intended) ships the corrected comment plus a tracking note for the deferred behaviour.

  • The pre-commit J1–J8 review applies as usual; J5 (load-bearing comment/reason= matches reality) is the gate these steps most directly satisfy.

Steps

Per-finding detail (verbatim comment, the contradicting code, the suggested correction) lives in each step’s GitLab issue. The summaries below are the decision and recommended approach; the issue is the working checklist.

Step 1 — #908 fix(authz): engine never emits authz.cache_miss / authz.cache_refreshed audit events

Files: crates/craig-authz/src/engine.rs, crates/craig-authz/src/error.rs, crates/craig-authz/src/invalidation.rs, crates/craig-authz/src/lib.rs, crates/craig-authz/src/audit.rs

build_cache_miss_event / build_cache_refreshed_event (audit.rs) exist and are unit-tested but are called only from tests; the cache-miss path (engine.rs:387-398) only warn!-logs and returns PolicyMissing → Forbidden, and refresh_entry stages no event. Comments at engine.rs:12/388, error.rs:14, invalidation.rs:18, lib.rs:15 assert the events ARE emitted. Decision: is operator-visible auditing of policy-missing / cache-refresh intended? If yes (recommended — the builders are ready), stage the envelopes on the caller’s outbox tx with an integration test; consider an ADR. If deferred, correct the four comments to say the builders exist but are not yet wired, and track the wiring separately. Highest priority — audit-trail gap on an access-control decision.

Step 2 — #909 fix(auth): introspection serve-on-outage grace is 1× not 2×

Files: crates/craig-auth/src/introspection.rs

Docs at introspection.rs:31 and :122 say expired cache entries serve up to 2 * cache_ttl_max_seconds past expiry; hard_grace = cache_ttl_max_seconds (line 618) makes it 1×. The internal get_stale docstring and hard_expires_at field doc already describe 1×. Decision: is 2× the intended grace? If yes, set hard_grace = 2 * cache_ttl_max_seconds; if no, correct the two public docstrings to 1×. Failure-posture doc that overstates the stale-auth window is security-relevant either way.

Step 3 — #910 fix(financial,security): overflow narrowings justify safety with validation that does not exist

Files: services/craig-financial/src/api/payments.rs, crates/craig-financial-contracts/src/payments.rs, services/craig-security/src/api/archive.rs, services/craig-security/src/store/archive.rs

financial payments.rs:243 reason= claims the period day-count is "bounded by validation (~366 days)", but period_start/period_end are #[garde(skip)] — request-supplied dates flow unvalidated into an i64→i32 narrowing. security archive.rs:104 reason= claims the purge count is bounded by batch size, but the store UPDATE has no LIMIT. Recommended: for payments, add range validation (or try_into + typed error) so the narrowing is actually bounded, then make the reason= true; for archive, either bound with a LIMIT matching the reason= or switch to a checked narrowing and correct the reason=. J7/J4 class.

Step 4 — #911 fix(rules): evaluate() reason= overstates the await/cache behaviour

Files: services/craig-rules/src/engine.rs

The #[expect] reason on evaluate (engine.rs:157) describes a "cache lookup with fallback compile" and a "timeout-bounded await"; the cache miss returns RuleSetNotFound (no compile) and reply_rx.await (lines 188-191) has no tokio::time::timeout. Decision: should the eval-thread await be timeout-bounded (a stuck thread otherwise hangs the request)? If yes, wrap in tokio::time::timeout with a typed timeout error; correct the reason= to match the actual cache-miss + await behaviour either way.

Step 5 — #912 fix(cases): effective_disposition returns one row but the doc promises many

Files: services/craig-cases/src/store/screening_decisions.rs

The doc-comment (lines 72-77) says concurrent-authority decisions yield multiple rows ordered decided_at DESC; the query is ORDER BY decided_at DESC LIMIT 1 + fetch_optional. Decision: must concurrent screening authority surface multiple rows? If yes, drop LIMIT 1 and return the set; if the single-row model is correct, rewrite the doc to describe LIMIT-1 behaviour.

Step 6 — #913 fix(web): /security/* "(admin only)" docs vs require_admin_or_supervisor mount

Files: services/craig-web/src/routes/security/{archive,audit,changes,nist,reviews}.rs, services/craig-web/src/main.rs, services/craig-web/src/middleware.rs

Five /security/ list handlers carry an "(admin only)" docstring but are mounted with require_admin_or_supervisor (admin + supervisor). *Decision: are supervisors intentionally allowed? If yes, change the five docstrings to "admin or supervisor"; if not, tighten the mount to require_admin and add a role-rejection test. Access-control doc vs guard.

Steps 7–25 — docs: comment-only corrections by crate (#914–#932)

Files: per the crate named in each step’s title (see the Files Touched roll-up).

Each step corrects the confirmed stale comments listed in its issue’s checklist to match current code (or deletes the obsolete comment). Comment-only — no production logic changes. Notable security-doc-integrity items live in these steps even though the code is correct: craig-cases (Step 10, #917) — api/encryption/mod.rs wrongly states narratives are NOT app-encrypted while report.rs encrypts them; craig-api (Step 18, #925) — rate_limit.rs references InsecureClientIp/SecureClientIp/TRUSTED_PROXIES that do not exist (actual extractor is socket-only ClientIp); craig-auth (Step 14, #921) — claims.rs describes actor-JWT validation against "craig-signing’s JWKS" rather than the per-peer registry; xtask (Step 7, #914) — quality_budgets/validate_authz_coverage still labelled "stub/report-only" for gates that now hard-fail.

Files Touched

Per-issue roll-up — labels/priority/weight as applied on each child. The issue itself carries the exact file:line list:

Issue Labels · weight Surface

#908

P1-high,fix · 5

craig-authz engine/error/invalidation/lib/audit

#909

P2-medium,fix · 2

craig-auth introspection

#910

P1-high,fix · 3

craig-financial payments (+contracts), craig-security archive (+store)

#911

P2-medium,fix · 3

craig-rules engine

#912

P2-medium,fix · 2

craig-cases store/screening_decisions

#913

P2-medium,fix · 2

craig-web routes/security/* + main + middleware

#914

P2-medium,docs · 5

xtask (51)

#915

P3-low,docs · 5

craig-web (48)

#916

P3-low,docs · 5

craig-cli (32)

#917

P2-medium,docs · 5

craig-cases (31)

#918

P3-low,docs · 3

craig-intake (29)

#919

P3-low,docs · 3

craig-test-lib (21)

#920

P3-low,docs · 3

craig-authz (19)

#921

P2-medium,docs · 3

craig-auth (18)

#922

P3-low,docs · 3

craig-exchange (17)

#923

P3-low,docs · 3

craig-common (13)

#924

P3-low,docs · 2

craig-security (12)

#925

P2-medium,docs · 2

craig-api (10)

#926

P2-medium,docs · 2

craig-placement (9)

#927

P3-low,docs · 2

craig-financial (9)

#928

P3-low,docs · 2

craig-seed (9)

#929

P3-low,docs · 2

craig-reference (9)

#930

P2-medium,docs · 2

craig-store (8)

#931

P3-low,docs · 3

craig-partner-* adapters (30)

#932

P3-low,docs · 5

smaller crates (31)

Verification

  1. cargo fmt --all + cargo clippy --all-targets — -D warnings — clean after each step.

  2. docs: steps: the staged diff touches only comment / doc-comment / attribute-string lines (no logic change); cargo nextest run --workspace still green.

  3. fix: steps: the chosen code change ships with a test asserting the now-true behaviour (or, for a decided deferral, the corrected comment + a tracking note for the deferred work).

  4. cargo xtask validate — the full pre-push battery (includes J5 comment-accuracy judgment + the plan-lint step).

  5. cargo xtask plan-lint — this plan’s Status table uses canonical tokens.

Documentation Updates

  • CHANGELOG.adoc — behaviour-changing fix: steps carried their own entries in their MRs (#910 !877, #912 semantics unchanged, #913 docs-to-mount); comment-only steps shipped none, per this rule.

  • ADRs — Step 1 resolved as a DEFERRAL (comments corrected; wiring re-scoped in #908 with an ADR required when picked up), so no ADR here per this line’s own rule.

  • This plan — all 25 Status cells Done; nav entry moved to Archive by the closing MR (which also corrected the pre-existing misfiling under Planned).

  • .claude/CLAUDE.md — reviewed by the FINAL MR: no status-table change warranted (epic &64 stays open pending the re-scoped #908 wiring feature).

Edit this page · latest