333 lines
21 KiB
Markdown
333 lines
21 KiB
Markdown
# Shared debug binding implementation plan
|
||
|
||
Status: in progress; Stages 1–4 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
|
||
|
||
**Complete.** The invocation exposes the existing reporter selection, including
|
||
zero and finalized values. Focused reporting and public accessor checks passed
|
||
with Go 1.27.0 and offline module resolution; concurrent reads also passed with
|
||
`-race -count=1`.
|
||
|
||
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
|
||
|
||
**Complete.** Declaration validation accepts only boolean debug bindings and
|
||
retains other reservation and wiring checks. Effective metadata combines the
|
||
shared option without removing consumer entries. Focused validation and metadata
|
||
tests passed offline with Go 1.27.0; backend adoption follows in Stage 3.
|
||
|
||
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
|
||
|
||
**Complete.** Backend registration and interpretation use effective flags while
|
||
consumer values retain their original indexes. Public mode/value cases, aliases,
|
||
invalid values, option-looking values and terminated operands pass, as do focused
|
||
backend parsing checks, offline with Go 1.27.0.
|
||
|
||
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
|
||
|
||
**Complete.** Existing capture and rename machinery retains the boolean
|
||
identity and requiredness without production changes. Focused public tests
|
||
cover true defaults, explicit false, direct and captured requiredness, both
|
||
application methods, renaming in both directions, and an alias to the local
|
||
short spelling; capture and alias tests pass offline with Go 1.27.0.
|
||
|
||
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
|
||
|
||
**Complete.** Completion uses the same effective declarations as parsing and
|
||
help. Canonical and alias help and usage failures expose the combined option;
|
||
focused tests cover local shorts, requiredness, application defaults, choices,
|
||
file and directory hints, boolean validation, and idle application callbacks.
|
||
|
||
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.
|