Record the implementation review findings

docs/roadmap/remediation.md

@@ -0,0 +1,258 @@
+# Implementation review remediation
+
+Reviewed on 2026-10-11, against committed implementation
+`cc574915ee17e2d738f27874d2c8857aa1f51a47`. The implementation sprint starts
+after planning commit `0d23676144ab1ce5047924e98f1ab6a2a9d6aeb6`.
+All issues below are open. 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](../policy/architecture.md), 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:** Open. **Priority:** P1.
+
+**Evidence:** [ckprompt/command.go](../../integrations/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:
+
+```text
+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](../adr/0011-captured-declarations-and-installation.md#uniform-captured-declarations):
+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:** Open. **Priority:** P2.
+
+**Evidence:** [ckconfig/input.go](../../integrations/ckconfig/input.go),
+`newConfigOperationCommand`, `selectorIndexesForOperation`, and
+`prepareConfigOperation`; [ckconfig/family.go](../../integrations/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](https://gitea.maximumdirect.net/eric/CommandKit/src/commit/0d23676144ab1ce5047924e98f1ab6a2a9d6aeb6/docs/roadmap/registration-backend.md#integration-preparation-without-parallel-execution-frameworks)
+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:** Open. **Priority:** P2.
+
+**Evidence and corrections:**
+
+- [Consumer guide](../consumers/guide.md#promptkit-configuration-and-definitions)
+  says each inspection gets a fresh input, while
+  [ckconfig's factory comments](../../integrations/ckconfig/contracts.go) 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](../../integrations/ckprompt/README.md)
+  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](../cli.md#promptkit-definition-inspection)
+  already distinguishes application acquisition/direct callback effects.
+- The [composed example README](../../integrations/ckprompt/examples/composed/README.md)
+  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](../../integrations/ckconfig/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](../adr/0011-captured-declarations-and-installation.md#separate-typed-input-from-shared-acquisition-state)
+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:** Open. **Priority:** P2.
+
+**Evidence:** [release policy](../policy/release.md#release-documentation-and-preparation)
+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](../release.md#modules-versions-notes-and-shared-files),
+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:** Open. **Priority:** P2.
+
+**Evidence:** [release.md](../release.md#modules-versions-notes-and-shared-files)
+lists module-local files as the integrations' shared project files and says
+integration publication validates those matching files. In contrast,
+all three [profiles](../../releasekit.json), including
+[ckconfig](../../integrations/ckconfig/releasekit.json) and
+[ckprompt](../../integrations/ckprompt/releasekit.json), 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](https://gitea.maximumdirect.net/eric/ReleaseKit/src/commit/b96262eb0ce9c8f2eb3e948a5841db2b799bc91b/docs/config.md),
+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:** Open. **Priority:** P3.
+
+**Evidence:** [command.go](../../command.go) retains
+`flagBinding.clear`, and [binding.go](../../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:** Open. **Priority:** P3.
+
+**Evidence:** [ckprompt attribution](../../integrations/ckprompt/docs/oss/attribution.md)
+labels ConfigKit as a transitive dependency, but
+[adapters.go](../../integrations/ckprompt/adapters.go) imports it directly and
+[go.mod](../../integrations/ckprompt/go.mod) lists it as a direct requirement.
+[ckconfig attribution](../../integrations/ckconfig/docs/oss/attribution.md)
+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](../policy/oss.md) 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.