docs(06-01): complete formatter interface + TableFormatter plan
This commit is contained in:
133
.planning/phases/06-output-reporting/06-01-SUMMARY.md
Normal file
133
.planning/phases/06-output-reporting/06-01-SUMMARY.md
Normal file
@@ -0,0 +1,133 @@
|
||||
---
|
||||
phase: 06-output-reporting
|
||||
plan: 01
|
||||
subsystem: pkg/output
|
||||
tags: [output, formatter, registry, tty, colors, refactor]
|
||||
requirements: [OUT-01, OUT-06]
|
||||
dependency-graph:
|
||||
requires: [pkg/engine.Finding]
|
||||
provides:
|
||||
- "output.Formatter interface"
|
||||
- "output.Options struct"
|
||||
- "output.Registry (Register/Get/Names/ErrUnknownFormat)"
|
||||
- "output.TableFormatter"
|
||||
- "output.IsTTY / output.ColorsEnabled"
|
||||
affects: [cmd/scan.go PrintFindings wrapper]
|
||||
tech-stack:
|
||||
added:
|
||||
- "github.com/mattn/go-isatty (promoted from indirect to direct)"
|
||||
patterns:
|
||||
- "Registry via init()-registered formatters"
|
||||
- "TTY-aware color stripping using zero-value lipgloss.Style"
|
||||
- "Deprecated wrapper (PrintFindings) delegating to new interface"
|
||||
key-files:
|
||||
created:
|
||||
- pkg/output/formatter.go
|
||||
- pkg/output/formatter_test.go
|
||||
- pkg/output/colors.go
|
||||
- pkg/output/colors_test.go
|
||||
modified:
|
||||
- pkg/output/table.go
|
||||
- pkg/output/table_test.go
|
||||
- go.mod
|
||||
decisions:
|
||||
- "Registry is unguarded: all registration happens at init() before main."
|
||||
- "newTableStyles(false) returns zero lipgloss.Style values — lipgloss passes text through verbatim, guaranteeing no ANSI escapes on non-TTY writers."
|
||||
- "PrintFindings(findings, unmask) kept as backward-compat wrapper so cmd/scan.go still compiles unchanged until Plan 06 (scan flag wiring)."
|
||||
- "ColorsEnabled honours NO_COLOR (https://no-color.org/) before the TTY check."
|
||||
metrics:
|
||||
duration: ~8m
|
||||
completed: 2026-04-05
|
||||
tasks: 2
|
||||
commits: 3
|
||||
---
|
||||
|
||||
# Phase 06 Plan 01: Formatter Interface + TableFormatter Refactor Summary
|
||||
|
||||
Established the `output.Formatter` interface, a name-keyed registry, and TTY-aware color detection, then refactored the existing colored-table output to implement `Formatter` and register itself as `"table"`. This is the foundation every other Phase 6 formatter (JSON, SARIF, CSV) will plug into.
|
||||
|
||||
## What Was Built
|
||||
|
||||
### 1. Formatter interface + registry (`pkg/output/formatter.go`)
|
||||
|
||||
- `Formatter` interface: `Format(findings []engine.Finding, w io.Writer, opts Options) error`
|
||||
- `Options` struct: `{ Unmask, ToolName, ToolVersion }`
|
||||
- `Register(name, f)` — called from `init()` functions
|
||||
- `Get(name)` — returns `ErrUnknownFormat` wrapped via `fmt.Errorf("%w: %q", ...)`
|
||||
- `Names()` — sorted list for `--output` help text
|
||||
- `ErrUnknownFormat` sentinel, discoverable via `errors.Is`
|
||||
|
||||
### 2. TTY detection (`pkg/output/colors.go`)
|
||||
|
||||
- `IsTTY(*os.File) bool` — wraps `isatty.IsTerminal` + `isatty.IsCygwinTerminal`, nil-safe
|
||||
- `ColorsEnabled(io.Writer) bool` — returns false when `NO_COLOR` is set, when writer is not an `*os.File`, or when the file is not a TTY
|
||||
- Promoted `github.com/mattn/go-isatty v0.0.20` from indirect (via lipgloss) to a direct dependency in `go.mod`
|
||||
|
||||
### 3. TableFormatter refactor (`pkg/output/table.go`)
|
||||
|
||||
- `TableFormatter struct{}` implements `Formatter`, registered under `"table"` in `init()`
|
||||
- Writes to the caller-supplied `io.Writer` (no more hardcoded `os.Stdout`)
|
||||
- New `tableStyles` bundle + `newTableStyles(colored bool)` factory — when `colored==false` every style is a zero `lipgloss.Style`, which renders text verbatim with zero ANSI escapes
|
||||
- Preserves the exact layout from Phase 5: PROVIDER/KEY/CONFIDENCE/SOURCE/LINE columns plus optional VERIFY column, indented sorted metadata line, footer `"N key(s) found."`
|
||||
- Respects `Options.Unmask` to toggle between `KeyMasked` and `KeyValue`
|
||||
- `PrintFindings(findings, unmask)` retained as a deprecated thin wrapper that delegates to `TableFormatter{}.Format(..., os.Stdout, Options{Unmask: unmask})` — keeps `cmd/scan.go` compiling untouched until Plan 06.
|
||||
|
||||
## Tests
|
||||
|
||||
All green on `go test ./pkg/output/... -count=1`:
|
||||
|
||||
- **formatter_test.go**: `Register/Get` round-trip, `Get` unknown → `ErrUnknownFormat` via `errors.Is`, `Names()` sort ordering, `Options` zero-value defaults.
|
||||
- **colors_test.go**: `ColorsEnabled(&bytes.Buffer{})==false`, `NO_COLOR=1` forces false, typed-nil writer does not panic, `IsTTY(nil)==false`.
|
||||
- **table_test.go (new)**:
|
||||
- Empty slice → exact `"No API keys found.\n"`
|
||||
- Two findings (one verified) written to `bytes.Buffer` → output contains no `\x1b[` escapes, includes `VERIFY` column and `"2 key(s) found."` footer
|
||||
- Unverified-only header does NOT contain `VERIFY`
|
||||
- Verified row shows `live`
|
||||
- Unmask=false renders `KeyMasked`; Unmask=true renders `KeyValue`
|
||||
- VerifyMetadata `{z:1, a:2}` renders `a: 2` before `z: 1`
|
||||
- `Get("table")` returns a `TableFormatter`
|
||||
- **table_test.go (legacy)**: `PrintFindings` stdout-capture tests still pass (backward compat preserved).
|
||||
|
||||
Full project suite (`go test ./...`) green: cmd, engine, engine/sources, legal, output, providers, storage, verify.
|
||||
|
||||
## Key Decisions
|
||||
|
||||
1. **Registry is unguarded.** All `Register` calls happen from package `init()` before `main`, which Go runs sequentially. Adding a mutex would be dead weight.
|
||||
|
||||
2. **Plain styles via zero lipgloss.Style.** Instead of conditionally calling different render functions, `newTableStyles(false)` returns `lipgloss.NewStyle()` for every field. A zero `lipgloss.Style` passes text through verbatim, so the same `fmt.Fprintf(... style.header.Render("PROVIDER") ...)` code path produces colored output on a TTY and plain output to a `bytes.Buffer`.
|
||||
|
||||
3. **Backward-compat wrapper.** `PrintFindings` stays so `cmd/scan.go` compiles without edits. Plan 06 will replace it with a registry lookup when wiring the `--output` flag. Deprecated comment in place.
|
||||
|
||||
4. **NO_COLOR precedence.** `ColorsEnabled` checks `NO_COLOR` before the TTY check, matching the https://no-color.org/ spec ("should check for a NO_COLOR environment variable that, when present (regardless of its value), prevents the addition of ANSI color").
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
None — plan executed exactly as written.
|
||||
|
||||
## Commits
|
||||
|
||||
| Commit | Type | Message |
|
||||
| ------- | ---- | ---------------------------------------------------------- |
|
||||
| 291c97e | feat | add Formatter interface, Registry, and TTY color detection |
|
||||
| 8c37252 | test | add failing tests for TableFormatter refactor |
|
||||
| 8e4db5d | feat | refactor table output into TableFormatter |
|
||||
|
||||
## Foundation for Next Plans
|
||||
|
||||
- **Plan 02** (JSON + SARIF): create `pkg/output/json.go` and `pkg/output/sarif.go`, each with an `init()` calling `Register`. Implement `Format` against `[]engine.Finding`. Use `Options.ToolName`/`Options.ToolVersion` for SARIF `tool.driver` metadata.
|
||||
- **Plan 03** (CSV): identical pattern with `encoding/csv`.
|
||||
- **Plan 06** (scan wiring): replace `output.PrintFindings(findings, unmask)` in `cmd/scan.go` with `output.Get(flag)` + `f.Format(findings, os.Stdout, Options{Unmask: unmask, ToolName: "keyhunter", ToolVersion: version})`.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- pkg/output/formatter.go — FOUND
|
||||
- pkg/output/formatter_test.go — FOUND
|
||||
- pkg/output/colors.go — FOUND
|
||||
- pkg/output/colors_test.go — FOUND
|
||||
- pkg/output/table.go — modified (TableFormatter present, `Register("table", ...)` in init)
|
||||
- pkg/output/table_test.go — modified (TableFormatter test block present)
|
||||
- go.mod — `github.com/mattn/go-isatty v0.0.20` in direct require block
|
||||
- Commits 291c97e, 8c37252, 8e4db5d — FOUND in `git log`
|
||||
- `go build ./...` — succeeds
|
||||
- `go test ./pkg/output/... -count=1` — PASS
|
||||
- `go test ./...` — PASS
|
||||
Reference in New Issue
Block a user