audit 6: documents, comments, dead code and staticcheck; the test gaps closed
test / test (push) Canceled after 0s
test / test (push) Canceled after 0s
The last stage of the 2026-10-12 audit; no cut. Doc drift: architecture.md (the sender check in § OpenDKIM, the limit: row's 48 hours, the inbound log and filter-list routes and who may open them, migrations to 0011 and every table, KitPage as production data, Reload configuration as a restore writes, the panel's roles counted), schema-migrations.md (tables and columns since v2), guide.md (a delivery's lines go at the next prune; INBOUND_JOURNAL_MILTER_SOCKET moved to architecture.md's internal list), development.md (the env guard, the deferral the e2e proves, its fixed sleeps), roadmap.md. Comments say what the code does now; TestBaselineIsTheOnlyMigration is TestChainStartsAtTheBaseline; the milter package doc names its three milters. Dead code gone: view.Facts, dmarc HostedSuggestion and ValidateHostedAddress, app UpdateMode (each with the test that alone kept it), the method checks the method-bound routes made dead; Settings.WithResult calls resultFlash. staticcheck 2026.1 in test.yml on both modules, clean: the e2e package builds without its tests now (a test and two constants moved out of non-test files), three findings fixed. The test moved, TestImageBuildPreservesLogrotateMode, had sat in a non-test file and never run; run, it failed on the entrypoint (docker run without a hostname) and now asks with --entrypoint stat. The test gaps no step named, each closed: follow() with mail.log gone past blindMin and made again; CheckRestore with a manifest that is not JSON; the origin check with a port; every /server/ pattern in web.go through server() and in the routes table (it found GET /server/health/fragment missing from the table - guarded, not tested); L9 as the owner decided it on 2026-10-12, the comment on senderSession.Headers and the test passing an h that holds the earlier messages' From:. Plan stage 6 ticked; roadmap audit row done, 81/81. Tested on the test server: go vet + go test of 27 packages, make e2e (all pass, 513 s) on this tree; staticcheck locally on both modules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -496,6 +496,49 @@ tests; `TestRenderInboundMaps` has no two-routes-one-next-hop case (L5);
|
||||
rows of `review-2026-08-followups` (apps/domains/users/dmarc handlers
|
||||
directly, `internal/dmarc` service) are still open.
|
||||
|
||||
*Checked in stage 6 (2026-10-15):* **closed** since, each by the test that
|
||||
now holds it: the H1 stamp (e2e `image_stamped_with_version`); cert re-copy
|
||||
(e2e `renewed_certificate_picked_up`); a recreated log and lines written to
|
||||
the old inode after the switch (`TestFollowTailsAndRotates`,
|
||||
`TestFollowReadsTheRotatedFileAfterTheSwitch`,
|
||||
`TestALogRotatedWhileDownIsReadToItsEnd`); a truncated `postqueue -p`
|
||||
(`TestQueueIDsRefusesATruncatedListing`); a `sent` row left alone by the
|
||||
sweep and by a reused id (`TestCloseUnreportedClosesOnlyQueuedRows`,
|
||||
`TestUpdateStatusRewritesOnlyTheNewestRow`); H4
|
||||
(`TestAddApplicationRefusesALoginDifferingOnlyInCase`,
|
||||
`TestServiceCreateFoldsTheLogin`); M1's ordering
|
||||
(`TestARestoreStartThatFailsIsARestoreAgain`; the real reload by e2e
|
||||
`full_backup_restore`); M3 (`TestSnapshotRefusesAMissingDatabase`,
|
||||
`TestBackupOfAMissingDatabaseIsRefused`); M5 (`TestEncryptionPasswordFloor`);
|
||||
L1 (`TestDecryptLeavesTheOutputAloneOnFailure`); L2
|
||||
(`TestCertsLinksAreFollowed`, `TestCheckRestoreReadsOnlyRegularFiles`); L3
|
||||
(`TestBytesAfterTheLastChunkRejected`); M13 (e2e
|
||||
`panel_hung_mid_message_is_counted`, and the unit fixture now holds the
|
||||
`cleanup` lines as the stand wrote them); M15
|
||||
(`TestServerPTRTimeoutIsUnknown`,
|
||||
`TestServerAddressTimeoutIsUnknownAndKeepsTheAddresses`);
|
||||
`TestLimitRefusalsSince` (the real `limitRefusalsQuery` is planned in
|
||||
`TestSendLogQueriesUseTheirIndexes`); `TestOpenMigratesA20Database` (migrates
|
||||
to the head, 11); M10 (`TestDomainImportIsCheckedLikeTheForms`); M11
|
||||
(`TestPanelJS`: `poll_test.js`, `unsaved_test.js`); M8
|
||||
(`TestLoginTakesAsLongForAnUnknownName`); M7
|
||||
(`TestTheLastGlobalSurvivesTwoAtOnce`); `internal/atomicfile` and
|
||||
`internal/supervisor` (tests of their own); L5
|
||||
(`TestRenderInboundMapsOneTLSLinePerNextHop`); the handler rows of
|
||||
`review-2026-08-followups` (struck in the roadmap with their reasons; users,
|
||||
DMARC and apps have handler tests). The five then **still open**, named by
|
||||
no step, were closed in stage 6 the same day: `follow()` with a log gone
|
||||
past `blindMin` and made again
|
||||
(`TestFollowCountsALogItCannotReadAndReadsItAgain`); `CheckRestore` with a
|
||||
manifest that is not JSON (`TestCheckRestoreRefusesAManifestThatIsNotJSON`);
|
||||
a Host with a port against an Origin (`TestOriginCheckComparesThePort`);
|
||||
every `/server/` pattern in web.go registered through `server()` and listed
|
||||
in the routes table (`TestEveryServerRouteIsGuardedAndListed` - it found
|
||||
`GET /server/health/fragment` missing from the table; the route was guarded,
|
||||
not tested); L9 as the owner decided it, the comment on
|
||||
`senderSession.Headers` and `TestSenderCheckResetsBetweenMessages` passing
|
||||
an `h` that holds the earlier messages' `From:`.
|
||||
|
||||
### Checked and sound
|
||||
|
||||
Worth writing down so the next review does not re-read it: the sender check's
|
||||
@@ -1512,20 +1555,55 @@ log it owns anyway; envdoc_test reads `${VAR:-}` and the Go helpers, not
|
||||
bare `$VAR` or python (none today); the 600 s pruning in crashexit is not
|
||||
tested (the count and the spared are).
|
||||
|
||||
### Stage 6 — documentation and roadmap sweep (no product code, no cut)
|
||||
### Stage 6 — documentation and roadmap sweep (no product code, no cut) (done 2026-10-15)
|
||||
|
||||
- [ ] Every item under *Doc drift* above, file by file. — **Sonnet**
|
||||
- [x] Every item under *Doc drift* above, file by file. — **Sonnet**
|
||||
*Done 2026-10-15:* architecture.md (the sender check in § OpenDKIM, the
|
||||
`limit:` row's 48 hours, the inbound log and filter-list routes and who
|
||||
may open them, migrations to `0011` and every table, `KitPage` as
|
||||
production data, Reload configuration as a restore writes, the panel's
|
||||
roles counted again); schema-migrations.md (the tables and columns since
|
||||
v2); guide.md (a delivery's lines go at the next prune;
|
||||
`INBOUND_JOURNAL_MILTER_SOCKET` out of the operator's table, into
|
||||
architecture.md's internal list); development.md (the env guard as
|
||||
envdoc_test reads now, the deferral the e2e proves, the e2e's fixed
|
||||
sleeps said where they are); roadmap.md's closing paragraph. Already right
|
||||
by then: the `.alive` mark, `dmarc-ingest` as a second writer, the chain
|
||||
head, M1's wording, the version stamp.
|
||||
- [ ] roadmap.md: done ahead on 2026-10-13/14 with the owner -
|
||||
`template-data-typing` struck, `structured-logging` dropped,
|
||||
`review-2026-08-followups` closed with each row placed in a stage or
|
||||
dropped with its reason. What is left: check that every *Test gaps* entry
|
||||
above is named by the step that touches its code, and add it to that step
|
||||
where it is not. — **Sonnet**
|
||||
- [ ] Comments named above (`milter.go`, `sendlog.go`, `store.go`, `schema_test.go`,
|
||||
*Done 2026-10-15:* see *Checked in stage 6* under *Test gaps*: five were
|
||||
open and named by no step; each got its test in this stage (L9 as the
|
||||
owner decided it).
|
||||
- [x] Comments named above (`milter.go`, `sendlog.go`, `store.go`, `schema_test.go`,
|
||||
`machine.go`, `inboundspam.go`, `logtail.go`, supervisord.conf,
|
||||
entrypoint.sh, docker-compose.yml). — **Haiku**
|
||||
- [ ] Dead code removal from § Nits, with `staticcheck` added to `test.yml` so it
|
||||
*Done 2026-10-15:* each said what its code does now (the journal-milter
|
||||
tempfails at MAIL FROM; the rejected status covers the sender check; the
|
||||
two roles; the log is read whole for a filter list's hits; logrotate
|
||||
uses `create`; the guide's headings as they are). `TestBaselineIsTheOnlyMigration`
|
||||
renamed `TestChainStartsAtTheBaseline`. supervisord.conf and
|
||||
entrypoint.sh were already right.
|
||||
- [x] Dead code removal from § Nits, with `staticcheck` added to `test.yml` so it
|
||||
stays gone. — **Haiku**
|
||||
*Done 2026-10-15:* gone: `view.Facts`, `dmarc.Service.HostedSuggestion`
|
||||
and `ValidateHostedAddress`, `app.Service.UpdateMode` (each with the
|
||||
test that alone kept it), the method checks of `HandleDomainDMARC` and the
|
||||
`default:` branches of the user and settings handlers;
|
||||
`Settings.WithResult` calls `resultFlash`. Kept: `domainIDSet` (used by
|
||||
the send log's authorization), `KitPage()`'s `User` (the fixtures render
|
||||
`KitPage()` without the handler); the `set-permissions` branch was gone
|
||||
already. staticcheck 2026.1 (the release for Go 1.26) runs in test.yml
|
||||
on both modules, clean; for the e2e module the package now builds without
|
||||
its tests (a test and two constants moved out of non-test files), and
|
||||
three findings there were fixed (ST1005, SA4006). The test moved,
|
||||
`TestImageBuildPreservesLogrotateMode`, had sat in a non-test file since
|
||||
it was written, and go test had never run it; run, it failed on the
|
||||
entrypoint (`docker run` without a hostname) and now asks past it.
|
||||
|
||||
### Decided with the owner on 2026-10-12
|
||||
|
||||
|
||||
Reference in New Issue
Block a user