Record required documentation reconciliation work

This commit is contained in:
2026-10-07 17:06:10 +00:00
parent cb0b8dd12b
commit 2af1355b78

View File

@@ -1,338 +1,220 @@
# Initial implementation review and remediation # Documentation reconciliation work
Status: all ten findings addressed. Status: required updates recorded; reconciliation pending.
Date: 2026-10-07. Date: 2026-10-07.
## Review baseline and scope ## Purpose and authority
Reviewed committed implementation at `85f44cd`, including the changes since This checklist replaces the completed code-remediation record with the
planning baseline `e5c361d`. The review covered production code, public contracts, documentation update work agreed after the documentation audit. Use the current
the internal adapter, integration helpers, maintained examples, and existing implementation, accepted ADRs, and [architecture policy](../policy/architecture.md)
behavioral tests against the accepted ADRs, architecture policy, and selected to reconcile stale status statements and superseded plans. The
[initial target](initial.md) and [implementation plan](implementation.md). [documentation policy](../policy/documentation.md) assigns canonical owners.
These are static review findings. No tests were added or executed during this Retain completed plans as clearly labeled historical records, preserve genuine
review; prior completion notes are historical evidence, not a new validation deferrals, and direct implemented contracts to their current owners. Exact Go
result. The triggering cases below describe those baseline code paths. The remediation contracts belong in declarations and GoDoc; operator contracts belong in the
progress and validation evidence record the subsequent fixes. [CLI reference](../cli.md); package inventory belongs in the
[internal overview](../internal/overview.md). Roadmaps retain planning history,
The findings below describe the review baseline. Completion is tracked here as implementation status, and future work. Historical proposals must not read as
implementation fixes land. current requirements.
## Remediation progress Check each item when its documentation changes and review are complete.
- R-01 complete: required selectors are checked after scope selection; unrelated ## D-01: reconcile the CLI proposal
scopes remain usable, and help describes conditional requiredness.
Owner: [01-cli-plan.md](01-cli-plan.md).
- R-02 complete: inline document starts are recognized and native content is
retained, including block scalars. - [ ] Replace the status claiming that no commands are implemented. Identify
implemented operations, superseded proposals, and deferred features.
- R-03 complete: standard inspection handlers check cancellation before runtime - [ ] Reconcile configuration scope defaults: every implemented configuration
construction/acquisition, after acquisition, and before prepared report delivery. operation defaults to `app`; aggregate validation/display requires explicit
`--scope all`. Retire the recommendation and rationale for validation defaulting
- R-04 complete: every help entry form finalizes report failures through the to `all`.
common diagnostic presentation path once. - [ ] Replace singleton-repetition rejection with the selected last-occurrence
rule, including conversion of every supplied occurrence. Remove repetition
- R-05 complete: validation header/record write failures retain `DeliveryError` from the list of usage failures and label the old alternative as superseded.
separately from acquisition failures and combined diagnostic-stream failures. - [ ] Replace obsolete `--prompt-version` examples and descriptions with the
implemented `--version` spelling.
- R-06 complete: zero outcomes normalize to success/0 and empty delivery state - [ ] Retire the upstream redaction prerequisite for rich profile reports.
normalizes to none without altering explicit domain outcomes. Record that effective `ExtraParams` is included using the current public
PromptKit API, without introducing additional redaction requirements.
- R-07 complete: nil diagnostic writers are no-output entries, do not count as - [ ] Remove CommandKit mandates for application root paths, rejection of empty
delivery attempts, and cannot suppress cause presentation. config values, overlay interpretation, and propagation from one application
root into PromptKit. Values and presence reach application callbacks; those
- R-08 complete: unknown scopes and absent default app scopes report available callbacks own their interpretation and acquisition paths.
choices in standard scope order without runtime acquisition. - [ ] Reconcile aggregate validation output: completed scope records may already
have been written when a later acquisition or output fails. Retire the claim
- R-09 complete: PromptKit selector collisions are checked against installed that failure always produces no success report.
standard flags; profile-only `version` and custom `scope` selectors are usable. - [ ] Reconcile native display syntax: `config show --format yaml` is accepted.
Link to the current framing contract rather than maintaining another report
- R-10 complete: help inventories and usage discover all registered children, reference.
including non-runnable metadata-only group chains. - [ ] Retire the proposed scope inventory fields for descriptions/default
indicators and the proposed source-table presentation as current expectations.
### Validation evidence Link to implemented scope/source report shapes in the CLI reference.
- [ ] Remove the claim that root `app --version` is supported. Reconcile identity
Each fix includes a focused regression check and passed `go test ./...` before presentation with the implemented version command, optional text revision, and
its commit. The final combined suite also passed with caching disabled using explicit YAML fields.
`go test -count=1 ./...`. Go formatting, diff whitespace, and affected local - [ ] Distinguish a missing definition ID operand from an explicitly blank ID:
Markdown links were checked. No live services or credentials were used. missing is usage failure; blank values reach the owning inspection API and
produce operational results.
| Findings | Regression evidence | - [ ] Remove promises of automatic shared dependency reuse or mandatory
| --- | --- | acquisition coordination. Describe application-composed loading and optional
| R-01, R-08 | [Configuration selection tests](../../config_test.go): scope-specific requiredness, help, unknown/default absent scopes, and callback avoidance. | invocation-local caching as consumer integration patterns.
| R-02 | [Native framing tests](../../show_test.go) and existing aggregate show tests in [configkit_test.go](../../configkit_test.go). | - [ ] Keep diff and PromptKit catalogs explicitly deferred. Retire completed
| R-03 | [Cancellation tests](../../cancellation_test.go): all four configuration operations, both definition operations, pre-acquisition cancellation, cancellation during successful acquisition, and completed aggregate validation prefix/diagnostics. | dispatch/report implementation tasks and the obsolete profile-report upstream
| R-04, R-06, R-07, R-10 | [Public invocation tests](../../execute_external_test.go): every help entry form, independent writer errors, outcome defaults/domain preservation, no-output diagnostics, and metadata-only group discovery. | dependency. Keep consumer migrations separate from library implementation.
| R-05 | [Validation tests](../../configkit_test.go): header/record failures, short writes, completed prefixes, independent acquisition errors, and combined stream failures. | - [ ] Label superseded alternatives and dated sibling-checkout evidence clearly.
| R-09 | [PromptKit tests](../../promptkit_test.go): profile-only version selector, custom scope selector, typed delivery, and rejection of actual installed flag collisions. | Replace duplicated current contracts with concise summaries and canonical links.
## Architecture assessment ## D-02: reconcile dispatcher planning
The main architecture is implemented: Owner: [02-dispatch.md](02-dispatch.md).
- The root `commandkit` package owns public named types, opaque handles, - [ ] Remove the pending status and claims that the registry, typed inputs,
per-instance registration, typed bindings, outcomes, standard handlers, scope constructors, binding/group APIs, handler signatures, and outcomes are absent
assembly, and ConfigKit/PromptKit integration. or unresolved.
- `internal/cobra` owns Cobra/pflag operations behind a neutral internal bridge. - [ ] Retire completed API design, implementation sequence, and evidence tasks.
Public signatures do not expose the backend. Each invocation rebuilds backend Clearly label retained illustrations as historical; the public `Supplied`
state; configuration and engine acquisition remain lazy. type is now implemented.
- Standard families own their namespaces. Applications retain custom commands, - [ ] Link current API usage to Go declarations/GoDoc and the
loaders, domain behavior, executable entry points, streams, and process exit. [consumer guide](../consumers/guide.md), and current mechanics to the internal
- Scope capabilities and prompt/profile callbacks can be supplied independently. overview. Preserve application migration work as separate future work.
Native YAML writers and public upstream inspection APIs are used. Profile
reports include `ExtraParams` without additional redaction. ## D-03: reconcile backend planning
- The module uses published dependencies rather than sibling internal packages
or local replacement directives. Simple and composed consumer examples exist. Owner: [03-backend.md](03-backend.md).
No package restructuring or additional implementation framework is recommended. - [ ] Remove claims that the module dependencies, adapter, and invocation behavior
Diff, catalogs, JSON output, consumer migrations, publication tooling, and are unimplemented. Link dependency facts to [go.mod](../../go.mod) and notices to
concurrent-dispatch guarantees are deliberate deferrals, not missing initial [attribution](../oss/attribution.md).
architecture. The issues below affect behavior within the implemented structure. - [ ] Remove the unresolved singleton-rejection discussion and reconcile it with
accepted repetition semantics.
## Findings and priority - [ ] Retire completed version-selection, translation, parsing/help, integration,
and evidence tasks. Link current behavior to its owners; retain backend
P1 means a supported invocation is blocked or its report content is corrupted. replacement and consumer adoption only as future work where applicable.
P2 means a selected behavior or public contract is incomplete or unnecessarily
restricted. The historical findings below have been resolved as recorded above. ## D-04: reconcile facade planning
| ID | Priority | Finding | Review category | Owner: [04-facade.md](04-facade.md).
| --- | --- | --- | --- |
| R-01 | P1 | Required selectors block scopes to which they do not apply | Strengthened requirement; behavior defect | - [ ] Remove the pending status and claims that public declarations,
| R-02 | P1 | Aggregate YAML framing inserts empty documents for inline start markers | Report correctness | implementation packages, the adapter, and dependency versions do not exist.
| R-03 | P2 | Standard inspection handlers omit cancellation boundary checks | Target gap | - [ ] Retire the illustrative `internal/engine` and `internal/command` layout as
| R-04 | P2 | Help shortcuts bypass diagnostic presentation | Target gap; inconsistent delivery | an active implementation target. Link to the actual root and internal adapter
| R-05 | P2 | Validation report failures omit `DeliveryError` | Public outcome contract gap | inventory without creating another maintained package reference.
| R-06 | P2 | Zero outcomes and empty delivery state are not normalized | Target gap | - [ ] Retire completed API/boundary design and documentation tasks. Preserve
| R-07 | P2 | Nil diagnostic writers are rejected despite the documented no-output behavior | Strengthened prohibition; contract inconsistency | legitimate future backend replacement considerations.
| R-08 | P2 | Unknown/default absent scopes omit available choices | Operator contract gap | - [ ] Keep old automated boundary-checker suggestions clearly historical or
| R-09 | P2 | PromptKit selectors reserve flag names absent from installed commands | Strengthened prohibition | optional; do not turn them into a new implementation requirement.
| R-10 | P2 | Parent help omits registered groups containing only help metadata | Discovery gap |
## D-05: close the preparation checklist
### R-01: enforce selector requiredness only where applicable
Owner: [initial-plan.md](initial-plan.md).
**Evidence:** [selectors.go](../../selectors.go), `adaptContextSelector` and
`configInputFlags`; [binding.go](../../binding.go), `prepareInput`. - [ ] Mark roadmap preparation and implementation-plan handoff complete.
- [ ] Remove claims that exact declarations still await specification and that
All configuration operation commands receive every suite selector. Adaptation the staged plan is the next deliverable. Link to the completed plan and current
copies `FlagMeta.Required` unchanged, and generic input preparation checks it API owners.
before scope selection or applicability checks. - [ ] Check the completed final handoff item and label the preparation process
and recommended outline as historical planning material.
With an app-only required `--campaign` selector and app/PromptKit validation
scopes, `config validate --scope promptkit` fails because campaign is missing. ## D-06: clarify completed feature and implementation records
Supplying campaign instead fails because it does not apply to PromptKit. There
is no successful invocation for that independently supported scope. Owners: [initial.md](initial.md) and [implementation.md](implementation.md).
**Authority:** selected scope-specific routing in INT-01 and the initial - [ ] Replace ownership statements written in future tense in the feature roadmap with
roadmap's selector section. A required selector for one scope does not authorize links to the existing GoDoc and CLI reference. Clearly identify detailed
requiring it for every other scope. specifications as the completed design baseline.
- [ ] Label the implementation plan as a completed execution record and explain
**Remediation:** apply selector requiredness after selecting applicable scopes. that subsequent fixes were committed after its original completion.
Keep ordinary custom-command required flags unchanged. Preserve requiredness - [ ] Reconcile the statement that definition commands have no `--scope` flag:
and applicability in help metadata without an unconditional preparation check. they have no built-in configuration-scope selection, but application-declared
context selectors may use that name when it does not collide.
**Completion evidence:** an unrelated scope succeeds without the selector; an - [ ] Preserve the explicitly labeled “At planning time” baseline as historical
applicable scope fails before acquisition when it is missing; `all` enforces context. Keep diff/catalog and other genuine deferrals visible; completed
the requirements of its selected scopes; help remains available. design records must not become competing current API/CLI references.
### R-02: recognize inline YAML document-start markers ## D-07: correct the current CLI reference
**Evidence:** [show.go](../../show.go), `frameEffectiveYAML`. Owner: [cli.md](../cli.md).
The framer recognizes only a line equal to `---` or starting with `--- #`. - [ ] Correct aggregate YAML framing prose: scope comments can precede an inline
A native document such as `--- {setting: value}\n` already has a valid document document-start marker; they are not always after the marker. Preserve the
start, but receives another opening marker and scope comment. The result starts native content and one YAML document per scope.
with an empty document followed by the actual value, so document count and scope - [ ] Qualify engine acquisition/construction wording. Standard handlers invoke
association are wrong even though execution returns success. registered inspection callbacks; the bundled helper acquires an engine through
the application loader. Direct callbacks retain their own implementation and
**Authority:** [ADR 0007](../adr/0007-native-yaml-reports.md) and CFG-03 require lifecycle rather than guaranteeing fresh engine construction.
native YAML preservation and one framed document per selected scope. - [ ] Describe the absence of execution/provider/credential operations as the
behavior of CommandKit's inspection path, preserving application callback
**Remediation:** recognize the document-start token with supported whitespace and ownership of its effects.
inline content. Preserve native bytes; a parallel decode/remarshal pipeline or - [ ] Correct cancellation attribution: cancellation can be detected at
a new prohibition on valid native YAML is unnecessary. CommandKit's standard-handler boundaries as well as returned by PromptKit.
Preserve supported error identity and operational classification.
**Completion evidence:** aggregate decoding yields exactly one value per scope
for inline mapping/scalar starts, ordinary marked documents, unmarked documents, ## D-08: remove stale policy status statements
and directive-bearing documents, while preserving native content.
Owners: [testing.md](../policy/testing.md) and [oss.md](../policy/oss.md).
### R-03: check cancellation at standard inspection boundaries
- [ ] Remove the testing policy's claim that there is currently no Go suite.
**Evidence:** `runConfigValidation`, `runConfigShow`, `runConfigSources`, and Keep concrete validation commands in the developer guide.
`runConfigExplanation` in [validate.go](../../validate.go), - [ ] Remove the OSS policy's claim that Cobra/pflag are not incorporated and
[show.go](../../show.go), [sources.go](../../sources.go), and that their notices still await introduction. Link to existing attribution;
[explain.go](../../explain.go); definition handlers and engine loading in keep version inventories out of the policy.
[promptkit.go](../../promptkit.go). - [ ] Preserve policy rules and upstream provenance while correcting local
implementation status. This work does not add testing or licensing gates.
The handlers pass context to callbacks but do not check `Context.Err()` at their
own boundaries. With an already canceled context and a successful loader that ## D-09: correct the internal bridge description
does not itself inspect context, configuration inspection can acquire and emit a
successful report. Direct PromptKit suite callbacks have the same exposure; Owner: [internal/overview.md](../internal/overview.md).
the engine helper also loads before delegating to context-aware named inspection.
- [ ] Correct the claim that application callbacks cross the adapter bridge.
**Authority:** INV-01/OUT-01 and implementation Stage 5 explicitly select standard Handler callbacks and typed invocation coordination remain in the root;
handler checks at operation/acquisition boundaries, with supported cancellation the adapter exchanges neutral metadata and parse results.
identity preserved. ADR 0005 classifies cancellation as operational failure. - [ ] Keep inventory/mechanics here and normative responsibility boundaries in
the architecture policy.
**Remediation:** check context at standard acquisition/report boundaries and
between aggregate acquisitions. Retain `context.Canceled`/deadline identity and ## D-10: reconcile navigation and reading guidance
existing diagnostics/partial-report information. This does not require a
goroutine cancellation runner, interruption of callbacks, or new restrictions on Owners: [development.md](../development.md) and [README.md](../../README.md).
application-owned custom command semantics.
- [ ] Route current implementation work to GoDoc, consumer guidance, CLI
**Completion evidence:** cancellation before acquisition and during acquisition reference, internal overview, and applicable ADRs/policies.
returns operational failure with the context cause, and prevents subsequent - [ ] Label the older numbered roadmaps as completed/historical planning rather
standard work at the next boundary. Include an acquisition callback that ignores than the active source of unresolved implementation decisions.
context so the test protects CommandKit's responsibility. - [ ] Update README roadmap labels/navigation to match the reconciled statuses.
Keep genuine future-work navigation discoverable without duplicating contracts.
### R-04: finalize help outcomes through the presentation path
## D-11: remove temporary references from durable ADRs
**Evidence:** [execute.go](../../execute.go), `Execute`; [builtins.go](../../builtins.go),
`renderHelpOutcome`. Owners:
[ADR 0004](../adr/0004-standard-inspection-surface.md),
Root help, non-runnable group help, and `--help`/`-h` return directly from [ADR 0006](../adr/0006-command-grammar.md),
`renderHelpOutcome`. A failed help report write returns an operational outcome [ADR 0007](../adr/0007-native-yaml-reports.md),
but bypasses `present`, so it produces no operational diagnostic and cannot [ADR 0008](../adr/0008-semantic-configuration-diff.md), and
record a simultaneous diagnostic-stream failure. Explicit `help PATH` goes [ADR 0009](../adr/0009-configuration-scopes.md).
through the normal handler finalization and behaves differently.
- [ ] Remove references to documents under `docs/roadmap` from these permanent
**Authority:** ADR 0005's diagnostic channels and INV-01's single presentation path. records, using durable references where appropriate.
- [ ] Preserve accepted decision content and historical context. Link maintenance
**Remediation:** route these outcomes through the same finalization path exactly must not change architectural decisions or manufacture new requirements.
once, preserving existing report failure fields and lazy metadata behavior. - [ ] Confirm that no ADR retains a temporary-roadmap reference after the edits.
**Completion evidence:** all help entry forms have consistent failure delivery; ## D-12: review the reconciled documentation
broken report and diagnostic streams retain both errors without double delivery.
- [ ] Review every changed document against its canonical owner and current code.
### R-05: retain validation report errors in `DeliveryError` Verify command spellings, defaults, outcomes, ownership, and report descriptions
affected by the corrections.
**Evidence:** [validate.go](../../validate.go), `runConfigValidation` and - [ ] Check local links and heading targets, including links affected by retiring
`validationFailure`; [outcome.go](../../outcome.go), `Outcome`. sections. The audit found no unresolved local prose links or heading targets;
preserve that result.
Both validation header and record write failures call `validationFailure`, which - [ ] Check diffs for whitespace, stale references, and claims that delivered
sets cause and incomplete-report state but never sets `DeliveryError`. With a components are absent or superseded choices remain active requirements.
working diagnostic stream, the returned delivery error remains nil despite a Historical baselines and explicitly labeled rejected alternatives remain valid.
failed report write. Other standard report paths record this field. - [ ] Confirm that deferred features are still identified accurately and that
reconciliation introduces no new APIs, commands, guarantees, or consumer policy.
**Remediation:** distinguish report-writing failures from acquisition failures - [ ] Record completion here and commit completed documentation units under the
and retain the former in `DeliveryError`, alongside the operational cause. local commit policy. Use document review for this prose work; do not add tests
Do not misclassify every validation acquisition error as a delivery error. or validation machinery solely to reconcile documentation.
**Completion evidence:** header/record errors and short writes populate both
relevant error fields and retain incomplete-report state. Combined stdout/stderr
failures preserve both errors. Existing validation short-write checks inspect
the report cause but do not check its `DeliveryError`.
### R-06: implement the selected outcome normalization
**Evidence:** [execute.go](../../execute.go), `present`; [outcome.go](../../outcome.go).
A handler returning `Outcome{}` retains an empty kind and delivery state because
`present` returns it unchanged when there are no diagnostics. An explicit domain
outcome with an omitted diagnostic state likewise retains an empty state.
**Authority:** the selected outcome model, specified in implementation Stage 1,
normalizes a zero outcome to success/0 and an empty delivery state to
`DeliveryNone`. The latter is also stated in public GoDoc.
**Remediation:** normalize these selected defaults during invocation coordination.
Preserve explicit domain classification, status, cause, and delivery information;
do not introduce inference of domain meaning from numeric status.
**Completion evidence:** a zero handler outcome returns success/0/none; explicit
domain outcomes retain their values with an omitted delivery state normalized.
### R-07: honor nil diagnostic writers as no-output diagnostics
**Evidence:** [outcome.go](../../outcome.go), `Diagnostic` GoDoc;
[execute.go](../../execute.go), `present` and `hasErrorDiagnostic`.
GoDoc says a nil `Write` function has no output. Coordination instead turns it
into an operational delivery failure with `diagnostic has no writer`. Thus an
otherwise successful result containing a zero diagnostic becomes a failure.
An error-severity diagnostic with a nil writer also suppresses automatic cause
presentation because deduplication checks severity alone.
**Remediation:** treat nil writers as no-output entries, consistent with the
documented behavior, and exclude them from cause-presentation deduplication and
delivery-attempt bookkeeping. Requiring every entry to have a writer would be a
new API restriction requiring a deliberate decision.
**Completion evidence:** nil writers do not change success/domain classifications
or suppress a real cause; active diagnostic writer failures remain observable.
### R-08: include available scope choices in selection failures
**Evidence:** [selectors.go](../../selectors.go), `selectConfigScopes`.
Unknown selection returns only `unknown configuration scope NAME`. This also
occurs when only a PromptKit scope exists and the default `app` scope is absent.
The operator receives no available choices.
**Authority:** CFG-01 expressly requires useful choices for unsupported scopes
and absent `app`; ADR 0009 requires available choices for unsupported selections.
**Remediation:** include registered scope choices in stable scope order. Retain
the usage classification, default selection, strict aggregate coverage, and
pre-acquisition failure; do not silently select an alternative.
**Completion evidence:** explicit unknown scope and omitted selection with absent
`app` identify available choices without invoking runtime callbacks.
### R-09: reserve selector names for actual command collisions
**Evidence:** [promptkit.go](../../promptkit.go), `normalizePromptKitSuite` and
`promptKitInputFlags`.
Normalization unconditionally rejects a selector called `version`, even when
only profile inspection is supplied. `--version` is installed only on prompt
inspection, so there is no flag collision in a profile-only suite. It also
reserves `scope` despite not installing a scope-selection flag on definitions.
**Authority:** capability-based family assembly and the agent policy's prohibition
on inventing restrictions. The selector rules in Stage 7, reused by Stage 12,
call for detecting collisions with standard local flags, not a reservation of
absent flags for hypothetical future use.
**Remediation:** detect collisions against flags actually installed on commands
receiving the selector. Keep genuine collisions rejected. If a broader vocabulary
reservation is desired, record it as a new decision rather than a parser necessity.
**Completion evidence:** a profile-only selector named `version` can register;
the same selector is rejected when it collides with installed prompt inspection.
Review the absent `scope` reservation against the same rule.
### R-10: expose registered metadata groups in parent help
**Evidence:** [command.go](../../command.go), `Group`;
[internal/cobra/parse.go](../../internal/cobra/parse.go), `buildTree` and
`usageTemplate`; pinned Cobra v1.10.1 `Command.IsAvailableCommand`.
CommandKit accepts a non-runnable group with no children, and direct invocation
renders its help. However, parent help filters commands with Cobra's
`IsAvailableCommand`, which requires a runnable command or an available
descendant. A metadata-only group, or a chain containing only such groups,
disappears from the parent's command inventory despite being registered and
addressable. The custom template provides no additional help-topic inventory.
**Authority:** HELP-01 and the architecture policy select discovery from registered
metadata; Cobra's availability defaults do not independently select a CommandKit
visibility restriction.
**Remediation:** derive help inventory from CommandKit's declared commands,
including groups that expose help. Do not prohibit empty groups or add fake
application handlers solely to satisfy a backend visibility rule.
**Completion evidence:** parent/root help discovers metadata-only groups and
their nesting; direct help works and leaves acquisition callbacks uninvoked.
## Follow-up boundaries
Address the behavioral findings with focused fixes and regression evidence at
the public invocation boundary or the narrow framing boundary as appropriate.
Update GoDoc/CLI references where needed to match the selected contracts.
Do not expand this work into upstream redaction changes, general framework
construction, new consumer mandates, blanket validation gates, or new concurrency
guarantees. Recheck the completed target after remediation; this review does not
claim that the existing tests establish these missing cases.