punkshell/goals/archive/G-160-commandstack-hygiene.md

G-160 commandstack hygiene pass: pinned-defect fixes and API ergonomics

Status: achieved 2026-08-03 Scope: src/modules/commandstack-999999.0a1.0.tm (token integrity, get_IMPLEMENTOR, debug validation, channel discipline, did_rename record key, -renamer placement, O(1) dispatch, Rename_stack/Delete_stack semantics, delegation helper, known_renamers defaults, PUNKARGS/help updates); src/modules/punk/packagepreference-999999.0a1.0.tm (install re-install guard record-key fix only); src/tests/modules/commandstack/testsuites/commandstack/commandstack.test (GAP flips + new pins); src/tests/modules/punk/packagepreference/testsuites/packagepreference/ (new double-install pin); src/vendormodules/packagetrace-.tm + src/vendormodules/packagesuppress-.tm (verification context only - not edited) Goal: the defects pinned by the 2026-08-03 commandstack characterisation suite are fixed and the API's sharp edges removed, so a developer can stack, re-stack and unstack overrides without foot-guns: per-(renamer,command) tokenids are unique and monotonic (same-renamer re-renames dispatch through the full chain and are individually removable by token), get_IMPLEMENTOR actually classifies builtins where cmdtype exists, debug validates its argument, informational output is debug-gated on defined channels, the rename record carries an explicit did_rename verdict without breaking the lsearch key-order contract, a misplaced -renamer errors instead of silently misparsing, per-call delegation resolution no longer scans the stack list, parked stacks are retrievable, and the packagepreference double-install guard reads the record key that exists. Acceptance: commandstack minted at >= 0.6.0 with all of: (1) the four GAP tests in the commandstack suite flipped to fixed-behaviour pins - same-renamer re-rename gets a distinct tokenid, delegates to that renamer's previous override (not the original) and is removable by exact token; a third same-renamer rename succeeds; get_IMPLEMENTOR returns builtin for a native command where tcl::info::cmdtype exists (undetermined on 8.6); Delete_stack with live renames errors (or an otherwise-pinned safe behaviour) instead of recursing; (2) new pins proving - debug rejects a non-boolean argument with a clean error naming the argument; with debug off a full rename/re-rename/remove cycle emits nothing on stdout or stderr (puts-shim assertion); records carry a trailing did_rename 0|1 with token still the first and renamer still the second dict entry; rename_command errors when -renamer appears anywhere but the leading position; get_stack retrieves a Rename_stack-parked stack; Rename_stack no longer returns the whole stacks dict; a delegation helper (working name commandstack::next) is PUNKARGS-documented, named in help, and demonstrated delegating correctly by a pin; the known_renamers defaults are reconciled with the vendored packages' actual renamer strings (decision recorded in this file) and the defaults pin updated; (3) get_next_command resolves tokens via dict lookup rather than a stack-list scan, with the mechanism and a timing or source-level verification note recorded under Progress; (4) punk::packagepreference::install reads the renamer key and a new double-install test proves a second install returns 0 silently; (5) the suite is green on tclsh90 and punk86, with all pre-existing non-GAP pins unchanged - the consumer patterns (packagepreference, punk::nav::fs, punk auto_execok, packagetrace, packagesuppress) keep working against the fixed module.

Context

The 2026-08-03 characterisation arc (commandstack 0.5.0 + first suite, git 145e785c / fossil 9ed984f5) pinned four defects as GAP tests and surfaced several ergonomic hazards. The sharpest defect: rename_command increments renamer_command_tokens inside its apply lambda, where the name is apply-local - the namespace variable never changes, tokenid is always 1, so a same-renamer re-rename stacks a DUPLICATE token. Because get_next_command takes the first lsearch match, the newer override then delegates straight to the oldest record, silently bypassing every intermediate layer (including other renamers' overrides), and a third same-renamer rename dies on a parked-name collision. This breaks the reload-my-package-during-development case and makes the documented exact-token removal of non-topmost same-renamer entries unusable.

Secondary defects: the get_IMPLEMENTOR builtin branch is unreachable on every Tcl version (unqualified info commands tcl::info::cmdtype pattern inside commandstack::util - info commands pattern namespaces resolve relative-only, unlike command invocation); debug validates the current variable rather than its argument; a stray bare puts stderr prints the literal word stderr to stdout; Delete_stack under a live rename leaves the override delegating to itself (recursion-limit error on next call); punk::packagepreference::install's already-installed guard reads record key rename where the key is renamer, so a second install throws instead of silently returning (latent - kits install once).

Ergonomic hazards from the same review: consumers must know that implementation eq "" means no-rename (implicit contract); every consumer hand-writes uplevel 1 [list $COMMANDSTACKNEXT {*}$args]; -renamer is recognised only as the first argument and is otherwise silently consumed as the command name; get_next_command does an lsearch on every call of the renamed command (hot for ::package - packagepreference already hardcodes around it); Rename_stack leaks the whole stacks dict as its return value and parked stacks are invisible to get_stack (namespace which resolution); the known_renamers defaults (::packagetrace ::packageSuppress) do not match what the vendored packages actually register (packagetrace / packagesuppress, plain words).

Approach

• tokenid: compute the increment against the real namespace variable (in the proc frame before the apply, or namespace-qualified access from inside it); tokens become unique and monotonic per (renamer, command).
• get_IMPLEMENTOR: fully qualify ::tcl::info::cmdtype in both the guard and the call.
• Channel discipline: route the informational warnings and the code-diff dump through the debug flag; fix the stray bare puts; errors stay errors.
• did_rename: APPEND the key - the record key ORDER (token first for lsearch -index 1, renamer second for lsearch -index 3) is a load-bearing contract pinned by the suite.
• Delete_stack: refuse (error) while the stack has live renames, or an equivalent guarded behaviour settled in the work; Rename_stack returns something deliberate.
• Delegation helper: settled in the work (helper resolving the caller's token, or an injected per-override convenience); the COMMANDSTACKNEXT variables remain, as consumers depend on them.
• packagepreference: two-character key fix plus the missing double-install test.
• Module stays dependency-free (no punk::args require; PUNKARGS docs stay inert/lazy).

Alternatives considered

• Searching the stack from the top (lsearch last-match) to tolerate duplicate tokens - rejected: unique tokens are the actual fix; last-match papers over removal semantics.
• Restructuring the record dict (explicit status field first / dict-of-dicts) - rejected for this pass: breaks the lsearch key-order contract consumers and the suite pin rely on; appended keys deliver the same information compatibly.

Notes

• Related: G-035 (sibling surface on src/modules/punk/packagepreference-999999.0a1.0.tm - that goal characterises mixed .tm/pkgIndex provision with packagepreference active and fixes it only for outright bugs in the provision machinery; this goal's packagepreference change is an outright bug in the commandstack-guard path, unrelated to provision semantics, and lands here).
• Overlap survey 2026-08-03: goals_xref paths over the Scope surfaced only G-035 (via packagepreference); no live goal touches commandstack itself or its test tree.
• Activation survey 2026-08-03 (freshness rule re-run, goals_xref score G-160): no goals drafted since this goal's drafting commit; top score remains G-035 (already Related: above). Next scores G-108 (debug buildsuite runtimes) and G-090 (shellfilter stack audit) are lexical-only overlap (debug/stack vocabulary), different subsystems, no dependency - no new Related: lines.
• Baseline: characterisation suite (29 tests) green on tclsh90 9.0.3 + punk86 8.6 at commandstack 0.5.0 (git 145e785c); the GAP tests to flip are commandstack_GAP_samerenamer_new_body_duplicate_token_bypass, commandstack_GAP_third_samerenamer_rename_name_collision, commandstack_GAP_rerename_after_other_renamer_bypasses_chain (subsumed by the tokenid fix), commandstack_GAP_delete_stack_live_rename_recursion, commandstack_GAP_get_implementor_builtin_branch_unreachable.
• Compatibility floors: packagesuppress does package require commandstack 0.3 (minimum bound - satisfied); bootsupport carries 0.3/0.4 snapshots and _vfscommon 0.4.x/0.5.0 until the next promotion cycle picks up the fixed module.
• Version: minor bump (0.6.0) expected - additive keys and fixes; the Delete_stack guard changes an unexported maintenance proc's behaviour (changelog-noted). Revisit as major only if the settled semantics break an exported surface.

Progress

2026-08-03 - full pass landed in one arc (activation bac3802c, source increment git 23077dc1 / fossil 52a1db73): commandstack 0.6.0 + punk::packagepreference 0.2.1, suite flips + new pins, minted, punkshell 0.49.3.

Decisions settled in the work (per the Approach's "settled in the work" clauses):

• tokenid mechanism: incremented on the real namespace variable in the proc frame BEFORE the apply and passed in as an apply parameter (the Approach's first option). Aborted renames (same-body refusals) consume an id - gaps are deliberate; uniqueness and monotonicity are the contract, not density. The known-renamer seeding (dict set ... 0) was dropped - dict incr creates missing keys, so seeding added nothing and its skip-when-default path was a latent inconsistency.
• O(1) dispatch mechanism (acceptance 3): a token_implementations namespace dict (token {command renamer tokenid} -> implementation) maintained by rename_command (set on stack push), remove_rename (unset doomed token; re-point the rewritten record's token at the doomed record's implementation). get_next_command is now a single dict exists/dict get pair - source-level verification: the proc contains no lsearch and no stack-list read on the hit path (the all_stacks lookup remains only to distinguish the two miss cases: stacked-but-bad-token error vs no-stack passthrough). Deliberate side effect: dispatch keeps working while a stack is parked under a Rename_stack key (map is token-keyed, not command-keyed) - pinned. Map/stack sync is guarded long-term by the record_key_order_and_token_map pin (every record's token maps to its implementation, map size == total records, through rename/remove sequences).
• Delete_stack settled semantics: ERROR while the stack holds records (live renames - the recursion foot-gun); empty-residue and missing entries return 1 (its legitimate maintenance use - pruning the empty key remove_rename leaves behind).
• Rename_stack returns 1 (moved) / 0 (no stack at oldname); error on existing newname unchanged.
• Channel discipline: ALL informational/warning output (missing-command, same-body abort + stack display, new-code notice + code dump, not-immediate-predecessor, unknown-implementor cooperate notice, remove_rename's next_implementor-mismatch anomaly warning) is debug-gated, unified on stderr (the code dump moved from stdout). The mismatch warning was gated rather than kept always-on: it reports a conservative proceed (parked proc left in place) in externally-interfered stack states, the state stays inspectable via show_stack, and erroring would break removal in exactly the states needing repair. Errors remain errors. The stray bare puts stderr (which printed the literal word stderr to stdout) is gone.
• known_renamers reconciliation (acceptance 2 "decision recorded in this file"): defaults changed to the strings the vendored packages actually register - packagetrace packagesuppress (plain words, per their rename_command -renamer calls; packagesuppress 0.4 renamed all-lowercase in 2024). The historical ::packagetrace ::packageSuppress matched no registration ever made. The defaults matter for cross-reload recovery: get_IMPLEMENTOR reads the magic comment of an override installed before a commandstack state reset, and only a default matching the recorded renamer string routes that into the cooperate branch; the stale defaults also let remove_rename accept never-registered renamer strings. The defaults pin (commandstack_known_renamers_defaults) updated.
• did_rename shape: performed-rename records append did_rename 1 as the LAST key (token/renamer stay 1st/2nd - lsearch -index 1/-index 3 contract pinned unchanged); both no-rename paths return {implementation {} did_rename 0} so consumers may test either key (in-tree consumers test implementation eq "" - unchanged).
• commandstack::next: helper resolving the CALLER's COMMANDSTACKNEXT via upvar 1 and invoking it at the override's caller frame (uplevel 2) - result and caller-context equivalent to the manual uplevel 1 [list $COMMANDSTACKNEXT {*}$args] convention (pinned by a namespace-context probe); clean error outside an override body. The COMMANDSTACKNEXT variables remain the primitive interface (consumers unchanged).
• "Pre-existing non-GAP pins unchanged" (acceptance 5) interpretation: pins whose expected values encode surfaces this goal deliberately changed were updated minimally to the contracted behaviour (basic_record + missing-command + same-body pins gained did_rename / debug-gating context, known_renamers defaults pin reconciled, Rename_stack parking pin now asserts retrievability + 1/0 returns); every other pre-existing pin passes untouched. The five GAP tests flipped to fixed-behaviour pins under non-GAP names (samerenamer_new_body_distinct_token_chain, third_samerenamer_rename_succeeds, rerename_after_other_renamer_chains, delete_stack_guard, get_implementor_classification).

Verification evidence:

• commandstack suite 33/33 (29 baseline tests: 5 flipped in place, none removed; plus 4 new tests) green on tclsh90 9.0.3 and the punk86 kit (8.6).
• packagepreference suites 6/6 on both runtimes: new installguard.test (double install returns 0 silently with one stack record; uninstall/re-install cycle gets the next unique tokenid {::package punk::packagepreference 2}, all silent) + staticpolicy.test 4/4 unchanged.
• Live consumer smoke (bin/punk91 src script): source commandstack 999999.0a1.0 loaded in a real shell boot - packagepreference installed on ::package (1 record, renamer punk::packagepreference, did_rename key present), package require http resolves through the override, punk auto_execok and punk::nav::fs cd stacks live.
• Minted via tclsh90 src/make.tcl modules: modules/commandstack-0.6.0.tm + modules/punk/packagepreference-0.2.1.tm; minted copy requires clean and exposes commandstack::next. Bootsupport/vfscommon snapshots deliberately not promoted (per Notes - next promotion cycle picks the fixed module up).

Follow-ons

Follow-on: remove_rename convenience forms parked as code todos (pop topmost-for-renamer, remove all entries for a renamer, restore-to-original regardless of stack) => landed 2026-08-04 (commandstack 0.7.0 - pop_rename/remove_renamer/restore_original) Follow-on: a module re-source resets known_renamers and debug while all_stacks/renamer_command_tokens/token_implementations survive their info-exists guards - guard known_renamers likewise or document the reload contract => landed 2026-08-04 (commandstack 0.7.1 - both guarded; reload contract documented in commandstack::help, pinned by commandstack_reload_preserves_state)