mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 12:21:31 +00:00
shankar0123
b1fa4970be
Phase 9 ARCH-M2 closure Sprint 9b — the orders cut Sprint 9
explicitly deferred. Closes the bigger half of the
internal/service/acme.go split via the Option B sibling-file pattern
(operator's post-Sprint-8 choice — package stays `service`, no
import-path churn for ~70 call sites).
Why Sprint 9b is a separate commit from Sprint 9
================================================
Sprint 9 shipped four cuts whose source ranges were each a single
contiguous region in acme.go (nonces, authz, challenges, gc — line
ranges 423-444 / 999-1018 / 1326-1561 / 1914-1965 at audit time).
Sprint 9b crosses a different shape:
1. Non-contiguous source: orders block A (lines 795-1223 pre-cut)
+ helpers block B (1237-1283 pre-cut), with
firstAvailableIssuer at 1227-1235 staying behind because it's
called from Phase 4 RevokeCert + RenewalInfo too.
2. Per-helper move-vs-stay decision: each helper in the
post-FinalizeOrder cluster needed an explicit call-graph audit
to decide whether it moves with orders or stays with the
surviving cross-concern surface in acme.go.
Same shape as the Sprint 8 / Sprint 8b split (mechanical vs harder-
shape on separate commits) — the Phase 9 prompt's "do not bundle"
rule enforcing itself.
What moved
==========
New `internal/service/acme_orders.go` (540 LOC)
-----------------------------------------------
Contains the entire Phase 2 orders concern:
- The `// --- Phase 2 — orders + authz + finalize + cert download`
banner (moves with its contents, not left as a phantom in
acme.go pointing at code that's no longer there).
- The four public order methods: CreateOrder, LookupOrder,
FinalizeOrder, LookupCertificate.
- The FinalizeOrderResult shape (consumed only by FinalizeOrder
callers).
- accountOwnsACMECert (only callsite: LookupCertificate).
- The three orders-internal ID helpers: randIDSuffix +
base32encode (random ACME entity IDs) + identifierStrings
(audit details).
Per-helper move-vs-stay analysis
================================
Grep against the post-Sprint-9 tree pinned every helper's call sites
before the cut decision:
randIDSuffix: callers in CreateOrder (4x) + FinalizeOrder
(1x) — all moving. MOVE.
base32encode: only caller is randIDSuffix. MOVE.
identifierStrings: only caller is CreateOrder. MOVE.
accountOwnsACMECert: only caller is LookupCertificate. MOVE.
firstAvailableIssuer: three call sites — FinalizeOrder (moving),
RevokeCert (staying, Phase 4), RenewalInfo
(staying, Phase 4). STAY in acme.go.
Doc-comment updated to flag cross-concern
status + explain why it's not moved.
mapACMERevocationReason: only caller is RevokeCert. STAY (already
sits in the Phase 4 region of acme.go and
belongs with its sole caller).
jwksThumbprintsEqualSvc: only caller is RotateAccountKey. STAY
(Phase 4 helper; never had an orders
relationship).
Side effect: import cleanup
===========================
With randIDSuffix moved, acme.go no longer references crypto/rand.
The `cryptorand "crypto/rand"` aliased import is removed.
Per-symbol audit confirmed every other import (context, crypto/x509,
errors, fmt, strings, sync/atomic, time, jose, internal/api/acme,
internal/config, internal/domain, internal/repository) is still
consumed by surviving code in acme.go.
Net effect
==========
acme.go: 1634 → 1158 LOC pre-doc-update; 1162 LOC post the four-line
firstAvailableIssuer doc-comment refresh (-472 net, -28.9% from the
post-Sprint-9 size). Original audit-time size was 1965 LOC; cumulative
Sprint-9 + Sprint-9b reduction: 1965 → 1162 = -803 LOC (-40.9%).
The biggest single backend hotspot from the audit is now smaller
than mcp/tools.go.
Behavior preservation contract
==============================
1. gofmt -l clean across acme.go + acme_orders.go.
2. go vet ./internal/service/... — no findings.
3. staticcheck ./internal/service/... ./cmd/server/...
./internal/api/handler/... ./internal/scheduler/...
./internal/mcp/... — no findings.
4. go test -short -count=1 ./internal/service/... — green
(including the orderTrackingRepo + TestCreateOrder_* +
TestFinalizeOrder_* + TestLookupCertificate_* surface that
pins the moved code's behavior).
5. Broader-importer suite green:
go test -short -count=1 ./cmd/server/... ./internal/api/handler/...
./internal/scheduler/...
6. Per-symbol import audit on both files (no unused imports left,
no missing imports introduced).
Same-package resolution means every call inside FinalizeOrder /
RevokeCert / RenewalInfo to firstAvailableIssuer crosses a file
boundary but stays within `package service` — zero overhead at
compile time, zero change to the public method-set on
service.ACMEService.
What remains for Phase 9
========================
Three sibling-file splits queued for Sprints 10-12:
- Sprint 10: internal/mcp/tools.go (1867 LOC) grouped by tool
domain (certificate / agent / job / discovery / admin).
- Sprint 11: internal/api/handler/auth_session_oidc.go (1577 LOC)
split per handler verb.
- Sprint 12: cmd/agent/main.go (1489 LOC) mirroring the cmd/server
pattern from Sprint 8.
Refs: ARCH-M2 (god-files), Phase 9 audit. Sprint 9b is the named
follow-on to Sprint 9; after this commit, the service-layer cut from
the audit's hotspot list is fully closed.