Files
commandkit/docs/roadmap/remediation.md

18 KiB

Implementation review remediation

Reviewed on 2026-10-11, against committed implementation cc574915ee17e2d738f27874d2c8857aa1f51a47. The implementation sprint starts after planning commit 0d23676144ab1ce5047924e98f1ab6a2a9d6aeb6. Completion status and evidence are recorded per issue below. P1 identifies a functional contract violation; P2 identifies architecture or documentation corrections; P3 identifies smaller maintenance corrections. These priorities do not add publication gates.

Review scope and evidence

The review compared the current code and documentation with the architecture policy, accepted ADRs, and the selected API/backend designs retained in Git history. The three-module layout, dependency direction, captured core declarations, batch installation, typed runners, neutral metadata/Cobra boundary, invocation state separation, native reports, and selected-module release launcher are substantially in place.

The review ran the existing deterministic suites for core, ckconfig, and ckprompt using Go 1.26.8, the existing local workspace, and disabled Go network access; all passed. Independent workspace-off package/test and module graph checks also passed, using disposable alternate module files and local replacements only for the unpublished CommandKit dependencies. Core excludes ConfigKit/PromptKit and both integrations; ckconfig excludes PromptKit and ckprompt. Those checks establish local wiring/isolation, not published-version availability.

A disposable public-API program reproduced R1. A separate case confirmed that omitted pointer input/state factories deliver nil zero values successfully, supporting the distinction in R3. No implementation files were changed, no regression tests were added, and no publication was attempted. These are focused findings, not a claim that passing tests prove the absence of other defects.

Keep the accepted scope intact during remediation. In particular, preserve caller-owned callback effects and objects, optional capabilities, unused-binding behavior, supported error identity, native reports, and source-only release completion. Do not introduce deep cloning, new report fields, public helpers, provider caches, or permanent validation gates to resolve these issues.

R1: Capture PromptKit selector handles before retaining the suite

Status: Completed (2026-10-11). Priority: P1.

Completed work: the enabled family copies its selector-handle collection once, preserving original identities for both generated commands and retained callbacks. Empty-suite behavior is unchanged. The public consumer regression replaces a source element after capture and exercises prompt/profile inspection in two apps reusing the declaration. It failed before the fix and passes after it; the complete ckprompt suite passes offline with Go 1.26.8.

Evidence: ckprompt/command.go, Command, newDefinitionCommand, and prepareDefinitionInput. CaptureFlags captures the original handles for parsing, but the execution closures retain a shallow copy of Suite whose Selectors slice still shares the caller's backing array. prepareDefinitionInput subsequently reads that array when choosing handles for ApplyTo.

Construct a suite with one campaign flag, call ckprompt.Command and install it, then replace the original selector slice's element with a separately constructed flag having the same name. The installed parser still captures the original handle, while execution selects the replacement. The public-API reproduction produced:

Before source edit: success, status 0, provider called.
After source edit: operational, status 1, ErrFlagWiring, provider not called.
Cause: flag --campaign was not captured.

This violates ADR 0011's capture decision: later edits to source metadata must not change an already captured declaration.

Required work: copy the enabled suite's selector-handle collection once at family construction and use that captured collection consistently for generated bindings, input application, and retained callbacks. Preserve original binding identity; do not reconstruct lookalike flags or clone callback objects. Keep the empty-suite no-op and retained-error behavior.

Completion evidence: public consumer regression coverage for source selector element edits after capture, both named command capabilities, and reuse across apps. The original selection and values must remain effective after the edit. Avoid assertions about a particular private representation.

R2: Finish declaration-time compilation of configuration metadata

Status: Completed (2026-10-11). Priority: P2.

Completed work: family capture orders scope references and prepares each scope's original binding list, each operation's binding union, and scope-choice text. Inspection preparation reads those lists without static sorting or membership compilation. A behavioral regression covers operation-specific flags and capture of capabilities/applicability/handles after source edits. Existing ordering, metadata-only, unused/required-selector, isolated-input, and shared-state coverage passes; all three module suites pass offline with Go 1.26.8. No provider cache or performance guarantee was introduced.

Evidence: ckconfig/input.go, newConfigOperationCommand, selectorIndexesForOperation, and prepareConfigOperation; ckconfig/family.go, captureSuite and orderedScopeRecords.

Operation-specific selector membership is computed at command construction and then recomputed for every inspection invocation. Preparation scans selector/scope capabilities again, builds a membership map, and assembles per-scope binding lists. An aggregate request also copies and sorts scope records again. These inputs are fixed captured metadata, not invocation-dependent configuration.

The selected backend design calls for ordered scope records and applicable original selector bindings to be prepared at family construction. The current code correctly shares one invocation plan between checking and running, but leaves this static compilation incomplete. No latency or allocation measurements are claimed here.

Required work: capture ordered scope references and each operation's relevant selector handles/per-scope binding lists once. Invocation preparation should resolve the requested selection, perform availability/input checks, and apply those lists. Keep this as ordinary private metadata, without a cache framework, bitsets, process-global state, or a provider-acquisition cache.

Completion evidence: inspect the invocation path for absence of repeated static sorting/membership compilation. Retain behavioral coverage for ordering, partial capabilities, metadata-only scopes, unused selectors, required selected flags, input isolation, and shared state. Do not add a performance threshold or require callbacks to run after failure.

R3: Make public documentation precise about ownership and the final API

Status: Completed (2026-10-11). Priority: P2.

Completed work: the suites' GoDoc now specifies initialization timing, omitted-factory zero values (including nil pointers), early-failure limits, and caller ownership of objects, aliases, and effects. Family GoDoc and consumer guidance link to those owners. Inspection claims are qualified, the composed example describes NewInput/NewState, and Scope.Summary no longer promises discovery output. Reviewed against current preparation/acquisition code and the example; current consumer/example Markdown has no obsolete context API names. Changed local links/fragments and whitespace checks pass. Runtime rules and report fields are unchanged.

Evidence and corrections:

  • Consumer guide says each inspection gets a fresh input, while ckconfig's factory comments describe fresh storage without documenting omitted factories and caller-owned results. State the selected initialization timing and zero-value behavior, including nil pointers. Distinguish separate initialization calls from a guarantee that caller factories return distinct objects.
  • The consumer guide and ckprompt README make unqualified statements that inspection does not execute prompts, contact providers, or resolve credentials. Limit these statements to CommandKit-owned inspection code. The CLI reference already distinguishes application acquisition/direct callback effects.
  • The composed example README still refers to ConfigSuite.NewContext and shared scope contexts. Its implementation uses Suite[I, S], separate NewInput initialization, and NewState for the shared cache. Replace the obsolete description.
  • Scope.Summary's GoDoc in contracts.go says it is shown when a scope is listed. Current discovery renders name and operations only, and does not use the summary. Correct the comment without inventing a report-field requirement or changing the selected discovery format.

ADR 0011 explicitly leaves factory objects, aliases, and callback effects with the caller. The pointer-zero-value reproduction succeeds today; the runtime must not be changed to require allocation or distinct pointers merely to match stronger documentation.

Required work: put exact lifecycle/ownership contracts in the owning exported declarations and link to them from task guidance. Qualify broader descriptions accordingly. Preserve arbitrary caller object ownership and the absence of automatic deep copies or concurrency promises.

Completion evidence: review the updated GoDoc, guide, and example descriptions against real code, including omitted factories, intentionally shared objects, early failure, and caller-owned effects. Check for remaining obsolete public API names. This is documentation work, not a request for additional runtime rules.

R4: Complete the module-specific release-note policy adaptation

Status: Completed (2026-10-11). Priority: P2.

Completed work: release policy now places notes relative to the selected module and distinguishes semantic-version filenames/titles from full Git tags and immutable source URLs. Checked all three prepared notes against their profile products, versions, full-tag footers, and existing module-local destinations; they agree with the runbook and ReleaseKit v0.4.0's public module-release ADR. The added local link/fragment and whitespace checks pass. Independent versioning, immutability, and source-push completion rules remain intact; no tags were created.

Evidence: release policy still requires docs/releases/<version>.md in the tagged source tree and says to use the complete version tag as the filename. The release procedure, module notes, and ReleaseKit v0.4.0 correctly use module-local note directories and a semantic-version filename. A nested full tag contains a module prefix; it is not that filename.

For example, ckconfig's prepared note is integrations/ckconfig/docs/releases/v0.1.0.md, with full tag integrations/ckconfig/v0.1.0. The generic policy wording does not describe that selected arrangement. Updating only the policy's release-scope paragraph left the controlling note requirement ambiguous/conflicting.

Required work: adapt the CommandKit release-note rule to the selected module directory and semantic version; distinguish semantic version, full Git tag, product title, note path, and immutable-link identity. Preserve immutability, independent versioning, and the successful-source-publication completion point. No accepted ADR decision text needs revision.

Completion evidence: the policy, runbook, three prepared notes, and released ReleaseKit contract agree for root and nested modules. This requires document review, not real tag creation or publication.

R5: Describe actual release file inputs and enabled checks

Status: Completed (2026-10-11). Priority: P2.

Completed work: the runbook names the common explicit repository-root shared-file list, separates module-local distribution notices, and distinguishes existence requirements from optional content checks. It describes the enabled suite/candidate/publication checks without promising automated licensing checks. Compared all three unchanged profiles with ReleaseKit v0.4.0's public field semantics and confirmed the listed files and local links/fragments exist. Whitespace checks pass. No additional release gates were enabled.

Evidence: release.md lists module-local files as the integrations' shared project files and says integration publication validates those matching files. In contrast, all three profiles, including ckconfig and ckprompt, explicitly select the repository-root COPYRIGHT, LICENSE, and docs/oss/attribution.md.

The profiles enable no optional file checks. Under the released ReleaseKit profile contract, an explicit shared file must exist even with no file checks enabled, but naming it does not enable content checks. Module-local files belong to automatic module inputs when applicable; that does not itself establish a license/content validation guarantee. The current runbook conflates these distinctions.

Required work: document the selected root shared-file list accurately, explain the separate module-local distribution materials, and state what the currently enabled validation actually checks. Preserve the selected profiles and one standard suite per selected module. Do not enable additional gates to make the overstatement true.

Completion evidence: compare the runbook with all three JSON profiles and v0.4.0's public field semantics. Module-local licensing obligations remain intact without an invented automated content-check promise.

R6: Remove the obsolete binding-clear mechanism

Status: Completed (2026-10-11). Priority: P3.

Completed work: verified the field had only constructor assignments and no callers, then removed it and the three unused closures. The removal leaves prepared value application, defaults, presence, and binding identity untouched. A follow-up source search finds no remaining binding-clear mechanism. All three module suites pass offline with Go 1.26.8; formatting and whitespace checks pass. No reset hook or public API change was introduced.

Evidence: command.go retains flagBinding.clear, and binding.go constructs clear closures for singleton, boolean, and repeatable flags. The old selector projection code that consumed these closures was removed; current production code does not read or call the field.

This is prototype machinery left behind by the input/state refactor. It adds unnecessary binding state and closure construction while suggesting that clearing non-applicable fields is still part of the implementation.

Required work: remove the unused field and closure construction. Keep prepared value application, defaults, presence, and original binding identity unchanged. Do not replace it with a new reset hook or clear initialized inapplicable fields.

Completion evidence: verify there are no callers before removal, then run the affected existing suites. Public behavior and exports remain unchanged.

R7: Correct the scope descriptions in module-local attribution

Status: Completed (2026-10-11). Priority: P3.

Completed work: ConfigKit is identified as a direct ckprompt dependency. Both integration notices now identify copied policy entries as core-dependency material with repository-root paths. Checked the direct import/requirement and the material's actual locations. Local license links pass; all other copyright, license, notice, and pinned source wording is preserved. Grants and verbatim license files are byte-for-byte unchanged, and whitespace checks pass.

Evidence: ckprompt attribution labels ConfigKit as a transitive dependency, but adapters.go imports it directly and go.mod lists it as a direct requirement. ckconfig attribution also reproduces the root policy-material entry, identifying docs/policy/*, AGENTS.md, and docs/development.md without identifying their root/core scope. Those files are outside the ckconfig module distribution.

The OSS policy requires factual component/material scope; the implementation target requires module-local notices relevant to actual distributed materials and dependencies.

Required work: correct the direct-dependency description. Review the copied policy-material entry against the module distribution: identify it explicitly as root/core dependency material if retained, or omit it if inapplicable to that notice's scope. Preserve applicable attribution, notices, grants, and verbatim license texts; do not infer new licensing obligations or relicense code.

Completion evidence: module-local attribution describes its actual imports, dependency roles, and material locations accurately, with working local license links. Repository-wide provenance remains owned by the root notices/policy provenance documents.