commandkit/docs/roadmap/audit/remediation-analysis.md

# Audit remediation analysis

Status: remediation decisions agreed; ready for implementation planning. No
implementation changes made.

This document records analysis of the remediation choices reviewed 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–F21 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? | Decided: shared public ckconfig helper; remediation pending |
| 3 | F16 | Which flag-consumption facts can parsing and completion usefully share? | Decided: shared internal mechanics, separate caller policies; remediation pending |
| 4 | F14–F15 | How should both operations reuse source-prefix processing? | Decided: reuse existing resolver tree; remediation pending |
| 5 | F17–F20 | Which private extractions materially simplify the existing code? | Decided: bounded private extractions, conservative F19; remediation 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 considered:** 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:** shared public ckconfig adaptation helper accepted and recorded in
the catalog, using the input/output shape above. Both integrations will use it;
the smaller scheduling switches remain with their callers. The function name
above is illustrative; implementation remains pending.

## 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 considered:** a small neutral consumption primitive or deliberate
retention of duplication where a shared helper needs extensive caller modes.

**Analysis:** execution's `interpretFlagTokens` in
[internal/cobra/flags.go](../../../internal/cobra/flags.go) and completion's
`walkCompletionFlags` and `completionOptionTarget` in
[completion_flags.go](../../../completion_flags.go) repeat the decisions about
short-flag clusters, attached values, boolean assignments, and whether a flag
consumes the next argument. They already share classification and lookup through
[commandmeta's flag helpers](../../../internal/commandmeta/flags.go).

Both paths need to recognize that `--profile=team` has an attached value and
`--profile team` takes the following argument. Their responses to unfinished
input differ: executing a bare `--profile` requires a missing-value error, while
completion after `--profile ` should offer values. Likewise, an unfinished
`--pro` can be useful for suggesting flag names without being a valid execution
argument. A single policy-heavy parser would obscure that difference.

**Recommendation:** extend the existing neutral internal helpers with a small
consumption operation. Given flag metadata and the current spelling/position,
it should expose facts such as the identified flag, any attached value, whether
the next argument is required, and where to continue within a short cluster.
Distinguish an explicitly empty attached value from an absent value. The exact
private signature is an implementation choice, not a new public contract.

Execution retains alias normalization, entered-spelling and occurrence tracking,
backend error handling, and typed conversion. Completion retains unfinished-word
handling, suggestion filtering, and insertion text. Existing token classification
continues to identify `--`; callers retain their appropriate end-of-options
handling. Supply each caller's existing lookup rather than extending completion
to new alias-discovery behavior. Do not add execution/completion mode flags,
callback registries, another public parser, or a replacement for Cobra/pflag.

Repair F06's fixture before validating the change. Use focused behavioral
coverage for shared grammar and the deliberately different missing/partial-value
paths, preserving explicit empties, short clusters, and entered spelling.
If a proposed extraction needs extensive caller-specific controls, narrow it
and retain the remaining duplication rather than forcing complete unification.

**Decision:** the recommended internal sharing boundary is accepted and recorded
in the catalog. Execution and completion retain their separate policies; exact
private signatures remain implementation choices. No refactor is implemented.

## 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 considered:** traversal and descendant lookup through the existing
alias source structure, or one shared private index if the existing structure
cannot serve both operations simply.

**Analysis:** the existing [alias resolver](../../../internal/alias/resolver.go)
already builds a source-path tree. Each node represents one distinct prefix,
has child links, and identifies whether a mapping ends there. Shared source
prefixes therefore already have a single structural representation.

F14's `discoverFlagContexts` in [flag_aliases.go](../../../flag_aliases.go)
instead walks each declaration independently, repeatedly copying and classifying
common prefixes before deduplicating accepted results. F15's `routeHelpAliases`
in [help_view.go](../../../help_view.go) scans the full alias catalog at every
intermediate rewrite path, even when that prefix has no descendants.

For example, sources `legacy team show` and `legacy team check` share `legacy`
and `legacy team`. Setup can examine each shared prefix once. Help at
`legacy team` can find its descendants through that tree branch without scanning
unrelated source paths.

**Recommendation:** add narrow internal read-only operations on the existing
resolver tree for these two needs:

1. Enumerate each distinct nonempty proper source prefix once for F14. Apply
   the existing discovery classification once per candidate, retaining exclusions
   for canonical paths, exact alias sources, and valid canonical operand space.
2. Find strictly descendant alias sources beneath a supplied prefix for F15.
   Continue visiting the existing intermediate resolution paths. Resolve each
   composed entered path through the authoritative resolver, then retain current
   canonical-target checks, suffix deduplication, and output sorting.

Keep trie nodes private and preserve captured path ownership. Reuse the tree's
existing routing-snapshot lifetime rather than creating a parallel index or a
cross-invocation result cache. Root help can continue using the complete catalog
when it actually needs all entries. No public API or routing grammar change is
needed; internal method names and return shapes remain implementation choices.

This removes the identified repeated scans and preparation, without claiming
that all routing or installation costs become linear. Test the preserved
discovery/help behavior, including shared prefixes, chains, longer overrides,
and updated registration where relevant. As already decided, performance testing
is not a prerequisite for remediation.

**Decision:** reuse of the existing resolver tree through the two internal
operations above is accepted and recorded in the catalog. Preserve existing
discovery and routing semantics; no parallel index, new cache lifecycle, public
API change, or performance-testing prerequisite is selected. Implementation
remains 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 considered:** cohesive functions over existing private types, or
retaining related code together where extraction only adds indirection.

These are maintenance improvements, not newly established incorrect CLI
outcomes. Recommendations below use existing private types and ordinary
functions. No public API, generic pipeline, or additional validation policy is
needed. Keep related work together; a lower function-length or complexity score
is not itself a completion criterion.

### F17 — Separate configuration suite setup responsibilities

**Analysis:** `captureSuite` in [family.go](../../../integrations/ckconfig/family.go)
captures scope and selector declarations, validates provider wiring, attaches
selectors to scopes, determines command availability, and builds discovery
metadata. Maintaining one of those responsibilities requires understanding
several partially populated collections.

**Recommendation:** extract cohesive scope capture/validation and selector
attachment responsibilities, then make operation bindings and discovery
inventory construction explicit. Keep related loops together and use the
existing prepared-suite types. Retain declaration ownership, accumulated
errors and their order, binding identity, and metadata-only construction.
Do not load configuration or invoke application factories during this setup.

The benefit is making it clear where a new scope or selector rule belongs.
Exact helper boundaries can follow the implementation; do not create a helper
for each loop or new intermediate records merely to divide the function.

### F18 — Separate ordinary configuration preparation from comparison

**Analysis:** `prepareConfigOperation` in
[input.go](../../../integrations/ckconfig/input.go) handles command-specific flag
rules, scope/capability checks, path parsing, and both ordinary and two-sided
input preparation. The command builder also mixes several operation-specific
flag groups into its declarations.

**Recommendation:** isolate operation-specific option checks, shared scope and
capability selection, and comparison input preparation with small private
functions. Keep an explicit orchestration function showing the actual order;
do not replace its operation branches with a rule registry. Extract flag groups
only where that makes their purpose clearer.

Preserve bare explain's help path, aggregate capability checking before input
construction, independent left/right comparison selections, and completion of
input checks before shared state creation and acquisition. This makes it easier
to change one command without reasoning through every other command's submode.

### F19 — Keep input-ordering machinery together

**Analysis:** `prepareInputWithOccurrences` in [binding.go](../../../binding.go)
combines value conversion, error attribution, occurrence-position assignment,
required checks, operand allocation, and assembly of later input setters.
Captured binding replay relies on the same occurrence identities.

**Recommendation:** start with small extractions for conversion-error
attribution and operand preparation. Keep flag preparation, occurrence identity,
and order reconstruction together as one coherent block; that block may remain
in the orchestration function if moving it requires excessive plumbing.

For example, a conversion error should still identify the spelling the operator
actually used, and repeated/interleaved inputs should still apply in their
selected order. Preserve supplied-state distinctions, original error identity,
and conversion before application input factories run. Retain the existing
dense reconstruction and captured-replay merge algorithms. This is the most
conservative extraction recommendation of the four; introducing new state
containers or a replay framework would defeat its purpose.

### F20 — Share detail-rendering error bookkeeping

**Analysis:** `renderTerminalLocked` and `submitMessageLocked` in
[reporter.go](../../../reporter.go) both invoke detail callbacks through managed
writers, distinguish rendering failures from destination write failures, and
retain the first error of each kind. Their headings and visibility rules differ.

**Recommendation:** use one private detail-rendering helper under the existing
reporting lock, returning rendering and writing errors separately. Callers keep
headings, visibility/discard selection, destination delivery observations, and
terminal timing. Preserve callback order, hidden-detail execution, sticky writer
errors, and callback non-reentrancy; add no locks or interchangeable renderers.

This is the clearest duplication removal of the four. Keep it separately
reviewable from F03's deferred-acceptance fix and F07's writer-reference release;
it does not itself repair either defect.

**Recommendation on sequencing:** correctness fixes remain first. Among these
maintenance changes, favor F20, then F18 and F17, with F19 kept small. Evaluate
each resulting diff by whether responsibilities are easier to follow and
existing behavioral coverage still protects the selected contracts. Tests should
not freeze new helper boundaries or incidental private call order.

**Decision:** all four bounded extraction recommendations and the sequencing
guidance above are accepted and recorded in the catalog. F19 remains deliberately
conservative. No refactoring has been implemented.

## Readiness for implementation planning

The discussion sequence is complete. No unresolved product or architectural
choice identified in this review blocks preparation of the implementation plan.
The catalog's 27 findings remain open until their remedies are implemented and
validated.

The plan can assign work units, dependencies, and proportionate verification.
Routine details such as private helper signatures, the public diagnostic
helper's final name, and the completion fixture's replacement flag are ordinary
implementation choices within the accepted scope. Preserve the recorded
boundaries; revisit a decision only if concrete implementation evidence exposes
a conflict or a materially different tradeoff.

Keep F14–F15 together, repair F06 before validating F16, and coordinate the
reporter work in F03, F07, F20, and the relevant parts of F24 without obscuring
their separate purposes. Validation should distinguish the aligned workspace
from independent modules. No implementation plan or production changes are
included in this analysis update.