B3.4.1

BUILD.md

@@ -26,10 +26,6 @@ go build -o bin/gomailtest.exe ./cmd/gomailtest
 go build -ldflags="-s -w" -o bin/gomailtest.exe ./cmd/gomailtest
 ```
 
-## Note on Legacy Binaries
-
-The individual shim binaries (`smtptool`, `imaptool`, `pop3tool`, `jmaptool`, `msgraphtool`) that existed in v3.0.x have been removed in v3.1.0. Use `gomailtest <protocol> <action>` instead.
-
 ## Cross-Platform Builds
 
 ### Build for Linux

ChangeLog/3.4.1.md

@@ -0,0 +1,29 @@
+# Version 3.4.1
+
+## Changes
+
+### Changed
+
+- Centralized TLS version parsing/formatting into the shared `internal/common/tls`
+  package (relocated from `internal/smtp/tls`). SMTP, IMAP, POP3, and EWS now use
+  the single `ParseTLSVersion`/`TLSVersionString` helpers instead of their own
+  copies. As a side effect, EWS now honors `--tlsversion 1.0`/`1.1` consistently
+  with the other protocols.
+
+### Added
+
+- Unit tests for the shared `internal/common/tls` package covering TLS version
+  parsing/formatting, cipher-strength analysis, TLS warnings/recommendations, and
+  certificate-chain analysis (raising the package from 0% to ~81% coverage).
+
+### Removed
+
+- Duplicated local `parseTLSVersion()` implementations in the IMAP and POP3
+  clients, and the EWS `buildTLSConfig` version switch and local
+  `tlsVersionString` helper.
+
+### Documentation
+
+- Removed stale "legacy per-protocol binaries were removed in v3.1.0"
+  announcements from current documentation (README, BUILD, TOOLS, and the
+  per-protocol docs), keeping the legacy-name migration aids.

README.md

@@ -161,7 +161,7 @@ Each protocol uses a dedicated prefix:
 
 ## Migrating from Legacy Binary Names
 
-The individual tool binaries (`smtptool`, `imaptool`, `pop3tool`, `jmaptool`, `msgraphtool`) were removed in v3.1.0. Replace them with `gomailtest <protocol> <action> --flag`:
+Replace legacy `*tool` invocations with `gomailtest <protocol> <action> --flag`:
 
 | Old | New |
 |-----|-----|

TOOLS.md

@@ -8,8 +8,6 @@ This document provides a comprehensive comparison of all protocols supported by
 > ```
 > gomailtest <protocol> <action> [flags]
 > ```
->
-> The legacy per-protocol binaries (`smtptool`, `imaptool`, `pop3tool`, `jmaptool`, `ewstool`, `msgraphtool`) were removed in v3.1.0.
 
 ## Quick Reference
 

cr-results/CODE_REVIEW_AND_IMPROVEMENTS_2026_06_22a.md

@@ -0,0 +1,281 @@
+# Repository and Architecture Evaluation: gomailtesttool
+**Date:** 2026-06-22
+**Version Reviewed:** `3.4.1`
+**Reviewer:** GitHub Copilot CLI
+
+## 1. Executive Summary
+
+The repository is in a **healthy and maintainable state overall**. It has a clear product direction, a coherent Go module layout, a unified CLI entry point, and a straightforward build/test flow. The main architectural risk is not disorder at the top level, but **duplication and boundary blur** inside the command layer.
+
+**Overall assessment:**
+* **Strength:** the codebase has the right macro-structure for a multi-protocol Go CLI suite.
+* **Primary concern:** command orchestration is repeated across many subcommands, which increases maintenance cost and drift risk.
+* **Recommended direction:** keep the current structure, but refactor toward **shared action execution paths** and a cleaner separation between Cobra wiring and reusable protocol operations.
+
+---
+
+## 2. Current Repository Assessment
+
+### 2.1 High-Level Shape Is Good
+
+The current structure is sensible for the product:
+
+* `cmd/gomailtest/` is a thin entry point.
+* `internal/common/` holds cross-cutting concerns such as bootstrap, logging, retry, validation, versioning, networking, and TLS helpers.
+* `internal/protocols/*` contains protocol-specific command/config/action code.
+* lower-level protocol details live in dedicated packages such as `internal/smtp/protocol`, `internal/imap/protocol`, `internal/pop3/protocol`, and `internal/jmap/protocol`.
+* optional additional surfaces exist in `internal/serve` and `internal/devtools`.
+
+This is a strong base. The repo already behaves like a **well-scoped tool suite**, not an ad hoc collection of scripts.
+
+### 2.2 Product Direction Is Coherent
+
+The repository presents a unified binary, `gomailtest`, covering:
+
+* SMTP
+* IMAP
+* POP3
+* JMAP
+* EWS
+* Microsoft Graph
+* serve mode
+* developer tooling
+
+That scope is reflected consistently in `README.md`, `BUILD.md`, the `Makefile`, and the active module path in `go.mod`.
+
+### 2.3 Documentation Coverage Is Strong
+
+The project has unusually good repo-level documentation for a CLI utility:
+
+* `README.md`
+* `BUILD.md`
+* `UNIT_TESTS.md`
+* protocol-specific docs under `docs/protocols/`
+* security and release documentation
+
+That documentation makes the codebase easier to extend safely, even when the implementation has some duplication.
+
+---
+
+## 3. Architecture Review
+
+### 3.1 The Top-Level Architecture Is Sound
+
+The command registration in `cmd/gomailtest/root.go` is clean and direct. The root command delegates to focused subcommands without accumulating protocol logic in the entry point.
+
+This is the correct shape:
+
+* one binary
+* multiple protocol subcommands
+* shared internal utilities
+* protocol-specific implementation packages
+
+There is no evidence that the repository needs a rewrite or a major re-layout.
+
+### 3.2 The Command Layer Has Too Much Repeated Plumbing
+
+The largest architectural issue is repeated setup logic across protocol commands. In multiple `internal/protocols/*/cmd.go` files, each action repeats the same sequence:
+
+1. bind flags
+2. load config file
+3. build config from Viper
+4. validate config
+5. create signal context
+6. initialize loggers
+7. run the action
+
+This pattern is visible across `smtp`, `imap`, `pop3`, `jmap`, `ews`, and `msgraph`.
+
+**Why it matters:**
+* bug fixes to command startup behavior must be applied in many places
+* logging/config behavior can drift between protocols
+* adding new actions becomes more copy/paste-driven than design-driven
+
+**Recommendation:** introduce a shared command runner/helper that encapsulates the standard lifecycle and leaves each action responsible only for protocol-specific config and execution.
+
+### 3.3 CLI Wiring and Reusable Operations Are Not Yet Separated Cleanly Enough
+
+`internal/protocols/*` currently mixes:
+
+* Cobra wiring
+* config loading
+* validation
+* operational behavior
+
+This is workable, but it weakens reuse. The best signal is `internal/serve`, which already needs to construct protocol configs and call protocol-layer behavior directly.
+
+**Why it matters:**
+* the CLI and HTTP server are two frontends for related behavior
+* if reusable execution logic stays embedded inside command-oriented packages, cross-surface reuse remains awkward
+* future surfaces such as automation hooks or additional APIs will increase this pressure
+
+**Recommendation:** move toward a clearer split:
+
+* **command packages** for Cobra flags, help text, and argument binding
+* **service/execution packages** for protocol actions that can be called from both CLI and `serve`
+
+This should be done incrementally, not as a large rewrite.
+
+### 3.4 Context Handling Is Duplicated and Partly Redundant
+
+`cmd/gomailtest/root.go` sets a signal-aware context in `PersistentPreRunE`, but many subcommands also call `bootstrap.SetupSignalContext()` again inside `RunE`.
+
+**Why it matters:**
+* cancellation behavior is not clearly centralized
+* the root-level setup becomes less meaningful if actions create replacement contexts
+* this makes future context propagation harder to reason about
+
+**Recommendation:** choose one model and standardize it:
+
+* either the root command owns context creation and subcommands use `cmd.Context()`
+* or subcommands own context setup and the root stops creating one
+
+The first model is usually cleaner for Cobra applications.
+
+### 3.5 The `serve` Surface Is Useful but Highlights the Missing Execution Boundary
+
+`internal/serve/server.go` is well-scoped and appropriately small. It exposes a narrow REST surface, applies API key middleware, and keeps request handling direct.
+
+At the same time, `internal/serve/cmd.go` has to know about:
+
+* SMTP config loading
+* MS Graph config loading
+* Graph client initialization
+* availability rules for backend actions
+
+That is a hint that the execution boundary is still too low-level.
+
+**Recommendation:** preserve `serve`, but reduce how much backend assembly logic it owns over time.
+
+---
+
+## 4. Quality and Test Signal
+
+### 4.1 Baseline Health Is Good
+
+Current repository tests pass with:
+
+```bash
+go test ./...
+go test -cover ./...
+```
+
+That is a strong baseline and means the repo is not in a broken or unstable state.
+
+### 4.2 Coverage Is Uneven in Important Areas
+
+Coverage is good in several shared utility packages:
+
+* `internal/common/email` — 97.3%
+* `internal/common/network` — 89.7%
+* `internal/common/ratelimit` — 100.0%
+* `internal/common/tls` — 81.4%
+* `internal/common/validation` — 89.6%
+
+But several operational packages remain thinly covered:
+
+* `internal/common/retry` — 0.0%
+* `internal/protocols/imap` — 4.5%
+* `internal/protocols/jmap` — 8.3%
+* `internal/protocols/msgraph` — 6.9%
+* `internal/protocols/pop3` — 3.8%
+* `internal/protocols/ews` — 13.0%
+* `internal/serve` — 46.9%
+
+**Why it matters:** the repo is strongest around helpers and validation, but weaker around real action execution paths and retry behavior.
+
+### 4.3 Shared Retry Logic Deserves Immediate Attention
+
+`internal/common/retry/retry.go` is a particularly important package because it influences user-visible reliability and failure behavior. Right now it has no tests, and it contains logic that deserves closer scrutiny, including error classification by string matching.
+
+This is a small package with high leverage. It should be one of the first places to strengthen.
+
+---
+
+## 5. Main Findings
+
+### 5.1 What Is Working Well
+
+* repository structure is easy to navigate
+* module/build story is clear
+* documentation is strong
+* the command tree is understandable
+* shared utility packages are reasonably well factored
+* the repo is currently passing tests
+
+### 5.2 Main Risks
+
+* repeated command startup/orchestration logic
+* weak separation between CLI wiring and reusable operations
+* duplicated context lifecycle setup
+* under-tested retry and action-layer behavior
+* slight drift risk between architecture docs and the live tree
+
+### 5.3 Architecture Verdict
+
+This is **not a repo with a bad architecture**. It is a repo with a **good core architecture that now needs consolidation work**. The next improvement phase should focus on reducing duplication and clarifying boundaries, not on changing the whole shape of the project.
+
+---
+
+## 6. Recommended Improvement Plan
+
+### Phase 1: Shared Command Lifecycle Cleanup (High Priority)
+
+Create a shared helper for the repeated command flow:
+
+* bind flags
+* load config
+* validate config
+* initialize context
+* initialize logging
+* invoke action
+
+This reduces drift across all protocol actions and makes future additions cheaper.
+
+### Phase 2: Context Model Standardization (High Priority)
+
+Standardize around one cancellation model:
+
+* root command creates context
+* subcommands consume `cmd.Context()`
+
+Remove duplicate signal-context creation from individual actions once that path is in place.
+
+### Phase 3: Execution Boundary Extraction (Medium Priority)
+
+Refactor reusable operational logic out of Cobra-oriented packages where practical so both:
+
+* `gomailtest <protocol> <action>`
+* `gomailtest serve`
+
+call the same execution layer.
+
+This will improve consistency and make future automation surfaces safer.
+
+### Phase 4: Targeted Test Hardening (Medium Priority)
+
+Focus on:
+
+* `internal/common/retry`
+* `internal/serve`
+* msgraph action paths
+* protocol action success/failure behavior
+
+Prefer targeted tests over broad coverage chasing.
+
+### Phase 5: Architecture Doc Refresh (Low/Medium Priority)
+
+Update `ARCHITECTURE_DIAGRAM.md` so it reflects the current tree more precisely, especially newer `internal/common/*` packages and the actual active surfaces.
+
+---
+
+## 7. Final Recommendation
+
+The repository should continue in its current direction. The structure is already strong enough to support future work. The priority now is **consolidation, not reinvention**:
+
+* centralize repeated command plumbing
+* clean up context ownership
+* extract reusable execution seams
+* strengthen tests where runtime behavior is most sensitive
+
+If those improvements are made, the existing architecture should scale well for the next phase of growth.