diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md new file mode 100644 index 0000000..9fcc7c0 --- /dev/null +++ b/docs/roadmap/implementation.md @@ -0,0 +1,306 @@ +# Shared debug binding implementation plan + +Status: planned; no implementation stages complete. +Baseline: `6727d22ce6e1ac668b56a3e6d8fa5f7a06e54d88`. +Execution audience: Luna coding agent. + +## Target and execution instructions + +The [feature roadmap](shared-debug-binding.md) owns the accepted behavior, +intent, examples, and scope. OD-1 and OD-2 are resolved. The original +Audita request is motivation, not an additional +capture policy for CommandKit. This plan owns implementation order and checks. + +Implement Stage 1 through Stage 7 in order as one end-to-end sprint after the +user approves implementation. Stages are work units, not approval checkpoints +or separate agent prompts. Continue through internal dependencies without +requesting permission after each stage. Intermediate stages need not compile +or leave a coherent repository. Record checks that depend on later stages as +pending, then resolve them before finishing; do not add temporary compatibility +code just to make an intermediate stage pass. Investigate and fix failures +within the selected scope. Surface a genuine contradiction rather than silently +changing the target. + +Start with [development guidance](../development.md) and follow the +[agent](../policy/agents.md), [architecture](../policy/architecture.md), +[testing](../policy/testing.md), and [documentation](../policy/documentation.md) +policies. Use graph discovery for code as directed by AGENTS.md. Preserve +unrelated work and the ignored workspace files. Record actual completion and +verification here as work lands. Commit completed nonempty units with plain +English messages; dependent stages may share a commit. This sprint does not +publish releases, push refs, or implement Audita artifact capture. + +## Implementation decisions + +- Accept the existing `BoolFlag` form named `debug`, including ordinary captured + and renamed boolean bindings. Its private `boolean` property identifies the + constructor semantics; a matching name alone does not. Retain validity, + duplicate-name, short-spelling, and wiring checks. Other reserved reporting + bindings remain rejected. There is no new public constructor or subscription. +- Retain every consumer entry in `commandmeta.Command.Flags` at its original + index, including shared debug. Retain the core reporting declarations in + `CoreFlags`. Produce one effective flag list for backend registration, + token interpretation, and completion, combining only the selected debug + overlap. Never remove a consumer entry to deduplicate parser registration. +- Build that effective list through one small helper in + [internal/commandmeta](../../internal/commandmeta/metadata.go), used by the + backend and completion. Copy metadata rather than changing captured + declarations. Combine core debug usage with application usage, short spelling, + requiredness, and completion settings directly. Keep the core reporting + default false; preserve the application default in its original binding. + When the application default is true, label it separately in help and explain + that reporting still defaults to ordinary. Exact prose and helper names are + implementation details; no generic metadata-merging facility is selected. +- When a debug binding participates, honor its completion hint just as for an + ordinary application boolean, including no completion or an empty choice list. + With no application binding, keep core's existing boolean suggestions. Hints + never constrain parsing. A command-local short spelling belongs only to that + command; existing flag aliases retain their existing scope and collision rules. +- Feed the single parsed occurrence sequence to reporting reduction and its + indexed raw debug values to ordinary typed preparation. Defaults and + requiredness are application input rules, not reporter selection inputs. +- Define public `ReportingMode uint8` and the four selected constants with + ordinary as zero. Use that type for the existing reporter mode field, replacing + the private mode type/constants rather than adding a second mutable selection. + Keep the internal parser's neutral type and translate at `applySelection`. + `Invocation.ReportingMode()` reads reporter state under its existing mutex, + returns ordinary for a zero invocation, and remains readable after finalization. + No public type aliases an internal/backend type; no setter or second public + accessor is added. Keep the existing reporting callback reentrancy contract. + +## Stage 1 — Expose the resolved reporting mode + +Primary files: [reporter.go](../../reporter.go), +[identity.go](../../identity.go), [execute.go](../../execute.go). + +1. Define `ReportingModeOrdinary`, `ReportingModeQuiet`, `ReportingModeVerbose`, + and `ReportingModeDebug` on the public type. Replace the private mode + representation in reporter state, visibility switches, and selection mapping. + Keep ordinary as the zero value. Update tests referring to removed private + identifiers without changing their behavioral expectations. +2. Add `func (inv Invocation) ReportingMode() ReportingMode`. Handle a missing + reporter/state as ordinary; otherwise read the existing field under its + mutex. Finalization must preserve the mode while releasing its current writer + references. Each execution continues to create its own reporter. +3. Preserve execution order: reduce explicit occurrences and call + `applySelection` before typed input preparation and the handler. Do not + infer mode from bound values, output, format, or progress. +4. Document the method's zero value, stable handler/finalized selection, and + compatibility with concurrent reporting. It reads selection, not delivery + success; it adds no concurrent App dispatch or writer-lifetime guarantee. +5. Use `shared_debug_external_test.go` (package `commandkit_test`) as the public + behavior owner for this API and the shared-binding table in Stage 3. Add + checks for all four modes, a zero invocation, retained invocation after + return, and a subsequent execution with no flags. + Exercise concurrent mode reads alongside reporter operations using bounded + goroutines and explicit synchronization, not sleeps or concurrent dispatch. + Reuse an existing safe in-memory destination and existing visibility checks. + +Complete when the accessor observes the existing selection without side effects, +and concurrent use and retained invocations preserve the selected result. + +## Stage 2 — Recognize shared boolean bindings and compose metadata + +Primary files: [command.go](../../command.go), [binding.go](../../binding.go), +[parser.go](../../parser.go), and +[neutral metadata](../../internal/commandmeta/metadata.go). + +1. In declaration validation, allow the reporting reservation exception only + for a valid boolean binding named `debug`. The existing boolean property is + carried by `BoolFlag`, `CaptureFlags`, and `RenameFlag`; it is not set by + `ValueFlag` or `OccurrenceFlag`. Preserve all ordinary validation, including + missing setters, duplicate debug bindings, other reserved names, duplicate + shorts and reserved `-h`. Do not accept a custom converter because it returns + a Go bool. +2. Keep the bridge's consumer metadata and indexes intact. Add the effective + list helper described above: retain ordinary consumer flags, replace the + participating debug entry with the combined declaration, then append the + remaining core flags. Only the boolean debug overlap is combined. The helper + does not turn invalid duplicate application declarations into valid ones. +3. The combined declaration remains boolean with core's false reporting + default. Use application short spelling, requiredness, and completion hint; + combine usage text and label a differing application default. The original + consumer declaration keeps its own default for typed preparation. Backend + raw input remains empty on omission; metadata must not synthesize an explicit + occurrence or presence. +4. Adjust the reservation tests in [flag_aliases_test.go](../../flag_aliases_test.go) + to distinguish accepted `BoolFlag` from still-rejected string/custom-parser + declarations. Add focused checks for successful registration and duplicate + or invalid wiring rejection, retaining existing transactional registration + assertions. Test the effective metadata at its package boundary where that + gives distinct protection; do not snapshot private slice layouts wholesale. + +Complete when declarations express the agreed exception and one effective debug +option can coexist with unchanged consumer indexing and application defaults. +Backend-dependent checks may remain pending until Stage 3. + +## Stage 3 — Parse one debug option into both consumers + +Primary files: [backend parse.go](../../internal/cobra/parse.go), +[backend flags.go](../../internal/cobra/flags.go), +[parser.go](../../parser.go), [binding.go](../../binding.go). + +1. Use the effective list in `buildTree` registration and `Session.Parse` token + interpretation. Iterate it once when queuing entered spellings; do not queue + the same raw debug value twice. Retain existing pflag boolean no-value syntax, + raw-value collection, alias interpretation, and failure precedence. +2. Extract `FlagValues` and `FlagSpellings` by iterating the original consumer + `Flags`, looking up each effective backend flag by canonical name. This + delivers shared debug to its original index while core-only flags still + occupy no consumer index. Preserve the occurrence log's canonical names, + entered spellings, and order, including interleaving with other flags. +3. Keep `reduceReportingOccurrences` authoritative for visibility and invalid + reporting values. Use ordinary `BoolFlag.prepare` for the application value. + Do not add a setter path in the reporter, rescan argv, or reconstruct mode + from the final boolean. Reporting must still work on unbound commands, + root/group help, version, and discovery routes. +4. At the backend boundary, extend + [parse_test.go](../../internal/cobra/parse_test.go) and existing flag grammar + coverage only where needed to establish shared extraction, stable indexes + with ordinary flags before/after debug, and one occurrence stream through + aliases/short spellings. Do not duplicate the facade behavior table here. +5. At the public command boundary, extend `shared_debug_external_test.go` with + the roadmap's mode/value/presence table, observing both typed input and the + new accessor. Check invalid/empty boolean input prevents factories and handler + execution as appropriate under existing preparation timing. Keep tokens consumed as + another flag's value and operands following `--` out of reporting reduction. + +Complete when debug feeds normal typed preparation and mode selection from one +parse, without changing unrelated input indexes, grammar, or failure handling. + +## Stage 4 — Preserve composition, defaults, and alias behavior + +Primary files: [flag_values.go](../../flag_values.go), +[binding.go](../../binding.go), [flag_aliases.go](../../flag_aliases.go). + +1. Review the shared binding through `CaptureFlags`, `FlagValues.ApplyTo`, and + `ApplySuppliedTo`. Retain original binding identity, independently selectable + destinations, and existing conversion timing. Capture already preserves the + boolean property; avoid adding another store or observer if no change is needed. +2. Preserve `deferredRequired`: direct bindings enforce required input during + ordinary preparation, while captured bindings check requiredness when selected + for application, as they do today. An explicit false satisfies presence. + A true application default with `Set=false` must not satisfy requiredness or + turn on reporting. `ApplySuppliedTo` must keep its ordinary omission behavior. +3. Keep shared behavior determined by the current binding name and boolean + semantics. Renaming debug to an ordinary name yields an ordinary independent + boolean and stops observing core debug; renaming an ordinary boolean to debug + uses the same accepted attachment. Do not leave a sticky sharing marker. +4. Review canonical short spellings and alias compilation together. A local + debug short and aliases targeting its canonical debug input must feed the + same occurrence stream. Preserve existing collision errors, scope, and + registration rollback; do not add short spellings to unrelated commands. +5. Extend [flag_values_test.go](../../flag_values_test.go), existing rename and + alias tests for representative captured application, true default/explicit + false, requiredness, renamed-away behavior, and an alias targeting the local + short spelling. Use the public table from Stage 3 for visibility behavior + instead of repeating it in each composition test. Modify production code + here only where the selected behavior actually needs a change. + +Complete when shared binding behaves like an ordinary reusable boolean across +capture, selection, application and rename, with visibility still independent. + +## Stage 5 — Present one debug option in help and completion + +Primary files: [completion_flags.go](../../completion_flags.go), +[internal/cobra/parse.go](../../internal/cobra/parse.go), +[internal/commandmeta](../../internal/commandmeta/metadata.go). + +1. Make `completionFlags` use the same effective declarations as the backend, + alongside the existing help flag. Eliminate the duplicate debug long option + without suppressing the application's local short spelling. Keep canonical + suggestion and existing alias interpretation policies; this is not alias + discovery work. +2. Ensure canonical command help, explicit help targets, alias help, and usage + failures show the combined debug option once. Core usage and application + usage must both be intelligible; a true application default is labeled as + application input, alongside the ordinary reporting default. Retain existing + required markers and help's exemption from required-input enforcement. +3. Preserve application completion hints, including choices, no completion, and + file/directory hints under ordinary handling. An arbitrary suggested value + remains subject to boolean parsing. Keep normal unbound core suggestions; + do not introduce a debug-specific completion rejection rule. +4. Use existing completion query tests and help tests to check one long debug + suggestion/row, the local short, a true default/required help case, explicit + boolean value completion, and one custom hint. Assert meaningful fragments + and counts for the effective option, not a complete help snapshot or exact + new explanatory wording. Queries and help must not call input factories, + setters, `CheckInput`, handlers, or providers. Retain help's existing conversion + of supplied input for syntax validation; completion continues to use metadata. + +Complete when help and completion describe the same single parsed option and +clearly distinguish application defaults from reporting selection. + +## Stage 6 — Document the implemented contracts + +1. Update `BoolFlag`, `FlagMeta`, binding composition, and Invocation GoDoc with + the supported debug exception and mode API. Explain defaults, presence, + requiredness, short spellings, and retained mode without making all reporting + names bindable or treating mode as delivered-output evidence. +2. Update the [CLI reference](../cli.md#placement-and-aliases) where it describes + reserved names and command help. Keep API wiring in the + [consumer guide](../consumers/guide.md#invocation-reporting). Add a small + compiling example showing `BoolFlag` on `debug` and the mode query, using + `--debug --quiet` to explain why their results differ. Applications still own + artifact creation and other effects. +3. Add the feature and consumer impact to the next core minor candidate note, + `docs/releases/v0.6.0.md`, using the release and OSS policies for its title + and immutable licensing footer. Core v0.5.0 is published and must not be + rewritten. If version state has changed before implementation, select the + next unused minor through the release procedure; this is release bookkeeping, + not a new decision about this feature. No tag or publication is part of this + sprint. Integration dependencies and upstream versions need no upgrade here. +4. Update [internal guidance](../internal/overview.md) only where the effective + flag view and authoritative reporter mode change its explanation. Validate + changed links, fragments, Go snippets, and prose against the final code. + Do not introduce a new ADR for this addition. + +Complete when GoDoc owns the precise API, operator guidance explains the flag, +and consumer guidance distinguishes input policy from reporting selection. + +## Stage 7 — Verify the assembled sprint and record completion + +1. Review every target/checklist item in the feature roadmap against the final + implementation and evidence. Ensure tests that formerly rejected all reserved + reporting bindings now allow only the selected boolean form. Review capture + and requiredness separately from the direct binding cases. +2. Run focused tests as the stages become testable, then the independent core + suite and the aligned three-module suite with Go 1.27.0 and cached dependencies: + + ```sh + GOWORK=off GOTOOLCHAIN=local GOPROXY=off go test -mod=readonly ./... + GOTOOLCHAIN=local GOPROXY=off go test -mod=readonly ./... ./integrations/ckconfig/... ./integrations/ckprompt/... + ``` + + Use the ignored workspace described in the developer guide. The aligned + integration run exercises this changed core; independent integrations still + select released core v0.5.0 and cannot establish that boundary by themselves. + Do not upgrade manifests or commit local replacements to run these checks. +3. Run the new focused concurrent mode-read check under `-race -count=1` when + supported; choose its actual test name after implementation. No full race + matrix, timing assertion, live provider, or Audita processing run is needed. + Record unavailable optional tooling accurately and continue with supported + verification; never describe an unavailable check as passed. +4. Run gofmt on changed Go files; inspect diffs and verify current Markdown links + and fragments, snippet compilation, and tracked/staged prose whitespace using + the developer guide. Use a temporary `GOCACHE` if the default is unwritable. + Reuse passed checks for unchanged content; repeat for actual changes or failures. +5. Record completed stages, actual checks, limitations, and residual release + preparation here. Mark the feature checklist complete only with its evidence. + Retire completed planning records through the normal documentation lifecycle, + preserving unfinished work in `future.md`, repairing links and retaining a + Git-history reference. Do not turn source completion into a publication claim. +6. Review and commit the remaining completed nonempty changes. Report the API + and behavior delivered, verification, any remaining work, commit identifiers, + and worktree state. Resolve supported failing checks within scope before + claiming completion; a broad passing suite does not replace missing behavior. + +Complete when the agreed target is implemented, verified and documented as one +coherent source state, with no new consumer restrictions or application policies. + +## Open Questions + +None. The feature roadmap resolves the public behavior and settings policy; +this plan selects the internal approach and validation ownership. Private helper +names, exact help prose, and test organization are ordinary implementation +choices within these decisions, not additional product gates. diff --git a/docs/roadmap/shared-debug-binding.md b/docs/roadmap/shared-debug-binding.md index 2e3eec8..926aa39 100644 --- a/docs/roadmap/shared-debug-binding.md +++ b/docs/roadmap/shared-debug-binding.md @@ -1,7 +1,8 @@ # Shared debug binding and reporting-mode access Status: agreed scope and public behavior; not implemented. OD-1 and OD-2 are -resolved; no significant open decisions remain. +resolved; no significant open decisions remain. The +[implementation plan](implementation.md) owns the ordered stages and verification. Baseline: `69257066e21c12ea3492237ebee072598adea60c`. ## Purpose and authority @@ -10,7 +11,7 @@ Applications should be able to receive the boolean value of CommandKit's `--debug` flag through normal typed input binding. Applications that need the resolved reporting mode should also have a read-only invocation API. -The [Audita request](audita-feature-request.md) motivated this work. Subsequent +The original Audita request motivated this work. Subsequent discussion selected a broader distinction: application debug behavior and CLI reporting visibility are separate choices. Audita's original requirement that capture follow reporting mode is an application policy, not a CommandKit @@ -63,6 +64,11 @@ declared default with `Set=false`; an explicitly supplied false produces binding semantics: every supplied value must be valid, and the last value wins. Other reporting flags do not rewrite the bound debug value. +Shared bindings retain ordinary capture, identity, presence, and application +semantics through `CaptureFlags`, `FlagValues.ApplyTo`, and `ApplySuppliedTo`. +Renaming the boolean to an ordinary application name makes it independent of +core debug input while retaining its ordinary binding behavior. + The following examples appear after a complete command path. They show the ordinary false-default binding and the existing mode-selection rules together: @@ -148,52 +154,18 @@ Progress and report format remain separate selections. Keep the public type independent of internal/backend types and avoid a second mutable copy of the resolved mode. A second accessor on `Reporter` is unnecessary for this scope. -## Implementation work - -1. **Implement ordinary binding attachment and metadata handling.** Use the - selected `BoolFlag` form and the accepted application metadata rules above. - Preserve application defaults, presence, requiredness, and short spellings; - combine help without changing core reporting selection. Preserve ordinary - validation for unrelated names and bindings; do not broadly disable reserved - name or collision checks. -2. **Connect shared parsing to typed input.** Update declaration validation, - metadata bridging, and backend value extraction together. Register one - effective debug flag, retain canonical occurrence order and supplied - spellings, and deliver its values through normal input preparation. Keep - reporting reduction based on the same parsed occurrences, including for - commands that have no application debug binding. Reuse current mechanisms; - a general observer registry, alternate parser, or subscription lifecycle is - not needed for this feature. -3. **Preserve binding composition.** Review `CaptureFlags`, `FlagValues.ApplyTo`, - and `ApplySuppliedTo` in [flag_values.go](../../flag_values.go). A shared - binding should participate through the existing identity and presence - machinery. Review `RenameFlag` if a special binding marker is introduced: - renaming to an ordinary application name must not accidentally keep reading - core debug input. Avoid adding separate storage or application callbacks just - to carry this boolean. -4. **Expose the mode query.** Reuse reporter state and its synchronization as - appropriate; document the zero-value, invocation lifetime, and concurrent-read - behavior. Do not expose backend objects or add public mutation APIs. -5. **Update canonical documentation when implemented.** Update binding and - invocation GoDoc, the reserved-name wording in `FlagMeta`, and the - [CLI reference](../cli.md#placement-and-aliases). Add a short example to the - [consumer guide](../consumers/guide.md#invocation-reporting) contrasting a - bound debug boolean with an explicit mode query. Explain application effects - in application help using the accepted metadata rules. Record the core API - addition in the applicable release note. Retire this roadmap through the - normal documentation lifecycle when complete. +## Scope and documentation This is core work. It does not require ConfigKit or PromptKit API additions or adoption of Audita's artifact implementation. Binding other core flags, adding mode setters, or creating a general flag-sharing framework is not part of this sprint; that scope limit is not a permanent prohibition on later capabilities. -## Validation and completion checklist +## Completion criteria -Follow the [testing policy](../policy/testing.md): test the public behavior and -reuse existing parser and reporting coverage rather than duplicate it wholesale. -The table above supplies representative boundary expectations, not a request -for separate copies of the same cases at every layer. +The table above supplies representative boundary expectations. The +[implementation plan](implementation.md) assigns test ownership and verification +under the [testing policy](../policy/testing.md). - [ ] A registered application command receives the shared debug boolean and presence through normal typed input preparation, including when quiet or @@ -212,15 +184,14 @@ for separate copies of the same cases at every layer. overriding that default, and ordinary requiredness/presence behavior. Help combines core and application explanations and distinguishes the application default; local short spellings use existing collision and alias rules. -- [ ] Replace obsolete blanket rejection assertions for the allowed binding - form. Preserve ordinary metadata handling without new debug-specific - restrictions beyond the selected boolean binding contract. -- [ ] Run focused binding/parser/reporter tests, then the core suite and aligned - integration suites using the [developer guide](../development.md). The shared - parser change justifies checking that standard integration commands still - register and execute. No live providers or Audita processing tests are needed. +- [ ] The allowed boolean binding is accepted while unsupported/custom-parser + debug declarations and other reserved reporting declarations retain their + ordinary validation. Metadata handling adds no debug-specific restrictions + beyond the selected boolean binding contract. - [ ] GoDoc and consumer guidance distinguish boolean input from reporting mode, and canonical docs describe the implemented registration exception. -For this documentation pass, review baseline accuracy, scope, links, and -whitespace only. Implementation and the unchecked items above remain future work. +Canonical binding and invocation GoDoc, the CLI reference, consumer guidance, +and the applicable core release note will describe the implemented exception +and mode query. The implementation plan owns the steps to reach and verify +these criteria; the unchecked items remain future work.