195 lines
10 KiB
Markdown
195 lines
10 KiB
Markdown
# Audit remediation analysis
|
||
|
||
Status: discussion in progress; no implementation changes made.
|
||
|
||
This document owns analysis of unresolved remediation choices, in the discussion
|
||
order below. The [catalog](catalog.md) owns the findings, accepted remediation
|
||
decisions, and their status. Original audits retain detailed evidence. Record
|
||
accepted conclusions in the catalog and mark the corresponding discussion here
|
||
decided; a decision does not close a finding before implementation.
|
||
|
||
Existing catalog decisions for F03, F10, F11, and F13–F15 remain selected. In
|
||
particular, performance testing is not a prerequisite for F13–F15, and F14 and
|
||
F15 are to be designed together. CommandKit is pre-release with no consumers;
|
||
consumer compatibility and migration are not constraints on these choices.
|
||
Straightforward correctness repairs need not wait for this discussion sequence.
|
||
|
||
| Order | Findings | Question | Discussion status |
|
||
| --- | --- | --- | --- |
|
||
| 1 | F10 | Which flag declaration rules should be documented as the intended contract? | Decided: retain and document existing rules; remediation pending |
|
||
| 2 | F21 | What narrow diagnostic adaptation API would simplify both integrations? | Adapters compared; recommendation pending agreement |
|
||
| 3 | F16 | Which flag-consumption facts can parsing and completion usefully share? | Boundary analysis pending |
|
||
| 4 | F14–F15 | How should both operations reuse source-prefix processing? | Joint design pending; remediation selected |
|
||
| 5 | F17–F20 | Which private extractions materially simplify the existing code? | Individual extraction review pending |
|
||
|
||
## F10 — Flag declaration naming
|
||
|
||
**Question:** should remediation document the existing naming rules, or change
|
||
them first? This concerns names declared by applications, not the contents of
|
||
operator-supplied flag values.
|
||
|
||
**Evidence:** [catalog F10](catalog.md#f10--p3-public-flag-metadata-omits-enforced-naming-and-reserved-spelling-rules)
|
||
and [documentation audit D2](13-documentation.md#d2--p3-public-flag-metadata-omits-enforced-declaration-constraints).
|
||
`FlagMeta`, `validateBindings`, and `validFlagName` in
|
||
[command.go](../../../command.go) establish the documentation gap and current
|
||
validation. Shared reporting spellings are defined in
|
||
[commandmeta](../../../internal/commandmeta/metadata.go).
|
||
|
||
The historical accepted grammar ADR explicitly selected lowercase names with
|
||
hyphen-separated words. It is available at commit
|
||
`82128463cd9649a426da7c5e3f3ce33bd37cdd9f`, path
|
||
`docs/adr/0006-command-grammar.md`; the same commit's
|
||
`docs/roadmap/initial.md` carried that direction into implementation. This
|
||
supports the broad naming convention, but does not independently establish every
|
||
character-level detail of the current validator. Historical records provide
|
||
context; they do not replace the current canonical API documentation.
|
||
|
||
Current long names consist of nonempty groups of Unicode lowercase letters or
|
||
digits separated by single hyphens. Names may start with digits or consist
|
||
entirely of digits. Uppercase letters, underscores, dots, and leading, trailing,
|
||
or repeated hyphens are rejected. Examples:
|
||
|
||
| Declaration name | Current result |
|
||
| --- | --- |
|
||
| `max-tokens`, `profile2`, `2fa`, `123`, `équipe` | Accepted, subject to ordinary collision checks |
|
||
| `maxTokens`, `max_tokens`, `max.tokens`, `max--tokens`, `-profile`, `profile-` | Rejected |
|
||
|
||
Core reserves `help`, `quiet`, `verbose`, `debug`, `progress`, and `format` for
|
||
its own flags. A short spelling is optional; when present it is one byte,
|
||
cannot be `-`, and cannot be the reserved help spelling `h`. The long-name
|
||
lowercase rule does not apply to short spellings. Declaration defects are
|
||
reported at installation, before command execution.
|
||
|
||
**Options:**
|
||
|
||
1. Retain and document the current rules. This makes the shared convention
|
||
explicit without introducing new validation or conversion machinery.
|
||
2. Relax the stylistic restrictions, allowing application names such as
|
||
`maxTokens` or `max_tokens` while preserving core reservations and necessary
|
||
parsing constraints. This gives applications more naming freedom but weakens
|
||
the common vocabulary; its precise supported grammar would need selection.
|
||
3. Tighten the rules, for example to ASCII names beginning with a letter. This
|
||
creates a narrower convention but removes currently accepted names without
|
||
an identified operator benefit.
|
||
|
||
**Recommendation:** option 1. Retain lowercase, hyphen-separated long names,
|
||
including current Unicode lowercase and digit support. Keep core reservations
|
||
and the existing short-spelling rules. Recommend readable names without banning
|
||
numeric names or adding an ASCII-only restriction. Do not automatically rewrite
|
||
invalid names or accept alternate spellings through normalization.
|
||
|
||
Document the exact declaration contract once in `FlagMeta` GoDoc and link related
|
||
helpers, including `RenameFlag` and flag-alias declarations, as applicable. Keep
|
||
operator syntax in the CLI reference. This is a documentation remediation;
|
||
it does not require a new ADR, parser, or validation framework.
|
||
|
||
**Decision:** option 1 accepted and recorded in the catalog. Retain the current
|
||
long-name grammar, Unicode lowercase and digit support, reservations, and
|
||
short-spelling rules. No runtime validation change or name normalization is
|
||
selected. F10 remains open until its public documentation is corrected.
|
||
|
||
## F21 — Shared ConfigKit diagnostic adaptation
|
||
|
||
**Question:** what concrete helper, if any, removes duplicated CLI adaptation
|
||
without introducing a broader diagnostics framework?
|
||
|
||
**Evidence:** [catalog F21](catalog.md#f21--p3-both-integrations-maintain-the-same-configkit-diagnostic-adaptation)
|
||
links both adapters and the original analysis.
|
||
|
||
**Options to examine:** a narrow public ckconfig helper returning existing core
|
||
diagnostics and an explanation, or retaining private implementations if the
|
||
shared API adds more complexity than it removes.
|
||
|
||
**Analysis:** ckconfig's private `configKitDiagnosticsWithHeading` already
|
||
accepts a heading, native sources, and native diagnostic records, and returns
|
||
core diagnostics and an explanation. ckprompt duplicates the evidence snapshot,
|
||
severity partition, source legend, and delegation to `configkit.WriteDiagnostics`.
|
||
Its scope and comparison-side labels can be supplied as the heading; they do not
|
||
require another shared options type. See
|
||
[ckconfig's adapter](../../../integrations/ckconfig/configkit_adapter.go) and
|
||
[ckprompt's adapter](../../../integrations/ckprompt/adapters.go).
|
||
|
||
**Recommendation:** expose the existing adaptation responsibility through one
|
||
narrow ckconfig function and have both integrations use it. An illustrative
|
||
signature, not an implemented API, is:
|
||
|
||
```go
|
||
func AdaptDiagnostics(
|
||
heading string,
|
||
sources []configkit.SourceInfo,
|
||
records []configkit.Diagnostic,
|
||
) ([]commandkit.Diagnostic, commandkit.Explanation)
|
||
```
|
||
|
||
Retain the existing evidence snapshot and severity treatment, ordering within
|
||
each group, source labels, and native record formatting. Return evidence ready
|
||
for CLI reporting; do not print immediately, select an invocation outcome, or
|
||
load or validate configuration. Callers supply headings and operation-specific
|
||
explanations and retain their error causes. Reporter remains responsible for
|
||
visibility and delivery.
|
||
|
||
Keep scheduling with the callers. The smaller scheduling switches do not
|
||
independently justify another public helper. ckprompt already depends on
|
||
ckconfig; this introduces no new module dependency or ConfigKit dependency in
|
||
core. The public function serves CLI presentation, within the integration's
|
||
ownership boundary. Compatibility and migration are not obstacles; clarity and
|
||
maintenance cost are the decision criteria.
|
||
|
||
**Decision:** recommendation and illustrative API pending agreement.
|
||
|
||
## F16 — Shared flag consumption
|
||
|
||
**Question:** which consumption facts can be shared while execution and
|
||
completion retain their different handling of incomplete input?
|
||
|
||
**Evidence:** [catalog F16](catalog.md#f16--p3-parsing-and-completion-duplicate-flag-consumption-grammar).
|
||
|
||
**Options to examine:** a small neutral consumption primitive or deliberate
|
||
retention of duplication where a shared helper needs extensive caller modes.
|
||
|
||
**Preliminary recommendation:** share mechanics only; keep normalization,
|
||
failure provenance, and partial-word completion in their current owners. Repair
|
||
F06's completion fixture before validating this refactor. Review a concrete
|
||
boundary before selecting an abstraction.
|
||
|
||
**Decision:** pending boundary analysis and agreement.
|
||
|
||
## F14–F15 — Joint source-prefix design
|
||
|
||
**Question:** how can discovery preparation and help lookup reuse one source
|
||
structure without parallel indexes or changed routing semantics?
|
||
|
||
**Evidence:** [catalog F14](catalog.md#f14--p3-discovery-context-preparation-repeatedly-rebuilds-shared-prefixes)
|
||
and [F15](catalog.md#f15--p3-alias-help-scans-the-entire-source-catalog-for-each-intermediate-route).
|
||
|
||
**Options to examine:** traversal and descendant lookup through the existing
|
||
alias source structure, or one shared private index if the existing structure
|
||
cannot serve both operations simply.
|
||
|
||
**Preliminary recommendation:** prefer reuse; preserve classification,
|
||
authoritative resolution, deduplication, and ordering. Select the concrete
|
||
traversal and ownership together. No performance test is required to justify
|
||
remediation; correctness validation remains appropriate.
|
||
|
||
**Decision:** joint remediation selected in the catalog; concrete design pending.
|
||
|
||
## F17–F20 — Proportionate private extractions
|
||
|
||
**Question:** which proposed extractions make responsibilities clearer without
|
||
adding records, phases, or configurable mechanisms merely to shorten functions?
|
||
|
||
**Evidence:** catalog entries [F17](catalog.md#f17--p3-configuration-scope-capture-combines-several-compilation-phases),
|
||
[F18](catalog.md#f18--p3-configuration-preflight-interleaves-shared-and-operation-specific-rules),
|
||
[F19](catalog.md#f19--p3-typed-input-preparation-mixes-conversion-provenance-and-replay-assembly),
|
||
and [F20](catalog.md#f20--p3-reporter-message-paths-duplicate-detail-rendering-bookkeeping).
|
||
|
||
**Options to examine:** cohesive functions over existing private types, or
|
||
retaining related code together where extraction only adds indirection.
|
||
|
||
**Preliminary recommendation:** review each extraction separately. F20 has a
|
||
concrete shared rendering/error policy; keep its refactor distinct from F03's
|
||
correctness repair. For F17–F19, make meaningful responsibilities and required
|
||
ordering visible without treating incidental private sequences as contracts.
|
||
|
||
**Decision:** individual extraction choices pending review.
|