jn
/
punkshell
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity
You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
1420 Commits
1 Branch
1 Tag
224 MiB
 
 
 
 
 
 
Tree: ea5971a1fa
Branches Tags
${ item.name }
Create tag ${ searchTerm }
Create branch ${ searchTerm }
from 'ea5971a1fa'
${ noResults }
punkshell/goals/G-160-commandstack-hygiene.md

8.7 KiB
Raw Blame History

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

Status: proposed 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.
  • 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.